Re: [PATCH v10 3/6] RDMA/uverbs: Add uverbs command for dma-buf based MR registration

[Jason Gunthorpe <jgg@ziepe.ca>]

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.

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...

Jason