Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
From: Leon Romanovsky <leon@kernel.org>
To: Jacob Moroni <jmoroni@google.com>
Cc: tatyana.e.nikolova@intel.com, jgg@ziepe.ca, linux-rdma@vger.kernel.org
Subject: Re: [PATCH] RDMA/core: Clear driver_udata before destroying objects in rdma_core
Date: Tue, 6 Oct 2026 16:37:39 +0300	[thread overview]
Message-ID: <20261006133739.GA7822@unreal> (raw)
In-Reply-To: <20261001162628.3187887-1-jmoroni@google.com>

On Thu, Oct 01, 2026 at 04:26:28PM +0000, Jacob Moroni wrote:
> When uobject creation fails while copying user output data (i.e.,
> after the HW object has been created), it calls
> rdma_alloc_abort_uobject() with hw_obj_valid=true which then
> invokes the object's destroy callback, but passes the
> uverbs_attr_bundle from the original creation command.
> 
> The issue is that drivers are expected to validate the udata
> and return an error if there's unexpected content, and data from
> the wrong command counts as "unexpected content", so this ends
> up causing the driver's destroy call to fail.
> 
> A similar issue exists in the rereg_mr path when a new MR
> is created and the old one is destroyed.
> 
> Fix this by clearing driver_udata before invoking the driver's
> destroy method.
> 
> Fixes: 6e0954b11c05 ("RDMA/uverbs: Allow drivers to create a new HW object during rereg_mr")
> Fixes: 0ac8903cbbe6 ("RDMA/core: Allow the ioctl layer to abort a fully created uobject")
> Signed-off-by: Jacob Moroni <jmoroni@google.com>
> ---
>  drivers/infiniband/core/ib_core_uverbs.c | 12 ------------
>  drivers/infiniband/core/rdma_core.c      |  2 ++
>  drivers/infiniband/core/rdma_core.h      | 17 +++++++++++------
>  3 files changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/infiniband/core/ib_core_uverbs.c b/drivers/infiniband/core/ib_core_uverbs.c
> index 41c84ffe8c09..1482d9b27b38 100644
> --- a/drivers/infiniband/core/ib_core_uverbs.c
> +++ b/drivers/infiniband/core/ib_core_uverbs.c
> @@ -535,18 +535,6 @@ int uverbs_destroy_def_handler(struct uverbs_attr_bundle *attrs)
>  }
>  EXPORT_SYMBOL(uverbs_destroy_def_handler);
>  
> -/*
> - * When calling a destroy function during an error unwind we need to pass in
> - * the udata that is sanitized of all user arguments. Ie from the driver
> - * perspective it looks like no udata was passed.
> - */
> -struct ib_udata *uverbs_get_cleared_udata(struct uverbs_attr_bundle *attrs)
> -{
> -	attrs->driver_udata = (struct ib_udata){};
> -	return &attrs->driver_udata;
> -}
> -EXPORT_SYMBOL_NS_GPL(uverbs_get_cleared_udata, "rdma_core");
> -
>  /**
>   * _uverbs_alloc() - Quickly allocate memory for use with a bundle
>   * @bundle: The bundle
> diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
> index fd5651c003ae..fc727dbbb897 100644
> --- a/drivers/infiniband/core/rdma_core.c
> +++ b/drivers/infiniband/core/rdma_core.c
> @@ -735,6 +735,7 @@ void rdma_assign_uobject(struct ib_uobject *to_uobj, struct ib_uobject *new_uobj
>  	 * If this fails then the uobject is still completely valid (though with
>  	 * a new ID) and we leak it until context close.
>  	 */
> +	uverbs_get_cleared_udata(attrs);
>  	uverbs_destroy_uobject(to_uobj, RDMA_REMOVE_DESTROY, attrs);
>  }
>  EXPORT_SYMBOL_NS_GPL(rdma_assign_uobject, "rdma_core");
> @@ -751,6 +752,7 @@ void rdma_alloc_abort_uobject(struct ib_uobject *uobj,
>  	int ret;
>  
>  	if (hw_obj_valid) {
> +		uverbs_get_cleared_udata(attrs);
>  		ret = uobj->uapi_object->type_class->destroy_hw(
>  			uobj, RDMA_REMOVE_ABORT, attrs);
>  		/*
> diff --git a/drivers/infiniband/core/rdma_core.h b/drivers/infiniband/core/rdma_core.h
> index 2b91e8527287..9de14e625dd3 100644
> --- a/drivers/infiniband/core/rdma_core.h
> +++ b/drivers/infiniband/core/rdma_core.h
> @@ -71,14 +71,19 @@ int uverbs_output_written(const struct uverbs_attr_bundle *bundle, size_t idx);
>  
>  void setup_ufile_idr_uobject(struct ib_uverbs_file *ufile);
>  
> -#if IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS)
> -struct ib_udata *uverbs_get_cleared_udata(struct uverbs_attr_bundle *attrs);
> -#else
> -static inline struct ib_udata *uverbs_get_cleared_udata(struct uverbs_attr_bundle *attrs)
> +/*
> + * When calling a destroy function during an error unwind we need to pass in
> + * the udata that is sanitized of all user arguments. Ie from the driver
> + * perspective it looks like no udata was passed.
> + */
> +static inline struct ib_udata *
> +uverbs_get_cleared_udata(struct uverbs_attr_bundle *attrs)
>  {
> -	return NULL;
> +	if (!attrs)
> +		return NULL;

I would expect changes in create_qp() too after writing this function
like you wrote.

Thanks

> +	attrs->driver_udata = (struct ib_udata){};
> +	return &attrs->driver_udata;
>  }
> -#endif
>  
>  /*
>   * This is the runtime description of the uverbs API, used by the syscall
> -- 
> 2.56.0.rc1.315.gc6ed9934b7-goog
> 

      parent reply	other threads:[~2026-10-06 13:37 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-01 16:26 [PATCH] RDMA/core: Clear driver_udata before destroying objects in rdma_core Jacob Moroni
2026-10-01 16:37 ` sashiko-bot
2026-10-06 13:37 ` Leon Romanovsky [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261006133739.GA7822@unreal \
    --to=leon@kernel.org \
    --cc=jgg@ziepe.ca \
    --cc=jmoroni@google.com \
    --cc=linux-rdma@vger.kernel.org \
    --cc=tatyana.e.nikolova@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox