The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] RDMA/rxe: validate num_sge/cur_sge before indexing wqe->dma.sge[]
@ 2026-07-08 22:45 Ibrahim Hashimov
  2026-07-09  2:38 ` yanjun.zhu
  2026-07-09  7:26 ` [PATCH v2] " Ibrahim Hashimov
  0 siblings, 2 replies; 5+ messages in thread
From: Ibrahim Hashimov @ 2026-07-08 22:45 UTC (permalink / raw)
  To: Zhu Yanjun, Jason Gunthorpe, Leon Romanovsky
  Cc: linux-rdma, linux-kernel, stable

A user QP's send queue (qp->sq.queue) is a shared ring the userspace
application writes to directly via mmap (vmalloc_user(), see
rxe_queue.c). For such a QP, rxe_post_send() takes the qp->is_user
branch and only schedules the requester task -- it never validates or
copies the posted WQE:

	drivers/infiniband/sw/rxe/rxe_verbs.c:
		if (qp->is_user) {
			rxe_sched_task(&qp->send_task);
			...
		}

The requester then consumes the WQE in place straight out of that
mmap'd ring:

	rxe_req.c:	wqe = req_next_wqe(qp);
	rxe_req.c:	err = copy_data(qp->pd, 0, &wqe->dma,
					payload_addr(pkt), payload,
					RXE_FROM_MR_OBJ);

