On Wed, Feb 13, 2019 at 12:26:18PM +0300, Dan Carpenter wrote:
> Hello Leon Romanovsky,
>
> The patch 21a428a019c9: "RDMA: Handle PD allocations by IB/core" from
> Feb 3, 2019, leads to the following static checker warning:
>
> drivers/infiniband/core/uverbs_cmd.c:440 ib_uverbs_alloc_pd()
> warn: 'pd' was already freed.
>
> drivers/infiniband/core/uverbs_cmd.c
> 393 static int ib_uverbs_alloc_pd(struct uverbs_attr_bundle *attrs)
> 394 {
> 395 struct ib_uverbs_alloc_pd cmd;
> 396 struct ib_uverbs_alloc_pd_resp resp;
> 397 struct ib_uobject *uobj;
> 398 struct ib_pd *pd;
> 399 int ret;
> 400 struct ib_device *ib_dev;
> 401
> 402 ret = uverbs_request(attrs, &cmd, sizeof(cmd));
> 403 if (ret)
> 404 return ret;
> 405
> 406 uobj = uobj_alloc(UVERBS_OBJECT_PD, attrs, &ib_dev);
> 407 if (IS_ERR(uobj))
> 408 return PTR_ERR(uobj);
> 409
> 410 pd = rdma_zalloc_drv_obj(ib_dev, ib_pd);
> 411 if (!pd) {
> 412 ret = -ENOMEM;
> 413 goto err;
> 414 }
> 415
> 416 pd->device = ib_dev;
> 417 pd->uobject = uobj;
> 418 pd->__internal_mr = NULL;
> 419 atomic_set(&pd->usecnt, 0);
> 420 pd->res.type = RDMA_RESTRACK_PD;
> 421
> 422 ret = ib_dev->ops.alloc_pd(pd, uobj->context, &attrs->driver_udata);
> 423 if (ret)
> 424 goto err_alloc;
> 425
> 426 uobj->object = pd;
> 427 memset(&resp, 0, sizeof resp);
> 428 resp.pd_handle = uobj->id;
> 429 rdma_restrack_uadd(&pd->res);
> 430
> 431 ret = uverbs_response(attrs, &resp, sizeof(resp));
> 432 if (ret)
> 433 goto err_copy;
> 434
> 435 return uobj_alloc_commit(uobj);
> ^^^^^^^^^^^^^^^^^^^^^^^
> Not related to your patch, but we should probably free "pd" on the
> error path if this fails?
It will internally call to proper "destroy path", including ib_dealloc_pd().
>
> 436
> 437 err_copy:
> 438 ib_dealloc_pd(pd);
> ^^
> Freed.
>
> 439 err_alloc:
> --> 440 kfree(pd);
> ^^
> Double free.
Thanks, I sent patch with fix.
>
> 441 err:
> 442 uobj_alloc_abort(uobj);
> 443 return ret;
> 444 }
>
> regards,
> dan carpenter
Attachment:
signature.asc
Description: PGP signature
