> -----Original Message----- > From: Jason Gunthorpe <[email protected]> > Sent: Thursday, November 12, 2020 4:34 PM > To: Xiong, Jianxin <[email protected]> > Cc: [email protected]; [email protected]; Doug Ledford > <[email protected]>; Leon Romanovsky > <[email protected]>; Sumit Semwal <[email protected]>; Christian Koenig > <[email protected]>; Vetter, Daniel > <[email protected]> > Subject: Re: [PATCH v10 3/6] RDMA/uverbs: Add uverbs command for dma-buf > based MR registration > > On Tue, Nov 10, 2020 at 01:41:14PM -0800, Jianxin Xiong wrote: > > + mr = pd->device->ops.reg_user_mr_dmabuf(pd, offset, length, virt_addr, > > + fd, access_flags, > > + &attrs->driver_udata); > > + if (IS_ERR(mr)) > > + return PTR_ERR(mr); > > + > > + mr->device = pd->device; > > + mr->pd = pd; > > + mr->type = IB_MR_TYPE_USER; > > + mr->uobject = uobj; > > + atomic_inc(&pd->usecnt); > > + > > + uobj->object = mr; > > + > > + uverbs_finalize_uobj_create(attrs, > > +UVERBS_ATTR_REG_DMABUF_MR_HANDLE); > > + > > + ret = uverbs_copy_to(attrs, UVERBS_ATTR_REG_DMABUF_MR_RESP_LKEY, > > + &mr->lkey, sizeof(mr->lkey)); > > + if (ret) > > + goto err_dereg; > > + > > + ret = uverbs_copy_to(attrs, UVERBS_ATTR_REG_DMABUF_MR_RESP_RKEY, > > + &mr->rkey, sizeof(mr->rkey)); > > + if (ret) > > + goto err_dereg; > > + > > + return 0; > > + > > +err_dereg: > > + ib_dereg_mr_user(mr, uverbs_get_cleared_udata(attrs)); > > This isn't how the error handling works with > uverbs_finalize_uobj_create() - you just return the error code and the caller > deals with destroying the fully initialized HW object properly. > Calling ib_dereg_mr_user() here will crash the kernel. > > Check the other uses for an example. >
Will fix. > Again I must ask what the plan is for testing as even a basic set of pyverbs > sanity tests would catch this. > > I've generally been insisting that all new uABI needs testing support in > rdma-core. This *might* be the exception but I want to hear a really > good reason why we can't have a test first... > So far I have been using a user space test that focuses on basic functionality and limited parameter checking (e.g. incorrect addr, length, flags). This specific error path happens to be untested. Will work more on the test front to increase the code coverage. > Jason _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