copy_data() immediately indexes the per-WQE sge array with the
attacker-controlled cur_sge field, before any bound is checked:

	rxe_mr.c:	struct rxe_sge *sge = &dma->sge[dma->cur_sge];
	rxe_mr.c:	...
	rxe_mr.c:	if (sge->length && (offset < sge->length)) {

dma->sge[] is a flex array whose real backing storage is exactly
qp->sq.max_sge entries per WQE slot (see rxe_qp.c, wqe_size computed
from max_sge at QP create time). Since a user QP's WQE bytes are
entirely attacker-supplied, both wqe->dma.num_sge and wqe->dma.cur_sge
can be set to arbitrary values independent of each other and of
max_sge. Only the *kernel*-QP post path bounds num_sge:

	rxe_verbs.c: validate_send_wr()
		if (num_sge > sq->max_sge) {
			rxe_err_qp(qp, "num_sge > max_sge\n");

but that function is only reachable from rxe_post_one_send() for
kernel-owned QPs; it is never consulted for a user QP's raw WQE.

The sibling receive path already has the equivalent guard, with the
literal comment documenting exactly why it is required:

	rxe_resp.c: get_srq_wqe()
		/* don't trust user space data */
		if (unlikely(wqe->dma.num_sge > srq->rq.max_sge)) {
			...
			rxe_dbg_qp(qp, "invalid num_sge in SRQ entry\n");
			return RESPST_ERR_MALFORMED_WQE;
		}

The send/requester path has no analogous check, so a local,
unprivileged user who can open /dev/infiniband/uverbs* and create a
user QP on a soft-RoCE (rxe) link can hand-craft a WQE in the shared
send queue with an out-of-range wqe->dma.cur_sge (or an oversized
wqe->dma.num_sge) and ring the send doorbell. rxe_requester() then
calls copy_data(), which dereferences &dma->sge[cur_sge] out of the
bounds of the per-WQE sge array -- a vmalloc out-of-bounds *read*
(confirmed via KASAN: "KASAN: vmalloc-out-of-bounds in copy_data"),
reliably panicking the kernel (local DoS). sge->addr itself is still
bounds-checked later by lookup_mr()/rxe_mr_copy(), so the primitive is
an OOB read of sge metadata, not an arbitrary read/write primitive.

Fix this the same way get_srq_wqe() already does for SRQ entries:
bound both fields pulled from the (possibly user-mapped) send queue
entry before they are ever used to index wqe->dma.sge[], right where
the requester fetches the next WQE off the ring in rxe_requester().
num_sge is capped at qp->sq.max_sge (matching the sibling SRQ check
and the kernel-QP validate_send_wr() check), and cur_sge is capped at
qp->sq.max_sge directly, since that is the true per-WQE array capacity
that copy_data() indexes into -- this also covers num_sge == 0 /
cur_sge == 0 local-op and zero-payload WQEs, which remain valid.

This is a long-standing bug in the rxe (soft-RoCE) driver: the
qp->is_user bypass in rxe_post_send() and the unbounded
&dma->sge[dma->cur_sge] indexing in copy_data() have been present
since the driver was introduced.

Runtime-verified on a v6.19 KASAN (CONFIG_KASAN_VMALLOC=y) stand: a
reproducer that posts a user QP send WQE with an out-of-range
cur_sge reliably tripped "KASAN: vmalloc-out-of-bounds in
copy_data" (an out-of-bounds read) before this patch, and no longer
triggers that report with the patch applied.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
 drivers/infiniband/sw/rxe/rxe_req.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 12d03f390b09..9fb2c49fb503 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -701,6 +701,22 @@ int rxe_requester(struct rxe_qp *qp)
 	if (unlikely(!wqe))
 		goto exit;
 
+	/*
+	 * Don't trust user space data: qp->sq.queue is a raw ring the
+	 * application writes directly for a user QP, so wqe->dma.num_sge
+	 * and wqe->dma.cur_sge must be bounds-checked the same way
+	 * get_srq_wqe() checks an SRQ entry's num_sge before it is used.
+	 * Otherwise copy_data() indexes wqe->dma.sge[wqe->dma.cur_sge]
+	 * with an unvalidated, attacker-controlled index/count and reads
+	 * out of bounds of the per-wqe sge array.
+	 */
+	if (unlikely(wqe->dma.num_sge > qp->sq.max_sge ||
+		     wqe->dma.cur_sge >= qp->sq.max_sge)) {
+		rxe_dbg_qp(qp, "invalid num_sge/cur_sge in send wqe\n");
+		wqe->status = IB_WC_LOC_QP_OP_ERR;
+		goto err;
+	}
+
 	if (rxe_wqe_is_fenced(qp, wqe)) {
 		qp->req.wait_fence = 1;
 		goto exit;
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] RDMA/rxe: validate num_sge/cur_sge before indexing wqe->dma.sge[]
  2026-07-08 22:45 [PATCH] RDMA/rxe: validate num_sge/cur_sge before indexing wqe->dma.sge[] Ibrahim Hashimov
@ 2026-07-09  2:38 ` yanjun.zhu
  2026-07-09  7:26   ` Ibrahim Hashimov
  2026-07-09  7:26 ` [PATCH v2] " Ibrahim Hashimov
  1 sibling, 1 reply; 5+ messages in thread
From: yanjun.zhu @ 2026-07-09  2:38 UTC (permalink / raw)
  To: Ibrahim Hashimov, Zhu Yanjun, Jason Gunthorpe, Leon Romanovsky,
	Zhu Yanjun
  Cc: linux-rdma, linux-kernel, stable

On 7/8/26 3:45 PM, Ibrahim Hashimov wrote:
> A user QP's send queue (qp->sq.queue) is a shared ring the userspace
> application writes to directly via mmap (vmalloc_user(), see
> rxe_queue.c). For such a QP, rxe_post_send() takes the qp->is_user
> branch and only schedules the requester task -- it never validates or
> copies the posted WQE:
> 
> 	drivers/infiniband/sw/rxe/rxe_verbs.c:
> 		if (qp->is_user) {
> 			rxe_sched_task(&qp->send_task);
> 			...
> 		}
> 
> The requester then consumes the WQE in place straight out of that
> mmap'd ring:
> 
> 	rxe_req.c:	wqe = req_next_wqe(qp);
> 	rxe_req.c:	err = copy_data(qp->pd, 0, &wqe->dma,
> 					payload_addr(pkt), payload,
> 					RXE_FROM_MR_OBJ);
> 
> copy_data() immediately indexes the per-WQE sge array with the
> attacker-controlled cur_sge field, before any bound is checked:
> 
> 	rxe_mr.c:	struct rxe_sge *sge = &dma->sge[dma->cur_sge];
> 	rxe_mr.c:	...
> 	rxe_mr.c:	if (sge->length && (offset < sge->length)) {
> 
> dma->sge[] is a flex array whose real backing storage is exactly
> qp->sq.max_sge entries per WQE slot (see rxe_qp.c, wqe_size computed
> from max_sge at QP create time). Since a user QP's WQE bytes are
> entirely attacker-supplied, both wqe->dma.num_sge and wqe->dma.cur_sge
> can be set to arbitrary values independent of each other and of
> max_sge. Only the *kernel*-QP post path bounds num_sge:
> 
> 	rxe_verbs.c: validate_send_wr()
> 		if (num_sge > sq->max_sge) {
> 			rxe_err_qp(qp, "num_sge > max_sge\n");
> 
> but that function is only reachable from rxe_post_one_send() for
> kernel-owned QPs; it is never consulted for a user QP's raw WQE.
> 
> The sibling receive path already has the equivalent guard, with the
> literal comment documenting exactly why it is required:
> 
> 	rxe_resp.c: get_srq_wqe()
> 		/* don't trust user space data */
> 		if (unlikely(wqe->dma.num_sge > srq->rq.max_sge)) {
> 			...
> 			rxe_dbg_qp(qp, "invalid num_sge in SRQ entry\n");
> 			return RESPST_ERR_MALFORMED_WQE;
> 		}
> 
> The send/requester path has no analogous check, so a local,
> unprivileged user who can open /dev/infiniband/uverbs* and create a
> user QP on a soft-RoCE (rxe) link can hand-craft a WQE in the shared
> send queue with an out-of-range wqe->dma.cur_sge (or an oversized
> wqe->dma.num_sge) and ring the send doorbell. rxe_requester() then
> calls copy_data(), which dereferences &dma->sge[cur_sge] out of the
> bounds of the per-WQE sge array -- a vmalloc out-of-bounds *read*
> (confirmed via KASAN: "KASAN: vmalloc-out-of-bounds in copy_data"),
> reliably panicking the kernel (local DoS). sge->addr itself is still
> bounds-checked later by lookup_mr()/rxe_mr_copy(), so the primitive is
> an OOB read of sge metadata, not an arbitrary read/write primitive.
> 
> Fix this the same way get_srq_wqe() already does for SRQ entries:
> bound both fields pulled from the (possibly user-mapped) send queue
> entry before they are ever used to index wqe->dma.sge[], right where
> the requester fetches the next WQE off the ring in rxe_requester().
> num_sge is capped at qp->sq.max_sge (matching the sibling SRQ check
> and the kernel-QP validate_send_wr() check), and cur_sge is capped at
> qp->sq.max_sge directly, since that is the true per-WQE array capacity
> that copy_data() indexes into -- this also covers num_sge == 0 /
> cur_sge == 0 local-op and zero-payload WQEs, which remain valid.
> 
> This is a long-standing bug in the rxe (soft-RoCE) driver: the
> qp->is_user bypass in rxe_post_send() and the unbounded
> &dma->sge[dma->cur_sge] indexing in copy_data() have been present
> since the driver was introduced.
> 
> Runtime-verified on a v6.19 KASAN (CONFIG_KASAN_VMALLOC=y) stand: a
> reproducer that posts a user QP send WQE with an out-of-range
> cur_sge reliably tripped "KASAN: vmalloc-out-of-bounds in
> copy_data" (an out-of-bounds read) before this patch, and no longer
> triggers that report with the patch applied.
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
> Assisted-by: AuditCode-AI:2026.07
> ---
>   drivers/infiniband/sw/rxe/rxe_req.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index 12d03f390b09..9fb2c49fb503 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c
> @@ -701,6 +701,22 @@ int rxe_requester(struct rxe_qp *qp)
>   	if (unlikely(!wqe))
>   		goto exit;
>   
> +	/*
> +	 * Don't trust user space data: qp->sq.queue is a raw ring the
> +	 * application writes directly for a user QP, so wqe->dma.num_sge
> +	 * and wqe->dma.cur_sge must be bounds-checked the same way
> +	 * get_srq_wqe() checks an SRQ entry's num_sge before it is used.
> +	 * Otherwise copy_data() indexes wqe->dma.sge[wqe->dma.cur_sge]
> +	 * with an unvalidated, attacker-controlled index/count and reads
> +	 * out of bounds of the per-wqe sge array.
> +	 */
> +	if (unlikely(wqe->dma.num_sge > qp->sq.max_sge ||

 From this function,

static int rxe_init_sq(struct rxe_qp *qp, struct ibv_qp_init_attr 
*init_attr)
{
     ...
     qp->sq.max_sge = init_attr->cap.max_send_sge;
     ...
}

qp->sq.max_sge is also from the user space application. It is possible 
that qp->sq.max_sge is 0.

Then this makes rxe_requester will return error and exit.

Zhu Yanjun

> +		     wqe->dma.cur_sge >= qp->sq.max_sge)) {
> +		rxe_dbg_qp(qp, "invalid num_sge/cur_sge in send wqe\n");
> +		wqe->status = IB_WC_LOC_QP_OP_ERR;
> +		goto err;
> +	}
> +
>   	if (rxe_wqe_is_fenced(qp, wqe)) {
>   		qp->req.wait_fence = 1;
>   		goto exit;


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] RDMA/rxe: validate num_sge/cur_sge before indexing wqe->dma.sge[]
  2026-07-09  2:38 ` yanjun.zhu
@ 2026-07-09  7:26   ` Ibrahim Hashimov
  0 siblings, 0 replies; 5+ messages in thread
From: Ibrahim Hashimov @ 2026-07-09  7:26 UTC (permalink / raw)
  To: yanjun.zhu, zyjzyj2000, jgg, leon; +Cc: linux-rdma, linux-kernel

Hi Zhu Yanjun, thanks for the review -- good catch, you're right.

> From this function,
> static int rxe_init_sq(struct rxe_qp *qp, struct ibv_qp_init_attr *init_attr)
> ...
>      qp->sq.max_sge = init_attr->cap.max_send_sge;
> ...
> qp->sq.max_sge is also from the user space application. It is possible
> that qp->sq.max_sge is 0.
> Then this makes rxe_requester will return error and exit.

Correct: with max_sge == 0 my "cur_sge >= qp->sq.max_sge" check rejects
a legitimate zero-SGE WQE (cur_sge == 0), so such a QP could never post.
The claim in my commit message that zero-payload WQEs "remain valid" was
simply wrong for the max_sge == 0 case -- thanks for catching it.

One subtlety while fixing it: gating the check on num_sge alone is not
enough. The requester's payload is wqe->dma.resid, which is independent
of num_sge, so a crafted WQE with num_sge == 0, a large resid and an
out-of-range cur_sge would still reach copy_data() and dereference
dma->sge[cur_sge]. What actually makes a zero-SGE WQE safe is that
copy_data() returns early on length == 0, before it ever touches
dma->sge[]. So in v2 I gate the cur_sge bound on wqe->dma.resid (i.e.
"is there payload that will make copy_data() index dma->sge[]") rather
than on num_sge:

	if (unlikely(wqe->dma.num_sge > qp->sq.max_sge ||
		     (wqe->dma.resid &&
		      wqe->dma.cur_sge >= qp->sq.max_sge))) {

This lets a zero-payload WQE through (including on a max_sge == 0 QP),
still rejects the oversized-num_sge and out-of-range-cur_sge cases, and
keeps multi-packet sends valid (cur_sge advances within [0, num_sge)
while resid > 0, and num_sge <= max_sge keeps that in the array).

Re-verified on the same v6.19 KASAN (CONFIG_KASAN_VMALLOC=y) stand: the
original reproducer -- which posts a user-QP send WQE with an
out-of-range cur_sge -- reaches the exploit path and the QP now
completes cleanly with no "vmalloc-out-of-bounds in copy_data" report,
whereas the pre-fix kernel trips it.

One thing worth flagging for completeness: this gate (like the sibling
validate_send_wr()/get_srq_wqe() checks) validates the WQE in place in
the mmap'd ring, so it narrows rather than fully closes the underlying
race -- a userspace thread can still mutate cur_sge/resid after the
check and before copy_data() re-reads them. The retransmit path
(advance_dma_data() via req_retry()) also indexes dma->sge[cur_sge]
ahead of this check. Both are pre-existing and out of scope for this
one-liner, but I'm happy to look at them separately if you'd like.

I'll send this as [PATCH v2].

Thanks,
Ibrahim

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] RDMA/rxe: validate num_sge/cur_sge before indexing wqe->dma.sge[]
  2026-07-08 22:45 [PATCH] RDMA/rxe: validate num_sge/cur_sge before indexing wqe->dma.sge[] Ibrahim Hashimov
  2026-07-09  2:38 ` yanjun.zhu
@ 2026-07-09  7:26 ` Ibrahim Hashimov
  2026-07-09 18:37   ` yanjun.zhu
  1 sibling, 1 reply; 5+ messages in thread
From: Ibrahim Hashimov @ 2026-07-09  7:26 UTC (permalink / raw)
  To: yanjun.zhu, zyjzyj2000, jgg, leon; +Cc: linux-rdma, linux-kernel, stable

A user QP's send queue (qp->sq.queue) is a shared ring the userspace
application writes to directly via mmap (vmalloc_user(), see
rxe_queue.c). For such a QP, rxe_post_send() takes the qp->is_user
branch and only schedules the requester task -- it never validates or
copies the posted WQE:

	drivers/infiniband/sw/rxe/rxe_verbs.c:
		if (qp->is_user) {
			rxe_sched_task(&qp->send_task);
			...
		}

The requester then consumes the WQE in place straight out of that
mmap'd ring:

	rxe_req.c:	wqe = req_next_wqe(qp);
	rxe_req.c:	err = copy_data(qp->pd, 0, &wqe->dma,
					payload_addr(pkt), payload,
					RXE_FROM_MR_OBJ);

copy_data() indexes the per-WQE sge array with the attacker-controlled
cur_sge field and dereferences it (once there is payload to copy):

	rxe_mr.c:	struct rxe_sge *sge = &dma->sge[dma->cur_sge];
	rxe_mr.c:	...
	rxe_mr.c:	if (sge->length && (offset < sge->length)) {

dma->sge[] is a flex array whose real backing storage is exactly
qp->sq.max_sge entries per WQE slot (see rxe_qp.c, wqe_size computed
from max_sge at QP create time). Since a user QP's WQE bytes are
entirely attacker-supplied, both wqe->dma.num_sge and wqe->dma.cur_sge
can be set to arbitrary values independent of each other and of
max_sge. Only the *kernel*-QP post path bounds num_sge:

	rxe_verbs.c: validate_send_wr()
		if (num_sge > sq->max_sge) {
			rxe_err_qp(qp, "num_sge > max_sge\n");

but that function is only reachable from rxe_post_one_send() for
kernel-owned QPs; it is never consulted for a user QP's raw WQE.

The sibling receive path already has the equivalent guard, with the
literal comment documenting exactly why it is required:

	rxe_resp.c: get_srq_wqe()
		/* don't trust user space data */
		if (unlikely(wqe->dma.num_sge > srq->rq.max_sge)) {
			...
			rxe_dbg_qp(qp, "invalid num_sge in SRQ entry\n");
			return RESPST_ERR_MALFORMED_WQE;
		}

The send/requester path has no analogous check, so a local,
unprivileged user who can open /dev/infiniband/uverbs* and create a
user QP on a soft-RoCE (rxe) link can hand-craft a WQE in the shared
send queue with an out-of-range wqe->dma.cur_sge (or an oversized
wqe->dma.num_sge) and ring the send doorbell. rxe_requester() then
calls copy_data(), which dereferences &dma->sge[cur_sge] out of the
bounds of the per-WQE sge array -- a vmalloc out-of-bounds *read*
(confirmed via KASAN: "KASAN: vmalloc-out-of-bounds in copy_data"),
reliably panicking the kernel (local DoS). sge->addr itself is still
bounds-checked later by lookup_mr()/rxe_mr_copy(), so the primitive is
an OOB read of sge metadata, not an arbitrary read/write primitive.

Fix this the same way get_srq_wqe() already does for SRQ entries:
bound the fields pulled from the (possibly user-mapped) send queue
entry before they are used to index wqe->dma.sge[], right where the
requester fetches the next WQE off the ring in rxe_requester(). num_sge
is capped at qp->sq.max_sge (matching the sibling SRQ check and the
kernel-QP validate_send_wr() check). cur_sge is bounded only when the
WQE actually carries payload (wqe->dma.resid): copy_data() dereferences
dma->sge[cur_sge] only after its own length == 0 early return, so a
zero-payload WQE never touches the sge array and must not be rejected
-- notably that is the only kind of WQE a max_sge == 0 QP can post
(qp->sq.max_sge is itself derived from user-supplied max_send_sge /
max_inline_data and may be 0). Gating on resid rather than num_sge is
deliberate: payload is wqe->dma.resid, which is independent of num_sge,
so a WQE with num_sge == 0 but a large resid and an out-of-range
cur_sge would still reach the sge dereference.

This is a long-standing bug in the rxe (soft-RoCE) driver: the
qp->is_user bypass in rxe_post_send() and the unbounded
&dma->sge[dma->cur_sge] indexing in copy_data() have been present
since the driver was introduced.

Runtime-verified on a v6.19 KASAN (CONFIG_KASAN_VMALLOC=y) stand: a
reproducer that posts a user QP send WQE with an out-of-range cur_sge
reliably tripped "KASAN: vmalloc-out-of-bounds in copy_data" (an
out-of-bounds read) before this patch, and no longer triggers that
report with the patch applied.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
v2: address Zhu Yanjun's review of v1
    (https://lore.kernel.org/linux-rdma/20260708224534.1206-1-security@auditcode.ai/):
    qp->sq.max_sge can legitimately be 0, and v1's unconditional
    "cur_sge >= max_sge" check then wrongly rejected a valid zero-payload
    WQE. Gate the cur_sge bound on wqe->dma.resid instead (copy_data()
    dereferences dma->sge[] only when there is payload), so zero-payload
    WQEs -- the only kind a max_sge == 0 QP can post -- are accepted while
    the out-of-range cur_sge OOB is still rejected. Commit message fixed.

 drivers/infiniband/sw/rxe/rxe_req.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 12d03f390b09..363c56a1edbb 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -701,6 +701,28 @@ int rxe_requester(struct rxe_qp *qp)
 	if (unlikely(!wqe))
 		goto exit;
 
+	/*
+	 * Don't trust user space data: for a user QP, qp->sq.queue is a
+	 * raw ring the application writes directly, so this WQE's num_sge
+	 * and cur_sge are attacker-controlled. copy_data() dereferences
+	 * dma->sge[cur_sge] without bounding the initial cur_sge against
+	 * the per-WQE sge array, whose capacity is qp->sq.max_sge (the
+	 * loop there only bounds subsequent increments, against num_sge).
+	 * Bound num_sge to that capacity, the way get_srq_wqe() and
+	 * validate_send_wr() already do, and bound cur_sge only when the
+	 * WQE actually carries payload (dma.resid): copy_data() returns
+	 * early on a zero-length copy before it ever touches dma->sge[],
+	 * so a zero-payload WQE -- the only valid WQE on a max_sge == 0
+	 * QP -- must not be rejected here.
+	 */
+	if (unlikely(wqe->dma.num_sge > qp->sq.max_sge ||
+		     (wqe->dma.resid &&
+		      wqe->dma.cur_sge >= qp->sq.max_sge))) {
+		rxe_dbg_qp(qp, "invalid num_sge/cur_sge in send wqe\n");
+		wqe->status = IB_WC_LOC_QP_OP_ERR;
+		goto err;
+	}
+
 	if (rxe_wqe_is_fenced(qp, wqe)) {
 		qp->req.wait_fence = 1;
 		goto exit;
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] RDMA/rxe: validate num_sge/cur_sge before indexing wqe->dma.sge[]
  2026-07-09  7:26 ` [PATCH v2] " Ibrahim Hashimov
@ 2026-07-09 18:37   ` yanjun.zhu
  0 siblings, 0 replies; 5+ messages in thread
From: yanjun.zhu @ 2026-07-09 18:37 UTC (permalink / raw)
  To: Ibrahim Hashimov, zyjzyj2000, jgg, leon, Zhu Yanjun
  Cc: linux-rdma, linux-kernel, stable

On 7/9/26 12:26 AM, Ibrahim Hashimov wrote:
> A user QP's send queue (qp->sq.queue) is a shared ring the userspace
> application writes to directly via mmap (vmalloc_user(), see
> rxe_queue.c). For such a QP, rxe_post_send() takes the qp->is_user
> branch and only schedules the requester task -- it never validates or
> copies the posted WQE:
> 
> 	drivers/infiniband/sw/rxe/rxe_verbs.c:
> 		if (qp->is_user) {
> 			rxe_sched_task(&qp->send_task);
> 			...
> 		}
> 
> The requester then consumes the WQE in place straight out of that
> mmap'd ring:
> 
> 	rxe_req.c:	wqe = req_next_wqe(qp);
> 	rxe_req.c:	err = copy_data(qp->pd, 0, &wqe->dma,
> 					payload_addr(pkt), payload,
> 					RXE_FROM_MR_OBJ);
> 
> copy_data() indexes the per-WQE sge array with the attacker-controlled
> cur_sge field and dereferences it (once there is payload to copy):
> 
> 	rxe_mr.c:	struct rxe_sge *sge = &dma->sge[dma->cur_sge];
> 	rxe_mr.c:	...
> 	rxe_mr.c:	if (sge->length && (offset < sge->length)) {
> 
> dma->sge[] is a flex array whose real backing storage is exactly
> qp->sq.max_sge entries per WQE slot (see rxe_qp.c, wqe_size computed
> from max_sge at QP create time). Since a user QP's WQE bytes are
> entirely attacker-supplied, both wqe->dma.num_sge and wqe->dma.cur_sge
> can be set to arbitrary values independent of each other and of
> max_sge. Only the *kernel*-QP post path bounds num_sge:
> 
> 	rxe_verbs.c: validate_send_wr()
> 		if (num_sge > sq->max_sge) {
> 			rxe_err_qp(qp, "num_sge > max_sge\n");
> 
> but that function is only reachable from rxe_post_one_send() for
> kernel-owned QPs; it is never consulted for a user QP's raw WQE.
> 
> The sibling receive path already has the equivalent guard, with the
> literal comment documenting exactly why it is required:
> 
> 	rxe_resp.c: get_srq_wqe()
> 		/* don't trust user space data */
> 		if (unlikely(wqe->dma.num_sge > srq->rq.max_sge)) {
> 			...
> 			rxe_dbg_qp(qp, "invalid num_sge in SRQ entry\n");
> 			return RESPST_ERR_MALFORMED_WQE;
> 		}
> 
> The send/requester path has no analogous check, so a local,
> unprivileged user who can open /dev/infiniband/uverbs* and create a
> user QP on a soft-RoCE (rxe) link can hand-craft a WQE in the shared
> send queue with an out-of-range wqe->dma.cur_sge (or an oversized
> wqe->dma.num_sge) and ring the send doorbell. rxe_requester() then
> calls copy_data(), which dereferences &dma->sge[cur_sge] out of the
> bounds of the per-WQE sge array -- a vmalloc out-of-bounds *read*
> (confirmed via KASAN: "KASAN: vmalloc-out-of-bounds in copy_data"),
> reliably panicking the kernel (local DoS). sge->addr itself is still
> bounds-checked later by lookup_mr()/rxe_mr_copy(), so the primitive is
> an OOB read of sge metadata, not an arbitrary read/write primitive.
> 
> Fix this the same way get_srq_wqe() already does for SRQ entries:
> bound the fields pulled from the (possibly user-mapped) send queue
> entry before they are used to index wqe->dma.sge[], right where the
> requester fetches the next WQE off the ring in rxe_requester(). num_sge
> is capped at qp->sq.max_sge (matching the sibling SRQ check and the
> kernel-QP validate_send_wr() check). cur_sge is bounded only when the
> WQE actually carries payload (wqe->dma.resid): copy_data() dereferences
> dma->sge[cur_sge] only after its own length == 0 early return, so a
> zero-payload WQE never touches the sge array and must not be rejected
> -- notably that is the only kind of WQE a max_sge == 0 QP can post
> (qp->sq.max_sge is itself derived from user-supplied max_send_sge /
> max_inline_data and may be 0). Gating on resid rather than num_sge is
> deliberate: payload is wqe->dma.resid, which is independent of num_sge,
> so a WQE with num_sge == 0 but a large resid and an out-of-range
> cur_sge would still reach the sge dereference.
> 
> This is a long-standing bug in the rxe (soft-RoCE) driver: the
> qp->is_user bypass in rxe_post_send() and the unbounded
> &dma->sge[dma->cur_sge] indexing in copy_data() have been present
> since the driver was introduced.
> 
> Runtime-verified on a v6.19 KASAN (CONFIG_KASAN_VMALLOC=y) stand: a
> reproducer that posts a user QP send WQE with an out-of-range cur_sge
> reliably tripped "KASAN: vmalloc-out-of-bounds in copy_data" (an
> out-of-bounds read) before this patch, and no longer triggers that
> report with the patch applied.
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
> Assisted-by: AuditCode-AI:2026.07
> ---
> v2: address Zhu Yanjun's review of v1
>      (https://lore.kernel.org/linux-rdma/20260708224534.1206-1-security@auditcode.ai/):
>      qp->sq.max_sge can legitimately be 0, and v1's unconditional
>      "cur_sge >= max_sge" check then wrongly rejected a valid zero-payload
>      WQE. Gate the cur_sge bound on wqe->dma.resid instead (copy_data()
>      dereferences dma->sge[] only when there is payload), so zero-payload
>      WQEs -- the only kind a max_sge == 0 QP can post -- are accepted while
>      the out-of-range cur_sge OOB is still rejected. Commit message fixed.

Thanks a lot. I am fine with this. Please Leon and Jason comment on this.

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

Zhu Yanjun

> 
>   drivers/infiniband/sw/rxe/rxe_req.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index 12d03f390b09..363c56a1edbb 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c
> @@ -701,6 +701,28 @@ int rxe_requester(struct rxe_qp *qp)
>   	if (unlikely(!wqe))
>   		goto exit;
>   
> +	/*
> +	 * Don't trust user space data: for a user QP, qp->sq.queue is a
> +	 * raw ring the application writes directly, so this WQE's num_sge
> +	 * and cur_sge are attacker-controlled. copy_data() dereferences
> +	 * dma->sge[cur_sge] without bounding the initial cur_sge against
> +	 * the per-WQE sge array, whose capacity is qp->sq.max_sge (the
> +	 * loop there only bounds subsequent increments, against num_sge).
> +	 * Bound num_sge to that capacity, the way get_srq_wqe() and
> +	 * validate_send_wr() already do, and bound cur_sge only when the
> +	 * WQE actually carries payload (dma.resid): copy_data() returns
> +	 * early on a zero-length copy before it ever touches dma->sge[],
> +	 * so a zero-payload WQE -- the only valid WQE on a max_sge == 0
> +	 * QP -- must not be rejected here.
> +	 */
> +	if (unlikely(wqe->dma.num_sge > qp->sq.max_sge ||
> +		     (wqe->dma.resid &&
> +		      wqe->dma.cur_sge >= qp->sq.max_sge))) {
> +		rxe_dbg_qp(qp, "invalid num_sge/cur_sge in send wqe\n");
> +		wqe->status = IB_WC_LOC_QP_OP_ERR;
> +		goto err;
> +	}
> +
>   	if (rxe_wqe_is_fenced(qp, wqe)) {
>   		qp->req.wait_fence = 1;
>   		goto exit;


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-09 18:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08 22:45 [PATCH] RDMA/rxe: validate num_sge/cur_sge before indexing wqe->dma.sge[] Ibrahim Hashimov
2026-07-09  2:38 ` yanjun.zhu
2026-07-09  7:26   ` Ibrahim Hashimov
2026-07-09  7:26 ` [PATCH v2] " Ibrahim Hashimov
2026-07-09 18:37   ` yanjun.zhu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox