[PATCH rdma-rc 1/2] RDMA/ucma: Lock the handler in ucma_write_cm_event()

[PATCH rdma-rc 1/2] RDMA/ucma: Lock the handler in ucma_write_cm_event()

[Norbert Szetei]

ctx->file may only be changed under the handler lock and the xa_lock, which
is what stops uevents being queued for a ctx while ucma_migrate_id() moves
it to another file.  The CM core takes that lock before invoking
ucma_event_handler(), but the write() paths that queue uevents themselves
do not.

ucma_write_cm_event() re-reads ctx->file for each of its four dereferences,
so ucma_migrate_id() can swap it mid-sequence:

	mutex_lock(&ctx->file->mut);			/* file A */
	list_add_tail(&uevent->list, &ctx->file->event_list);	/* file B */
	mutex_unlock(&ctx->file->mut);			/* file B */
	wake_up_interruptible(&ctx->file->poll_wait);	/* file B */

The window is the mutex_lock() itself: the writer sleeps in it while the
migration reassigns ctx->file.  The list_add_tail() then runs on file B's
event_list holding only file A's mutex:

  list_add corruption. prev->next should be next (ffff888101320f30),
    but was ffff88814a08c418. (prev=ffff88814a075c18).
  kernel BUG at lib/list_debug.c:32!
  Call Trace:
   ucma_write_cm_event+0x36e/0x5e0

and file A's mut is left held forever, wedging its next writer in D state.
The uevent is also stranded on a list ucma_cleanup_ctx_events() will not
walk, so it outlives its context.  /dev/infiniband/rdma_cm is 0666 and no
RDMA device is involved, so an unprivileged user reaches all of this.

Take the handler lock, as ucma_cleanup_mc_events() does; ctx->cm_id is
pinned by the ucma_get_ctx() reference.

