Re: [PATCH] RDMA/rxe: refuse to destroy a QP with type 2 MWs still bound

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] RDMA/rxe: TOCTOU race in rxe_qp_chk_destroy() allows UAF
--

commit 94e08f1cc2290d06c7fcd154d031cde9d5a0054c
Author: Youngsung Ahn <ays511.kr@gmail.com>

    RDMA/rxe: refuse to destroy a QP with type 2 MWs still bound

This commit adds tracking for type 2 memory windows bound to a QP via an
atomic counter. It attempts to prevent the destruction of a QP if there are
outstanding type 2 MW bindings by returning -EBUSY.

> diff --git a/drivers/infiniband/sw/rxe/rxe_mw.c b/drivers/infiniband/sw/rxe/rxe_mw.c
> index bddb7a2578313..a2e3a6ce09463 100644
> --- a/drivers/infiniband/sw/rxe/rxe_mw.c
> +++ b/drivers/infiniband/sw/rxe/rxe_mw.c
[ ... ]
> @@ -161,6 +161,7 @@ static void rxe_do_bind_mw(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
>  
>  	if (mw->ibmw.type == IB_MW_TYPE_2) {
>  		rxe_get(qp);
> +		atomic_inc(&qp->mw_bind_num);
>  		mw->qp = qp;
>  	}
>  }

[Severity: Critical]
Is it possible for the worker task to execute this concurrently during QP
destruction?

Since qp->valid is not cleared until rxe_qp_do_cleanup() runs, the requester
can freely process pending BIND_MW work requests during the cleanup delay:

drivers/infiniband/sw/rxe/rxe_req.c:rxe_requester() {
    ...
    if (unlikely(!qp->valid)) {
    ...
}

If this runs concurrently, couldn't the counter be incremented too late,
holding the reference that triggers the 50-second timeout during cleanup?

> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
> index 311f285d78a6b..fea55bc3b0cb9 100644
> --- a/drivers/infiniband/sw/rxe/rxe_qp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c
[ ... ]
> @@ -858,6 +858,16 @@ int rxe_qp_chk_destroy(struct rxe_qp *qp)
>  		return -EBUSY;
>  	}
>  
> +	/* An attempt to destroy a QP while it still holds references for
> +	 * bound type 2 memory windows will fail immediately.  Otherwise the
> +	 * QP is freed with those references outstanding and the windows are
> +	 * left pointing at freed memory (use-after-free).
> +	 */
> +	if (atomic_read(&qp->mw_bind_num)) {
> +		rxe_dbg_qp(qp, "Attempt to destroy while type 2 MWs are bound\n");
> +		return -EBUSY;
> +	}
> +
>  	return 0;
>  }

[Severity: Critical]
Does this lockless check fully prevent the regression?

Because this check happens before the QP is drained or the worker task is
stopped, a racing userspace application could post an IB_WR_BIND_MW work
request that is processed immediately after this check passes.

When that happens, the delayed bind will cause rxe_cleanup() to hit its
50-second timeout waiting for the reference to drop. However, the timeout
error from rxe_cleanup() is ignored:

drivers/infiniband/sw/rxe/rxe_verbs.c:rxe_destroy_qp() {
    ...
    err = rxe_cleanup(qp);
    if (err)
        rxe_err_qp(qp, "cleanup failed, err = %d\n", err);

    return 0;
}

Since it unconditionally returns 0, won't the core still free the QP memory
while the memory window retains a dangling pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930182223.2511145-1-ays511.kr@gmail.com?part=1