Re: [PATCH 09/12] rdma: use __anon_inode_getfd

Jason Gunthorpe
2020-05-08T19:53:02Z

On Fri, May 08, 2020 at 05:36:31PM +0200, Christoph Hellwig wrote:
> Use __anon_inode_getfd instead of opencoding the logic using
> get_unused_fd_flags + anon_inode_getfile.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/infiniband/core/rdma_core.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)

> diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
> index 5128cb16bb485..541e5e06347f6 100644
> --- a/drivers/infiniband/core/rdma_core.c
> +++ b/drivers/infiniband/core/rdma_core.c
> @@ -462,30 +462,21 @@ alloc_begin_fd_uobject(const struct uverbs_api_object *obj,
> if (WARN_ON(fd_type->fops->release != &uverbs_uobject_fd_release))
> return ERR_PTR(-EINVAL);
>
> - new_fd = get_unused_fd_flags(O_CLOEXEC);
> - if (new_fd < 0)
> - return ERR_PTR(new_fd);
> -
> uobj = alloc_uobj(attrs, obj);
> if (IS_ERR(uobj))
> - goto err_fd;
> + return uobj;
>
> /* Note that uverbs_uobject_fd_release() is called during abort */
> - filp = anon_inode_getfile(fd_type->name, fd_type->fops, NULL,
> - fd_type->flags);
> - if (IS_ERR(filp)) {
> - uobj = ERR_CAST(filp);
> + new_fd = __anon_inode_getfd(fd_type->name, fd_type->fops, NULL,
> + fd_type->flags | O_CLOEXEC, &filp);
> + if (new_fd < 0)
> goto err_uobj;

This will conflict with a fix (83a267021221 'RDMA/core: Fix
overwriting of uobj in case of error') that is going to go to -rc
soon.

Also the above misses returning an ERR_PTR if __anon_inode_getfd fails, it
returns a uobj that had been freed.. I suppose it should be something
like

if (new_fd < 0) {
uverbs_uobject_put(uobj);
return ERR_PTR(new_fd)
}

?

Jason