Fixes: a3c9d0fcd371 ("RDMA/ucma: Support write an event into a CM")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Norbert Szetei <norbert@doyensec.com>
---
 drivers/infiniband/core/ucma.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index 878561fa1cb5..ba4dfa7f12de 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -1784,10 +1784,12 @@ static ssize_t ucma_write_cm_event(struct ucma_file *file,
 	memcpy(uevent->resp.param.arg32, &event.param.arg,
 	       sizeof(event.param.arg));
 
+	rdma_lock_handler(ctx->cm_id);
 	mutex_lock(&ctx->file->mut);
 	list_add_tail(&uevent->list, &ctx->file->event_list);
 	mutex_unlock(&ctx->file->mut);
 	wake_up_interruptible(&ctx->file->poll_wait);
+	rdma_unlock_handler(ctx->cm_id);
 
 out:
 	ucma_put_ctx(ctx);
-- 
2.55.0

[PATCH rdma-rc 2/2] RDMA/ucma: Lock the handler in ucma_set_ib_path()

[Norbert Szetei]

ucma_set_ib_path() calls ucma_event_handler() straight from the write()
path, without the handler lock that keeps ctx->file stable while a uevent
is queued.  The handler re-reads ctx->file for every dereference:

	mutex_lock(&ctx->file->mut);			/* file A */
	list_add_tail(&uevent->list, &ctx->file->event_list);	/* file B */
	mutex_unlock(&ctx->file->mut);			/* file B */
	wake_up_interruptible(&ctx->file->poll_wait);	/* file B */

A concurrent ucma_migrate_id() reassigns ctx->file while the SET_OPTION
caller sleeps in mutex_lock(), so the list_add_tail() lands on file B's
event_list while only file A's mutex is held, racing every other user of
that list:

  BUG: KASAN: slab-use-after-free in __list_add_valid_or_report+0x1aa/0x1c0
  Read of size 8 at addr ffff888153c6a418 by task poc_corr/486
  Call Trace:
   __list_add_valid_or_report+0x1aa/0x1c0
   ucma_event_handler+0x1be/0xc00
   ucma_set_ib_path+0x45e/0x710
   ucma_set_option+0x32e/0x590
   ucma_write+0x1f9/0x330
  Allocated by task 505:
   ucma_write_cm_event+0x1a1/0x660
  Freed by task 505:
   kfree+0x1da/0x4c0
   ucma_get_event+0x5d5/0x7e0

The freed object is a ucma_event that another thread dequeued from file B's
list under file B's mutex.  File A's mut is left held on top of that,
wedging its next writer in uninterruptible sleep.

This path needs a bound and address-resolved cm_id, so it requires an RDMA
device to be present.

Take the handler lock around the call.

Fixes: f5449e74802c ("RDMA/ucma: Rework ucma_migrate_id() to avoid races with destroy")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Norbert Szetei <norbert@doyensec.com>
---
 drivers/infiniband/core/ucma.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index ba4dfa7f12de..2347c0c59b90 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -1404,7 +1404,10 @@ static int ucma_set_ib_path(struct ucma_context *ctx,
 
 	memset(&event, 0, sizeof event);
 	event.event = RDMA_CM_EVENT_ROUTE_RESOLVED;
-	return ucma_event_handler(ctx->cm_id, &event);
+	rdma_lock_handler(ctx->cm_id);
+	ret = ucma_event_handler(ctx->cm_id, &event);
+	rdma_unlock_handler(ctx->cm_id);
+	return ret;
 }
 
 static int ucma_set_option_ib(struct ucma_context *ctx, int optname,
-- 
2.55.0

Re: [PATCH rdma-rc 2/2] RDMA/ucma: Lock the handler in ucma_set_ib_path()

[Jason Gunthorpe]

On Mon, Jul 27, 2026 at 10:08:36AM +0200, Norbert Szetei wrote:

> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index ba4dfa7f12de..2347c0c59b90 100644
> --- a/drivers/infiniband/core/ucma.c
> +++ b/drivers/infiniband/core/ucma.c
> @@ -1404,7 +1404,10 @@ static int ucma_set_ib_path(struct ucma_context *ctx,
>  
>  	memset(&event, 0, sizeof event);
>  	event.event = RDMA_CM_EVENT_ROUTE_RESOLVED;
> -	return ucma_event_handler(ctx->cm_id, &event);
> +	rdma_lock_handler(ctx->cm_id);
> +	ret = ucma_event_handler(ctx->cm_id, &event);
> +	rdma_unlock_handler(ctx->cm_id);
> +	return ret;
>  }

Applied, something went off with patchworks and I didn't notice it

Thanks,
Jason

Re: [PATCH rdma-rc 1/2] RDMA/ucma: Lock the handler in ucma_write_cm_event()

[Jason Gunthorpe]

On Mon, Jul 27, 2026 at 10:06:12AM +0200, Norbert Szetei wrote:
> The window is the mutex_lock() itself: the writer sleeps in it while the
> migration reassigns ctx->file.  The list_add_tail() then runs on file B's
> event_list holding only file A's mutex:
> 
>   list_add corruption. prev->next should be next (ffff888101320f30),
>     but was ffff88814a08c418. (prev=ffff88814a075c18).
>   kernel BUG at lib/list_debug.c:32!
>   Call Trace:
>    ucma_write_cm_event+0x36e/0x5e0
> 
> and file A's mut is left held forever, wedging its next writer in D state.
> The uevent is also stranded on a list ucma_cleanup_ctx_events() will not
> walk, so it outlives its context.  /dev/infiniband/rdma_cm is 0666 and no
> RDMA device is involved, so an unprivileged user reaches all of this.
> 
> Take the handler lock, as ucma_cleanup_mc_events() does; ctx->cm_id is
> pinned by the ucma_get_ctx() reference.
> 
> Fixes: a3c9d0fcd371 ("RDMA/ucma: Support write an event into a CM")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Norbert Szetei <norbert@doyensec.com>
> ---
>  drivers/infiniband/core/ucma.c | 2 ++
>  1 file changed, 2 insertions(+)

There was another one of this mistake too, I'll send a patch

applied to for-next

Thanks,
Jason

Re: [PATCH rdma-rc 1/2] RDMA/ucma: Lock the handler in ucma_write_cm_event()

[Jason Gunthorpe]

On Tue, Aug 11, 2026 at 01:54:25PM -0300, Jason Gunthorpe wrote:
> On Mon, Jul 27, 2026 at 10:06:12AM +0200, Norbert Szetei wrote:
> > The window is the mutex_lock() itself: the writer sleeps in it while the
> > migration reassigns ctx->file.  The list_add_tail() then runs on file B's
> > event_list holding only file A's mutex:
> > 
> >   list_add corruption. prev->next should be next (ffff888101320f30),
> >     but was ffff88814a08c418. (prev=ffff88814a075c18).
> >   kernel BUG at lib/list_debug.c:32!
> >   Call Trace:
> >    ucma_write_cm_event+0x36e/0x5e0
> > 
> > and file A's mut is left held forever, wedging its next writer in D state.
> > The uevent is also stranded on a list ucma_cleanup_ctx_events() will not
> > walk, so it outlives its context.  /dev/infiniband/rdma_cm is 0666 and no
> > RDMA device is involved, so an unprivileged user reaches all of this.
> > 
> > Take the handler lock, as ucma_cleanup_mc_events() does; ctx->cm_id is
> > pinned by the ucma_get_ctx() reference.
> > 
> > Fixes: a3c9d0fcd371 ("RDMA/ucma: Support write an event into a CM")
> > Cc: stable@vger.kernel.org
> > Assisted-by: Claude:claude-opus-5
> > Signed-off-by: Norbert Szetei <norbert@doyensec.com>
> > ---
> >  drivers/infiniband/core/ucma.c | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> There was another one of this mistake too, I'll send a patch
> 
> applied to for-next

Actually this is not quite right either, ctx->uid is also protected by
the handler lock and pedantically should be checked for zero like the
normal event delivery path.

I will squish this in:

@@ -1779,6 +1779,13 @@ static ssize_t ucma_write_cm_event(struct ucma_file *file,
                goto out;
        }
 
+       rdma_lock_handler(ctx->cm_id);
+       if (!ctx->uid) {
+               kfree(uevent);
+               ret = -EINVAL;
+               goto err_unlock;
+       }
+
        uevent->ctx = ctx;
        uevent->resp.uid = ctx->uid;
        uevent->resp.id = ctx->id;
@@ -1787,13 +1794,13 @@ static ssize_t ucma_write_cm_event(struct ucma_file *file,
        memcpy(uevent->resp.param.arg32, &event.param.arg,
               sizeof(event.param.arg));
 
-       rdma_lock_handler(ctx->cm_id);
        mutex_lock(&ctx->file->mut);
        list_add_tail(&uevent->list, &ctx->file->event_list);
        mutex_unlock(&ctx->file->mut);
        wake_up_interruptible(&ctx->file->poll_wait);
-       rdma_unlock_handler(ctx->cm_id);
 
+err_unlock:
+       rdma_unlock_handler(ctx->cm_id);
 out:
        ucma_put_ctx(ctx);
        return ret;

Jason