[PATCH] RDMA/rxe: validate access flags before swapping the MR's PD

[PATCH] RDMA/rxe: validate access flags before swapping the MR's PD

[Norbert Szetei]

rxe_rereg_user_mr() reassigns mr->ibmr.pd first and only then
validates the IB_MR_REREG_ACCESS argument:

	if (flags & IB_MR_REREG_PD) {
		rxe_put(old_pd);
		rxe_get(pd);
		mr->ibmr.pd = ibpd;
	}

	if (flags & IB_MR_REREG_ACCESS) {
		if (access & ~RXE_ACCESS_SUPPORTED_MR)
			return ERR_PTR(-EOPNOTSUPP);
		mr->access = access;
	}

Both flags pass the entry check because RXE_MR_REREG_SUPPORTED is
IB_MR_REREG_PD | IB_MR_REREG_ACCESS, so a caller can reach the access
check with mr->ibmr.pd already reassigned.

mr->ibmr.pd is owned by the core, which adjusts pd->usecnt only on the
success path: ib_uverbs_rereg_mr() jumps to put_new_uobj on a driver error
without undoing the reassignment, so mr->pd == new_pd while the usecnts
still charge the MR to orig_pd. ib_dereg_mr_user() then decrements
new_pd, whose count can reach zero while a memory window still references
it; uverbs_free_pd() frees the PD on that count alone and rxe_mw_cleanup()
writes to freed memory:

  BUG: KASAN: slab-use-after-free in __rxe_put+0x31/0xa0
  Write of size 4 at addr ffff8881301dd690 by task rxe_poc/591
   __rxe_put+0x31/0xa0
   rxe_mw_cleanup+0x42/0x200
   __rxe_cleanup+0x115/0x370
   rxe_dealloc_mw+0x4c/0x80
  Allocated by task 591:
   ib_uverbs_alloc_pd+0x258/0x540
  Freed by task 591:
   ib_dealloc_pd_user+0x174/0x210
   uverbs_free_pd+0x8d/0xc0
   ib_uverbs_dealloc_pd+0x18e/0x1d0

Validate the access flags before mutating any state so the callback either
applies every requested change or none.

Fixes: 544c7f62cf32 ("RDMA/rxe: Implement rereg_user_mr")
Cc: stable@vger.kernel.org
Signed-off-by: Norbert Szetei <norbert@doyensec.com>
---
 drivers/infiniband/sw/rxe/rxe_verbs.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index 96c7716057fe..3864284522eb 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -1331,19 +1331,20 @@ static struct ib_mr *rxe_rereg_user_mr(struct ib_mr *ibmr, int flags,
 	if (err)
 		return ERR_PTR(err);
 
+	if ((flags & IB_MR_REREG_ACCESS) &&
+	    (access & ~RXE_ACCESS_SUPPORTED_MR)) {
+		rxe_err_mr(mr, "access = %#x not supported\n", access);
+		return ERR_PTR(-EOPNOTSUPP);
+	}
+
 	if (flags & IB_MR_REREG_PD) {
 		rxe_put(old_pd);
 		rxe_get(pd);
 		mr->ibmr.pd = ibpd;
 	}
 
-	if (flags & IB_MR_REREG_ACCESS) {
-		if (access & ~RXE_ACCESS_SUPPORTED_MR) {
-			rxe_err_mr(mr, "access = %#x not supported\n", access);
-			return ERR_PTR(-EOPNOTSUPP);
-		}
+	if (flags & IB_MR_REREG_ACCESS)
 		mr->access = access;
-	}
 
 	return NULL;
 }
-- 
2.55.0

Re: [PATCH] RDMA/rxe: validate access flags before swapping the MR's PD

[Zhu Yanjun]

在 2026/8/27 10:18, Norbert Szetei 写道:
> rxe_rereg_user_mr() reassigns mr->ibmr.pd first and only then
> validates the IB_MR_REREG_ACCESS argument:
> 
> 	if (flags & IB_MR_REREG_PD) {
> 		rxe_put(old_pd);
> 		rxe_get(pd);
> 		mr->ibmr.pd = ibpd;
> 	}
> 
> 	if (flags & IB_MR_REREG_ACCESS) {
> 		if (access & ~RXE_ACCESS_SUPPORTED_MR)
> 			return ERR_PTR(-EOPNOTSUPP);
> 		mr->access = access;
> 	}
> 
> Both flags pass the entry check because RXE_MR_REREG_SUPPORTED is
> IB_MR_REREG_PD | IB_MR_REREG_ACCESS, so a caller can reach the access
> check with mr->ibmr.pd already reassigned.
> 
> mr->ibmr.pd is owned by the core, which adjusts pd->usecnt only on the
> success path: ib_uverbs_rereg_mr() jumps to put_new_uobj on a driver error
> without undoing the reassignment, so mr->pd == new_pd while the usecnts
> still charge the MR to orig_pd. ib_dereg_mr_user() then decrements
> new_pd, whose count can reach zero while a memory window still references
> it; uverbs_free_pd() frees the PD on that count alone and rxe_mw_cleanup()
> writes to freed memory:
> 
>    BUG: KASAN: slab-use-after-free in __rxe_put+0x31/0xa0
>    Write of size 4 at addr ffff8881301dd690 by task rxe_poc/591
>     __rxe_put+0x31/0xa0
>     rxe_mw_cleanup+0x42/0x200
>     __rxe_cleanup+0x115/0x370
>     rxe_dealloc_mw+0x4c/0x80
>    Allocated by task 591:
>     ib_uverbs_alloc_pd+0x258/0x540
>    Freed by task 591:
>     ib_dealloc_pd_user+0x174/0x210
>     uverbs_free_pd+0x8d/0xc0
>     ib_uverbs_dealloc_pd+0x18e/0x1d0
> 
> Validate the access flags before mutating any state so the callback either
> applies every requested change or none.
> 
> Fixes: 544c7f62cf32 ("RDMA/rxe: Implement rereg_user_mr")> Cc: stable@vger.kernel.org
> Signed-off-by: Norbert Szetei <norbert@doyensec.com>

Thanks a lot. I am fine with this commit.

Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev>

Zhu Yanjun

> ---
>   drivers/infiniband/sw/rxe/rxe_verbs.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> index 96c7716057fe..3864284522eb 100644
> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> @@ -1331,19 +1331,20 @@ static struct ib_mr *rxe_rereg_user_mr(struct ib_mr *ibmr, int flags,
>   	if (err)
>   		return ERR_PTR(err);
>   
> +	if ((flags & IB_MR_REREG_ACCESS) &&
> +	    (access & ~RXE_ACCESS_SUPPORTED_MR)) {
> +		rxe_err_mr(mr, "access = %#x not supported\n", access);
> +		return ERR_PTR(-EOPNOTSUPP);
> +	}
> +
>   	if (flags & IB_MR_REREG_PD) {
>   		rxe_put(old_pd);
>   		rxe_get(pd);
>   		mr->ibmr.pd = ibpd;
>   	}
>   
> -	if (flags & IB_MR_REREG_ACCESS) {
> -		if (access & ~RXE_ACCESS_SUPPORTED_MR) {
> -			rxe_err_mr(mr, "access = %#x not supported\n", access);
> -			return ERR_PTR(-EOPNOTSUPP);
> -		}
> +	if (flags & IB_MR_REREG_ACCESS)
>   		mr->access = access;
> -	}
>   
>   	return NULL;
>   }