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@xxxxxxxxxxxxxxx
> Signed-off-by: Norbert Szetei <norbert@xxxxxxxxxxxx>

Thanks a lot. I am fine with this commit.

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

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;
>   }