[PATCH v4 0/2] RDMA/rxe: fix TOCTOU races in send WQE processing

[PATCH v4 0/2] RDMA/rxe: fix TOCTOU races in send WQE processing

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The SoftRoCE driver maps its send work queue into userspace for direct
posting. The kernel reads WQE fields directly from this shared mapping,
which allows a concurrent userspace thread to modify fields between
kernel reads -- classic TOCTOU.

This series copies each send WQE to a kernel-private buffer before
processing, in both the requester (patch 1) and completer (patch 2)
paths.

Changes since v3:
  - Copy the full queue element (max_sge SGEs) instead of computing a
    per-WQE copy size from num_sge. This ensures inline data (which
    shares the flex array with SGEs) is always captured, and eliminates
    a TOCTOU on the copy size itself.
  - Clamp dma.num_sge against qp->sq.max_sge after copy (patch 2)
    instead of against the global RXE_MAX_SGE constant.
  - Invalidate the cached copy on QP reset (rxe_qp_reset), on the
    ERR flush path, and on the RESET state check in the requester.
    This prevents stale-cache reuse after state transitions.
  - Each patch now also touches rxe_qp.c for the reset invalidation.

Changes since v2:
  - Addressed review comments on naming and ordering.
  - Writeback status using WRITE_ONCE() instead of plain store.
  - Added smp_store_release() in requester for state transitions.

Changes since v1:
  - Split into per-path patches (requester, completer).
  - Cache the local copy across retransmits and multi-packet operations.
  - Added writeback of completion status and rd_atomic state.

Tristan Madani (2):
  RDMA/rxe: copy send WQE to kernel buffer before processing
  RDMA/rxe: copy send WQE to kernel buffer in completer path

 drivers/infiniband/sw/rxe/rxe_comp.c  | 53 +++++++++++++++++++++++++-
 drivers/infiniband/sw/rxe/rxe_qp.c    |  2 +
 drivers/infiniband/sw/rxe/rxe_req.c   | 55 +++++++++++++++++++++++++--
 drivers/infiniband/sw/rxe/rxe_verbs.h | 12 ++++++
 4 files changed, 116 insertions(+), 6 deletions(-)

-- 
2.47.3

[PATCH v4 1/2] RDMA/rxe: copy send WQE to kernel buffer before processing

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The rxe send queue is mapped into userspace via mmap. The requester
processes Work Queue Entries (WQEs) directly from this shared buffer
without first copying them to kernel memory. Userspace can modify WQE
fields (num_sge, sge_offset, SGE entries) between kernel reads,
leading to inconsistent state in copy_data().

This is the send-path counterpart to the receive-path fixes:
  - commit 22b8fbded65b8 ("RDMA/rxe: Fix TOCTOU heap overflow in
    get_srq_wqe")
  - commit d6ab440240a04 ("RDMA/rxe: Copy WQE to local buffer in
    non-SRQ receive path")

Fix by copying the send WQE to a kernel-private buffer in req_next_wqe()
before processing. The copy uses the full queue element size (max_sge
SGEs) so that inline data, which shares the flex array with SGEs, is
always captured regardless of num_sge.

The local copy is reused across multi-packet sends to preserve DMA
progress state (cur_sge, sge_offset, resid). Field updates are written
back to the shared queue using WRITE_ONCE() for individual fields and
smp_store_release() for state transitions, so the completer and
userspace observe consistent values without the tearing risk of bulk
memcpy().

The copy is invalidated when the WQE index advances (last packet sent,
error, local ops, UD oversized), when a retry resets WQE state, on QP
reset, or when the QP enters error state for flush.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 drivers/infiniband/sw/rxe/rxe_qp.c    |  1 +
 drivers/infiniband/sw/rxe/rxe_req.c   | 55 +++++++++++++++++++++++++--
 drivers/infiniband/sw/rxe/rxe_verbs.h |  6 +++
 3 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index 311f285d78a6b..77606c4a039b1 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -581,6 +581,7 @@ static void rxe_qp_reset(struct rxe_qp *qp)
 	qp->req.need_retry = 0;
 	qp->req.wait_for_rnr_timer = 0;
 	qp->req.noack_pkts = 0;
+	qp->req.send_wqe_valid = false;
 	qp->resp.msn = 0;
 	qp->resp.opcode = -1;
 	qp->resp.drop_msg = 0;
diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 24f5c044363f7..2abb2131c0a90 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -161,6 +161,26 @@ static void req_check_sq_drain_done(struct rxe_qp *qp)
 	spin_unlock_irqrestore(&qp->state_lock, flags);
 }
 
+/* Write back requester WQE fields to shared memory using targeted
+ * stores so the completer and userspace observe consistent state.
+ */
+static void rxe_req_writeback_wqe(struct rxe_qp *qp)
+{
+	struct rxe_send_wqe *shared = qp->req.shared_wqe;
+	struct rxe_send_wqe *local = &qp->req.send_wqe.wqe;
+
+	if (!qp->req.send_wqe_valid || !shared)
+		return;
+
+	WRITE_ONCE(shared->status, local->status);
+	WRITE_ONCE(shared->first_psn, local->first_psn);
+	WRITE_ONCE(shared->last_psn, local->last_psn);
+	WRITE_ONCE(shared->mask, local->mask);
+	WRITE_ONCE(shared->has_rd_atomic, local->has_rd_atomic);
+	/* State must be last so the completer sees prior updates */
+	smp_store_release(&shared->state, local->state);
+}
+
 static struct rxe_send_wqe *__req_next_wqe(struct rxe_qp *qp)
 {
 	struct rxe_queue *q = qp->sq.queue;
@@ -193,6 +213,20 @@ static struct rxe_send_wqe *req_next_wqe(struct rxe_qp *qp)
 	}
 	spin_unlock_irqrestore(&qp->state_lock, flags);
 
+	/* Reuse the existing kernel-private copy if still valid */
+	if (qp->req.send_wqe_valid && qp->req.shared_wqe == wqe)
+		return &qp->req.send_wqe.wqe;
+
+	/* Copy WQE from userspace-mapped shared queue to kernel-private
+	 * buffer. Copy the full element (max_sge SGEs) so that inline
+	 * data, which shares the flex array with SGEs, is always captured.
+	 */
+	memcpy(&qp->req.send_wqe.wqe, wqe,
+	       sizeof(*wqe) + qp->sq.max_sge * sizeof(struct rxe_sge));
+	qp->req.shared_wqe = wqe;
+	qp->req.send_wqe_valid = true;
+
+	wqe = &qp->req.send_wqe.wqe;
 	wqe->mask = wr_opcode_mask(wqe->wr.opcode, qp);
 	return wqe;
 }
@@ -582,9 +616,13 @@ static void update_state(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
 {
 	qp->req.opcode = pkt->opcode;
 
-	if (pkt->mask & RXE_END_MASK)
+	rxe_req_writeback_wqe(qp);
+
+	if (pkt->mask & RXE_END_MASK) {
 		qp->req.wqe_index = queue_next_index(qp->sq.queue,
 						     qp->req.wqe_index);
+		qp->req.send_wqe_valid = false;
+	}
 
 	qp->need_req_skb = 0;
 
@@ -634,7 +672,9 @@ static int rxe_do_local_ops(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
 
 	wqe->state = wqe_state_done;
 	wqe->status = IB_WC_SUCCESS;
+	rxe_req_writeback_wqe(qp);
 	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
+	qp->req.send_wqe_valid = false;
 
 	return 0;
 }
@@ -666,6 +706,7 @@ int rxe_requester(struct rxe_qp *qp)
 		wqe = __req_next_wqe(qp);
 		spin_unlock_irqrestore(&qp->state_lock, flags);
 		if (wqe) {
+			qp->req.send_wqe_valid = false;
 			wqe->status = IB_WC_WR_FLUSH_ERR;
 			goto err;
 		} else {
@@ -681,6 +722,7 @@ int rxe_requester(struct rxe_qp *qp)
 		qp->req.wait_psn = 0;
 		qp->req.need_retry = 0;
 		qp->req.wait_for_rnr_timer = 0;
+		qp->req.send_wqe_valid = false;
 		spin_unlock_irqrestore(&qp->state_lock, flags);
 		goto exit;
 	}
@@ -695,6 +737,7 @@ int rxe_requester(struct rxe_qp *qp)
 	if (unlikely(qp->req.need_retry && !qp->req.wait_for_rnr_timer)) {
 		req_retry(qp);
 		qp->req.need_retry = 0;
+		qp->req.send_wqe_valid = false;
 	}
 
 	wqe = req_next_wqe(qp);
@@ -772,10 +815,12 @@ int rxe_requester(struct rxe_qp *qp)
 			wqe->last_psn = qp->req.psn;
 			qp->req.psn = (qp->req.psn + 1) & BTH_PSN_MASK;
 			qp->req.opcode = IB_OPCODE_UD_SEND_ONLY;
-			qp->req.wqe_index = queue_next_index(qp->sq.queue,
-						       qp->req.wqe_index);
 			wqe->state = wqe_state_done;
 			wqe->status = IB_WC_SUCCESS;
+			rxe_req_writeback_wqe(qp);
+			qp->req.wqe_index = queue_next_index(qp->sq.queue,
+						       qp->req.wqe_index);
+			qp->req.send_wqe_valid = false;
 			goto done;
 		}
 		payload = mtu;
@@ -839,8 +884,10 @@ int rxe_requester(struct rxe_qp *qp)
 	goto out;
 err:
 	/* update wqe_index for each wqe completion */
-	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
 	wqe->state = wqe_state_error;
+	rxe_req_writeback_wqe(qp);
+	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
+	qp->req.send_wqe_valid = false;
 	rxe_qp_error(qp);
 exit:
 	ret = -EAGAIN;
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index 0f5ffd94643f9..a22dfc6e5ae3c 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -114,6 +114,12 @@ struct rxe_req_info {
 	int			wait_for_rnr_timer;
 	int			noack_pkts;
 	int			again;
+	struct rxe_send_wqe	*shared_wqe;
+	bool			send_wqe_valid;
+	struct {
+		struct rxe_send_wqe	wqe;
+		struct ib_sge		sge[RXE_MAX_SGE];
+	} send_wqe;
 };
 
 struct rxe_comp_info {
-- 
2.47.3

Re: [PATCH v4 1/2] RDMA/rxe: copy send WQE to kernel buffer before processing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] rdma: remote kernel heap information leak via unvalidated inline data length
- [High] mm: fortify_source field-spanning write warning in req_next_wqe memcpy
- [High] mm: truncated memcpy of WQE flex array corrupts inline data
- [Medium] locking: missing memory barriers when flushing WQEs leads to stale completion status
--

commit 05fdd210d6581da6d266954d4f637a96e6640c15
Author: Tristan Madani <tristan@talencesecurity.com>
Subject: RDMA/rxe: copy send WQE to kernel buffer before processing

This commit copies the requester WQE from a user-space mapped buffer to a
kernel-private buffer before processing to prevent Time-Of-Check to
Time-Of-Use (TOCTOU) modifications. It also introduces a local copy reuse
mechanism and targeted writebacks to the shared queue to prevent tearing.

> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index 53f7c1e7fe55b..df0fa455cd9c1 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c

[ ... ]

> @@ -193,6 +213,20 @@ static struct rxe_send_wqe *req_next_wqe(struct rxe_qp *qp)
>  	}
>  	spin_unlock_irqrestore(&qp->state_lock, flags);
>  
> +	/* Reuse the existing kernel-private copy if still valid */
> +	if (qp->req.send_wqe_valid && qp->req.shared_wqe == wqe)
> +		return &qp->req.send_wqe.wqe;
> +
> +	/* Copy WQE from userspace-mapped shared queue to kernel-private
> +	 * buffer. Copy the full element (max_sge SGEs) so that inline
> +	 * data, which shares the flex array with SGEs, is always captured.
> +	 */
> +	memcpy(&qp->req.send_wqe.wqe, wqe,
> +	       sizeof(*wqe) + qp->sq.max_sge * sizeof(struct rxe_sge));

[Severity: High]
Does this memcpy trigger a fortify_source field-spanning write warning?

The destination pointer &qp->req.send_wqe.wqe explicitly points to the wqe
member of the anonymous send_wqe struct, which has a size of 64 bytes. If
max_sge > 0, the copy length exceeds 64 bytes.

When compiled with CONFIG_FORTIFY_SOURCE=y, __builtin_object_size(p, 1) bounds
the destination to the exact size of the wqe member because it is embedded
inside an outer struct before the sge array. This detects the out-of-bounds
write and triggers a deterministic run-time field-spanning write warning,
potentially crashing the kernel if panic_on_warn is enabled.

[Severity: High]
Can this size calculation truncate the inline data if max_inline_data is not
a multiple of 16?

During QP initialization, qp->sq.max_sge is computed via integer division
(wqe_size / sizeof(struct ib_sge)). If max_inline_data is configured to a
value that is not a multiple of 16 (e.g., 20 bytes), wqe_size evaluates to
20, but max_sge evaluates to 1.

The memcpy size would then evaluate to 64 + 1 * 16 = 80 bytes, whereas the
inline data spans 20 bytes up to offset 84. This truncates the copy, leaving
the last 4 bytes of inline data uncopied. When finish_packet() reads the
inline data from the kernel buffer, will it transmit uninitialized or stale
kernel memory instead of the user's provided payload?

> +	qp->req.shared_wqe = wqe;
> +	qp->req.send_wqe_valid = true;
> +
> +	wqe = &qp->req.send_wqe.wqe;
>  	wqe->mask = wr_opcode_mask(wqe->wr.opcode, qp);
>  	return wqe;
>  }

[ ... ]

> @@ -666,6 +706,7 @@ int rxe_requester(struct rxe_qp *qp)
>  		wqe = __req_next_wqe(qp);
>  		spin_unlock_irqrestore(&qp->state_lock, flags);
>  		if (wqe) {
> +			qp->req.send_wqe_valid = false;
>  			wqe->status = IB_WC_WR_FLUSH_ERR;
>  			goto err;

[ ... ]

> @@ -695,6 +737,7 @@ int rxe_requester(struct rxe_qp *qp)
>  	if (unlikely(qp->req.need_retry && !qp->req.wait_for_rnr_timer)) {
>  		req_retry(qp);
>  		qp->req.need_retry = 0;
> +		qp->req.send_wqe_valid = false;
>  	}
>  
>  	wqe = req_next_wqe(qp);

[Severity: Critical]
Does this introduce a remote kernel heap information leak for inline packets?

By moving the WQE buffer to the kernel heap, we now need to validate bounds
that were previously safe to read from shared memory. For inline data packets,
wqe->dma.resid (which stores the user-provided payload length) is only bounded
by qp->mtu (e.g. 4096). The validation checks in this function completely
ignore wqe->dma.resid and wqe->dma.sge_offset for inline sends.

Later in finish_packet(), the inline payload is copied into the network packet:

memcpy(payload_addr(pkt), &wqe->dma.inline_data[wqe->dma.sge_offset], payload);

Could a malicious user set sge_offset = 0 and length to 4096? The memcpy
would read past the end of the qp->req.send_wqe allocation and leak adjacent
kernel heap memory into the outgoing network packet.

>  	if (unlikely(!wqe))
>  		goto exit;

[ ... ]

> @@ -839,8 +884,10 @@ int rxe_requester(struct rxe_qp *qp)
>  	goto out;
>  err:
>  	/* update wqe_index for each wqe completion */
> -	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
>  	wqe->state = wqe_state_error;
> +	rxe_req_writeback_wqe(qp);

[Severity: Medium]
Does this fast-flush path miss memory barriers when updating the shared WQE?

When handling IB_QPS_ERR, this function sets qp->req.send_wqe_valid = false,
assigns wqe->status = IB_WC_WR_FLUSH_ERR directly, and then jumps to err:,
where it sets wqe->state = wqe_state_error; and calls rxe_req_writeback_wqe(qp).

Because qp->req.send_wqe_valid is false, rxe_req_writeback_wqe() returns
immediately without issuing the intended smp_store_release() memory barrier.

Could the CPU or compiler reorder the writes to status and state, causing
the completer polling the shared queue to observe wqe_state_error but read a
stale or uninitialized status?

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

[PATCH v4 2/2] RDMA/rxe: copy send WQE to kernel buffer in completer path

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The completer reads send WQEs from the same userspace-mapped shared
queue as the requester. Even after the requester copies the WQE to a
kernel-private buffer (previous patch), the completer still reads
directly from shared memory, leaving it exposed to the same TOCTOU
races.

Fix by copying the WQE in get_wqe() to a completer-private buffer
using the full queue element size (max_sge SGEs). The local copy is
reused when the same WQE is being processed with unchanged state,
which preserves DMA progress across multi-packet RDMA READ responses.
A fresh copy is taken when the state changes or a new WQE appears.

State transitions from the requester are observed through an
smp_load_acquire() / smp_store_release() pair. Completion status and
rd_atomic state are written back through WRITE_ONCE() before the
consumer index advances.

The copy is invalidated on QP reset and when the completion advances
to the next WQE.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 drivers/infiniband/sw/rxe/rxe_comp.c  | 53 ++++++++++++++++++++++++++-
 drivers/infiniband/sw/rxe/rxe_qp.c    |  1 +
 drivers/infiniband/sw/rxe/rxe_verbs.h |  6 +++
 3 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
index 1390e861bd1d7..ca31093fdf3aa 100644
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -137,22 +137,69 @@ void rxe_comp_queue_pkt(struct rxe_qp *qp, struct sk_buff *skb)
 	rxe_sched_task(&qp->send_task);
 }
 
+/* Write back completer WQE fields to shared memory */
+static void rxe_comp_writeback_wqe(struct rxe_qp *qp)
+{
+	struct rxe_send_wqe *shared = qp->comp.shared_wqe;
+	struct rxe_send_wqe *local = &qp->comp.comp_wqe.wqe;
+
+	if (!qp->comp.comp_wqe_valid || !shared)
+		return;
+
+	WRITE_ONCE(shared->status, local->status);
+	WRITE_ONCE(shared->has_rd_atomic, local->has_rd_atomic);
+}
+
 static inline enum comp_state get_wqe(struct rxe_qp *qp,
 				      struct rxe_pkt_info *pkt,
 				      struct rxe_send_wqe **wqe_p)
 {
 	struct rxe_send_wqe *wqe;
+	u32 state;
 
 	/* we come here whether or not we found a response packet to see if
 	 * there are any posted WQEs
 	 */
 	wqe = queue_head(qp->sq.queue, QUEUE_TYPE_FROM_CLIENT);
-	*wqe_p = wqe;
 
 	/* no WQE or requester has not started it yet */
-	if (!wqe || wqe->state == wqe_state_posted)
+	if (!wqe) {
+		*wqe_p = NULL;
 		return pkt ? COMPST_DONE : COMPST_EXIT;
+	}
+
+	/* Pairs with smp_store_release() in rxe_req_writeback_wqe() */
+	state = smp_load_acquire(&wqe->state);
+	if (state == wqe_state_posted) {
+		*wqe_p = wqe;
+		return pkt ? COMPST_DONE : COMPST_EXIT;
+	}
+
+	/* Reuse existing local copy if still processing same WQE
+	 * with unchanged state (preserves DMA progress for multi-packet ops)
+	 */
+	if (qp->comp.comp_wqe_valid && qp->comp.shared_wqe == wqe &&
+	    qp->comp.comp_wqe.wqe.state == state) {
+		wqe = &qp->comp.comp_wqe.wqe;
+		*wqe_p = wqe;
+		goto check_state;
+	}
+
+	/* Copy shared WQE to kernel-private buffer. Copy the full
+	 * element (max_sge SGEs) so that inline data, which shares
+	 * the flex array with SGEs, is always captured.
+	 */
+	qp->comp.shared_wqe = wqe;
+	memcpy(&qp->comp.comp_wqe.wqe, wqe,
+	       sizeof(*wqe) + qp->sq.max_sge * sizeof(struct rxe_sge));
+	qp->comp.comp_wqe_valid = true;
+	if (qp->comp.comp_wqe.wqe.dma.num_sge > qp->sq.max_sge)
+		qp->comp.comp_wqe.wqe.dma.num_sge = qp->sq.max_sge;
+
+	wqe = &qp->comp.comp_wqe.wqe;
+	*wqe_p = wqe;
 
+check_state:
 	/* WQE does not require an ack */
 	if (wqe->state == wqe_state_done)
 		return COMPST_COMP_WQE;
@@ -454,6 +501,8 @@ static void do_complete(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
 	if (post)
 		make_send_cqe(qp, wqe, &cqe);
 
+	rxe_comp_writeback_wqe(qp);
+	qp->comp.comp_wqe_valid = false;
 	queue_advance_consumer(qp->sq.queue, QUEUE_TYPE_FROM_CLIENT);
 
 	if (post)
diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index 77606c4a039b1..450254a131017 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -582,6 +582,7 @@ static void rxe_qp_reset(struct rxe_qp *qp)
 	qp->req.wait_for_rnr_timer = 0;
 	qp->req.noack_pkts = 0;
 	qp->req.send_wqe_valid = false;
+	qp->comp.comp_wqe_valid = false;
 	qp->resp.msn = 0;
 	qp->resp.opcode = -1;
 	qp->resp.drop_msg = 0;
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index a22dfc6e5ae3c..6205dbc29dff6 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -130,6 +130,12 @@ struct rxe_comp_info {
 	int			started_retry;
 	u32			retry_cnt;
 	u32			rnr_retry;
+	struct rxe_send_wqe	*shared_wqe;
+	bool			comp_wqe_valid;
+	struct {
+		struct rxe_send_wqe	wqe;
+		struct ib_sge		sge[RXE_MAX_SGE];
+	} comp_wqe;
 };
 
 /* responder states */
-- 
2.47.3

Re: [PATCH v4 2/2] RDMA/rxe: copy send WQE to kernel buffer in completer path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] rdma/rxe: data corruption in RDMA READ retry due to private WQE DMA state
- [High] rdma/rxe: TOCTOU / lockless torn read of wqe->state during memcpy
- [High] RDMA/rxe: memcpy into WQE buffer triggers FORTIFY_SOURCE panic
- [High] RDMA/rxe: TOCTOU bypass in completer WQE copy via timeout retry path
--

commit 59f69c9fb12619fc21466b561c2742dd05e89831
Author: Tristan Madani <tristan@talencesecurity.com>
Date:   Wed Oct 7 22:32:22 2026 +0000

    RDMA/rxe: copy send WQE to kernel buffer in completer path
    
This patch aims to prevent TOCTOU races by copying send WQEs to a kernel
private buffer during completion processing. It uses an acquire/release
pattern to observe state transitions and writes back completion status
and rd_atomic state to the shared memory queue.

> diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
> index 1390e861bd1d7..ca31093fdf3aa 100644
> --- a/drivers/infiniband/sw/rxe/rxe_comp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_comp.c
[ ... ]
> +/* Write back completer WQE fields to shared memory */
> +static void rxe_comp_writeback_wqe(struct rxe_qp *qp)
> +{
> +	struct rxe_send_wqe *shared = qp->comp.shared_wqe;
> +	struct rxe_send_wqe *local = &qp->comp.comp_wqe.wqe;
> +
> +	if (!qp->comp.comp_wqe_valid || !shared)
> +		return;
> +
> +	WRITE_ONCE(shared->status, local->status);
> +	WRITE_ONCE(shared->has_rd_atomic, local->has_rd_atomic);
> +}

[Severity: Critical]
Does this omit writing back the advanced dma.resid and dma.sge_offset fields
to the shared WQE?

If a multi-packet RDMA READ response experiences a network drop and triggers
a requester retry, the requester will read the shared WQE where dma.resid
still equals dma.length. This causes it to re-request data from the original
starting iova.

>  static inline enum comp_state get_wqe(struct rxe_qp *qp,
>  				      struct rxe_pkt_info *pkt,
>  				      struct rxe_send_wqe **wqe_p)
>  {
>  	struct rxe_send_wqe *wqe;
> +	u32 state;
[ ... ]
> +	/* Pairs with smp_store_release() in rxe_req_writeback_wqe() */
> +	state = smp_load_acquire(&wqe->state);
> +	if (state == wqe_state_posted) {
> +		*wqe_p = wqe;
> +		return pkt ? COMPST_DONE : COMPST_EXIT;
> +	}

[Severity: High]
If state is wqe_state_posted, this assigns the shared memory pointer directly
to wqe_p without making a local copy.

If a timeout triggers this path, rxe_completer() transitions to
COMPST_ERROR_RETRY where it re-reads wqe->state from this shared pointer.
Could a concurrent userspace modification change the state to bypass the
wqe_state_posted check, leading to COMPST_ERROR where it re-reads
wqe->status directly from shared memory?

If userspace races to set wqe->status to IB_WC_SUCCESS, could this trigger
the WARN_ON_ONCE(wqe->status == IB_WC_SUCCESS) in rxe_completer() and
subsequently process a corrupted WQE?

> +
> +	/* Reuse existing local copy if still processing same WQE
> +	 * with unchanged state (preserves DMA progress for multi-packet ops)
> +	 */
> +	if (qp->comp.comp_wqe_valid && qp->comp.shared_wqe == wqe &&
> +	    qp->comp.comp_wqe.wqe.state == state) {
> +		wqe = &qp->comp.comp_wqe.wqe;
> +		*wqe_p = wqe;
> +		goto check_state;
> +	}

[Severity: Critical]
Could this reuse logic cause memory corruption during an RDMA READ retry?

Because rxe_comp_writeback_wqe() does not sync the advanced DMA state to
shared memory, a retried request will re-request the beginning of the
data. However, the requester's retry rewinds the shared WQE state to
wqe_state_posted and back to wqe_state_pending.

This allows the state check here to succeed, meaning the completer reuses
its private WQE containing the advanced DMA pointer. Won't the completer
then write the re-transmitted beginning data into the middle of the local
buffer?

> +
> +	/* Copy shared WQE to kernel-private buffer. Copy the full
> +	 * element (max_sge SGEs) so that inline data, which shares
> +	 * the flex array with SGEs, is always captured.
> +	 */
> +	qp->comp.shared_wqe = wqe;
> +	memcpy(&qp->comp.comp_wqe.wqe, wqe,
> +	       sizeof(*wqe) + qp->sq.max_sge * sizeof(struct rxe_sge));

[Severity: High]
This problem wasn't introduced by this patch, but it appears here and was
also introduced in the requester path in the previous patch of this series.

Because the destination pointer specifically targets the wqe field
(&qp->comp.comp_wqe.wqe), the compiler evaluates the object size strictly
as sizeof(struct rxe_send_wqe).

When max_sge > 0, the size exceeds the bounds of the wqe field. When
CONFIG_FORTIFY_SOURCE=y is enabled, will this trigger a runtime
field-spanning write warning or panic? Would it be safer to copy into
the enclosing struct address, such as &qp->comp.comp_wqe?

[Severity: High]
Is there a Time-of-Check to Time-of-Use race when copying the state?

The memcpy is an unordered bulk copy that re-reads wqe->state from shared
memory. A concurrent requester could update the WQE fields in shared memory
during this copy. Because memcpy provides no memory ordering, it could copy
the new state but an old status.

> +	qp->comp.comp_wqe_valid = true;
> +	if (qp->comp.comp_wqe.wqe.dma.num_sge > qp->sq.max_sge)
> +		qp->comp.comp_wqe.wqe.dma.num_sge = qp->sq.max_sge;
> +
> +	wqe = &qp->comp.comp_wqe.wqe;
> +	*wqe_p = wqe;
>  
> +check_state:
>  	/* WQE does not require an ack */
> -	if (wqe->state == wqe_state_done)
> +	if (wqe->state == wqe_state_done)

[Severity: High]
Here, the code checks if wqe->state == wqe_state_done using the value
inside the locally copied WQE.

Since this evaluates the re-read state from the memcpy rather than the
state local variable acquired safely via smp_load_acquire() earlier, could
this defeat the memory barrier and cause the completer to process a done
WQE with stale fields?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007223222.2342804-1-tristmd@gmail.com?part=2

Re: [PATCH v4 0/2] RDMA/rxe: fix TOCTOU races in send WQE processing

[Zhu Yanjun]

在 2026/10/7 15:32, Tristan Madani 写道:
> From: Tristan Madani <tristan@talencesecurity.com>
> 
> The SoftRoCE driver maps its send work queue into userspace for direct
> posting. The kernel reads WQE fields directly from this shared mapping,
> which allows a concurrent userspace thread to modify fields between
> kernel reads -- classic TOCTOU.
> 
> This series copies each send WQE to a kernel-private buffer before
> processing, in both the requester (patch 1) and completer (patch 2)
> paths.
> 
> Changes since v3:
>    - Copy the full queue element (max_sge SGEs) instead of computing a
>      per-WQE copy size from num_sge. This ensures inline data (which
>      shares the flex array with SGEs) is always captured, and eliminates
>      a TOCTOU on the copy size itself.
>    - Clamp dma.num_sge against qp->sq.max_sge after copy (patch 2)
>      instead of against the global RXE_MAX_SGE constant.
>    - Invalidate the cached copy on QP reset (rxe_qp_reset), on the
>      ERR flush path, and on the RESET state check in the requester.
>      This prevents stale-cache reuse after state transitions.
>    - Each patch now also touches rxe_qp.c for the reset invalidation.
> 
> Changes since v2:
>    - Addressed review comments on naming and ordering.
>    - Writeback status using WRITE_ONCE() instead of plain store.
>    - Added smp_store_release() in requester for state transitions.
> 
> Changes since v1:
>    - Split into per-path patches (requester, completer).
>    - Cache the local copy across retransmits and multi-packet operations.
>    - Added writeback of completion status and rd_atomic state.
> 
> Tristan Madani (2):

Hi,

Please check the feedback from Sashiko.

Thanks a lot.
Yanjun Zhu

>    RDMA/rxe: copy send WQE to kernel buffer before processing
>    RDMA/rxe: copy send WQE to kernel buffer in completer path
> 
>   drivers/infiniband/sw/rxe/rxe_comp.c  | 53 +++++++++++++++++++++++++-
>   drivers/infiniband/sw/rxe/rxe_qp.c    |  2 +
>   drivers/infiniband/sw/rxe/rxe_req.c   | 55 +++++++++++++++++++++++++--
>   drivers/infiniband/sw/rxe/rxe_verbs.h | 12 ++++++
>   4 files changed, 116 insertions(+), 6 deletions(-)
> 

Re: [PATCH v4 0/2] RDMA/rxe: fix TOCTOU races in send WQE processing

[Tristan Madani]

Hi Yanjun,

Thanks for flagging the Sashiko review. I went through all 8 findings
across both patches.

One was a genuine issue: the memcpy size used qp->sq.max_sge * sizeof(sge),
which truncates when max_inline_data is not a multiple of sizeof(struct
ib_sge). For example, if a QP is created with max_inline_data=20, the
integer division gives max_sge=1, and only 16 bytes of the flex array are
copied instead of 20. v5 fixes this by using qp->sq.max_inline directly,
which is the exact data portion size stored at QP creation.

The remaining 7 seem to be false positives:

  - FORTIFY_SOURCE (raised on both patches): struct rxe_send_wqe ends
    with struct rxe_dma_info which contains __DECLARE_FLEX_ARRAY, so
    __builtin_object_size returns (size_t)-1. The merged receive-path
    fix (d6ab440240a04) uses the same pattern without issues.

  - Inline data / sge_offset / resid / cur_sge validation: these fields
    were already unvalidated against max_inline before this series. The
    requester path bounds-checks num_sge and cur_sge (lines 754-760 in
    rxe_req.c); adding resid/sge_offset validation is a valid hardening
    improvement, but it is a pre-existing gap, not introduced here.

  - wr_opcode_mask() OOB via untrusted opcode: the call existed at the
    same location before this series. We did not add it.

  - DMA state writeback for RDMA READ retry: on retry, the requester
    changes the WQE state via smp_store_release(). The completer's
    smp_load_acquire() detects the change, bypasses the reuse path, and
    takes a fresh copy from shared memory with the original DMA offsets.

  - ERR flush path barriers: when send_wqe_valid is false, writes go
    directly to shared memory, which is identical to the pre-patch
    behavior.

  - Shared pointer for wqe_state_posted: posted WQEs cause the
    completer to return COMPST_DONE/COMPST_EXIT without processing.

v5 is already sent with the max_inline fix:
  https://lore.kernel.org/linux-rdma/20261008093740.3034881-1-tristmd@gmail.com/

Best,
Tristan

Re: [PATCH v4 0/2] RDMA/rxe: fix TOCTOU races in send WQE processing

[Zhu Yanjun]

在 2026/10/8 5:01, Tristan Madani 写道:
> Hi Yanjun,
>
> Thanks for flagging the Sashiko review. I went through all 8 findings
> across both patches.
>
> One was a genuine issue: the memcpy size used qp->sq.max_sge * sizeof(sge),
> which truncates when max_inline_data is not a multiple of sizeof(struct
> ib_sge). For example, if a QP is created with max_inline_data=20, the
> integer division gives max_sge=1, and only 16 bytes of the flex array are
> copied instead of 20. v5 fixes this by using qp->sq.max_inline directly,
> which is the exact data portion size stored at QP creation.
>
> The remaining 7 seem to be false positives:
>
>    - FORTIFY_SOURCE (raised on both patches): struct rxe_send_wqe ends
>      with struct rxe_dma_info which contains __DECLARE_FLEX_ARRAY, so
>      __builtin_object_size returns (size_t)-1. The merged receive-path
>      fix (d6ab440240a04) uses the same pattern without issues.
>
>    - Inline data / sge_offset / resid / cur_sge validation: these fields
>      were already unvalidated against max_inline before this series. The
>      requester path bounds-checks num_sge and cur_sge (lines 754-760 in
>      rxe_req.c); adding resid/sge_offset validation is a valid hardening
>      improvement, but it is a pre-existing gap, not introduced here.
>
>    - wr_opcode_mask() OOB via untrusted opcode: the call existed at the
>      same location before this series. We did not add it.
>
>    - DMA state writeback for RDMA READ retry: on retry, the requester
>      changes the WQE state via smp_store_release(). The completer's
>      smp_load_acquire() detects the change, bypasses the reuse path, and
>      takes a fresh copy from shared memory with the original DMA offsets.
>
>    - ERR flush path barriers: when send_wqe_valid is false, writes go
>      directly to shared memory, which is identical to the pre-patch
>      behavior.
>
>    - Shared pointer for wqe_state_posted: posted WQEs cause the
>      completer to return COMPST_DONE/COMPST_EXIT without processing.
>
> v5 is already sent with the max_inline fix:
>    https://lore.kernel.org/linux-rdma/20261008093740.3034881-1-tristmd@gmail.com/

Thanks a lot. I checked the V5 patchset, and Sashiko is still 
complaining about quite a few things. I’m not sure whether the latest 
complaints from Sashiko are different from the previous ones or if they 
are essentially the same issues.

BTW, the V5 patchset is not a standalone patchset. Maybe there is 
something wrong with the way the patchset was sent out.

The following steps can be used to generate and send the latest patchset 
as a standalone patchset:

1. git format-patch -2 -n -s -o ./tmp --cover-letter -v 6
This command generates the patchset, including the cover letter, in the 
`./tmp` directory.

2. git send-email --to linux-rdma@vger.kernel.org --to xxx --to xxx ... 
./tmp/*
This command sends all the patches in ./tmp/ as a standalone patchset.

Thanks,

Yanjun Zhu

>
> Best,
> Tristan

-- 
Best Regards,
Yanjun.Zhu

[PATCH v5 0/2] RDMA/rxe: fix send-path TOCTOU races on shared WQEs

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The rxe driver maps send queues into userspace. Both the requester and
completer read Work Queue Entries (WQEs) directly from this shared
buffer. Userspace can modify WQE fields between kernel reads, causing
inconsistent state in copy_data() and related paths.

This series copies the send WQE to kernel-private buffers, mirroring
the receive-path fixes (commits 22b8fbded65b8 and d6ab440240a04).

Changes v4 -> v5:
  - Use qp->sq.max_inline instead of qp->sq.max_sge * sizeof(rxe_sge)
    for the copy size, avoiding integer division truncation when
    max_inline_data is not a multiple of sizeof(struct ib_sge)

Changes v3 -> v4:
  - Use full queue element size (max_sge SGEs) for the copy instead of
    per-WQE num_sge. Eliminates inline data gap, sizeof mismatch, and
    simplifies both patches
  - Invalidate requester copy on ERR flush path before writing status
    to shared memory (prevents writeback from clobbering error state)
  - Invalidate both caches on QP reset (rxe_qp.c changes added)

Changes v2 -> v3:
  - Add completer-path copy (patch 2/2) to close the remaining TOCTOU
    window. The completer was still reading directly from shared memory
  - Add smp_load_acquire()/smp_store_release() for state transitions
    between requester and completer

Changes v1 -> v2:
  - Added writeback mechanism using WRITE_ONCE() and smp_store_release()
  - Reuse kernel copy across multi-packet sends to preserve DMA state
  - Invalidate on retry, QP reset, and error paths

Tristan Madani (2):
  RDMA/rxe: copy send WQE to kernel buffer before processing
  RDMA/rxe: copy send WQE to kernel buffer in completer path

 drivers/infiniband/sw/rxe/rxe_comp.c  | 53 ++++++++++++++++++++++++++++--
 drivers/infiniband/sw/rxe/rxe_qp.c    |  2 ++
 drivers/infiniband/sw/rxe/rxe_req.c   | 55 ++++++++++++++++++++++++++++++---
 drivers/infiniband/sw/rxe/rxe_verbs.h | 12 ++++++++
 4 files changed, 116 insertions(+), 6 deletions(-)

-- 
2.39.5

[PATCH v5 1/2] RDMA/rxe: copy send WQE to kernel buffer before processing

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The rxe send queue is mapped into userspace via mmap. The requester
processes Work Queue Entries (WQEs) directly from this shared buffer
without first copying them to kernel memory. Userspace can modify WQE
fields (num_sge, sge_offset, SGE entries) between kernel reads,
leading to inconsistent state in copy_data().

This is the send-path counterpart to the receive-path fixes:
  - commit 22b8fbded65b8 ("RDMA/rxe: Fix TOCTOU heap overflow in
    get_srq_wqe")
  - commit d6ab440240a04 ("RDMA/rxe: Copy WQE to local buffer in
    non-SRQ receive path")

Fix by copying the send WQE to a kernel-private buffer in req_next_wqe()
before processing. The copy uses the full queue element data size
(max_inline bytes) so that inline data, which shares the flex array
with SGEs, is always captured regardless of num_sge.

The local copy is reused across multi-packet sends to preserve DMA
progress state (cur_sge, sge_offset, resid). Field updates are written
back to the shared queue using WRITE_ONCE() for individual fields and
smp_store_release() for state transitions, so the completer and
userspace observe consistent values without the tearing risk of bulk
memcpy().

The copy is invalidated when the WQE index advances (last packet sent,
error, local ops, UD oversized), when a retry resets WQE state, on QP
reset, or when the QP enters error state for flush.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 drivers/infiniband/sw/rxe/rxe_qp.c    |  1 +
 drivers/infiniband/sw/rxe/rxe_req.c   | 55 +++++++++++++++++++++++++--
 drivers/infiniband/sw/rxe/rxe_verbs.h |  6 +++
 3 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index 311f285d78a6b..77606c4a039b1 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -581,6 +581,7 @@ static void rxe_qp_reset(struct rxe_qp *qp)
 	qp->req.need_retry = 0;
 	qp->req.wait_for_rnr_timer = 0;
 	qp->req.noack_pkts = 0;
+	qp->req.send_wqe_valid = false;
 	qp->resp.msn = 0;
 	qp->resp.opcode = -1;
 	qp->resp.drop_msg = 0;
diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 24f5c044363f7..ab371f2d7791a 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -161,6 +161,26 @@ static void req_check_sq_drain_done(struct rxe_qp *qp)
 	spin_unlock_irqrestore(&qp->state_lock, flags);
 }
 
+/* Write back requester WQE fields to shared memory using targeted
+ * stores so the completer and userspace observe consistent state.
+ */
+static void rxe_req_writeback_wqe(struct rxe_qp *qp)
+{
+	struct rxe_send_wqe *shared = qp->req.shared_wqe;
+	struct rxe_send_wqe *local = &qp->req.send_wqe.wqe;
+
+	if (!qp->req.send_wqe_valid || !shared)
+		return;
+
+	WRITE_ONCE(shared->status, local->status);
+	WRITE_ONCE(shared->first_psn, local->first_psn);
+	WRITE_ONCE(shared->last_psn, local->last_psn);
+	WRITE_ONCE(shared->mask, local->mask);
+	WRITE_ONCE(shared->has_rd_atomic, local->has_rd_atomic);
+	/* State must be last so the completer sees prior updates */
+	smp_store_release(&shared->state, local->state);
+}
+
 static struct rxe_send_wqe *__req_next_wqe(struct rxe_qp *qp)
 {
 	struct rxe_queue *q = qp->sq.queue;
@@ -193,6 +213,20 @@ static struct rxe_send_wqe *req_next_wqe(struct rxe_qp *qp)
 	}
 	spin_unlock_irqrestore(&qp->state_lock, flags);
 
+	/* Reuse the existing kernel-private copy if still valid */
+	if (qp->req.send_wqe_valid && qp->req.shared_wqe == wqe)
+		return &qp->req.send_wqe.wqe;
+
+	/* Copy WQE from userspace-mapped shared queue to kernel-private
+	 * buffer. Use max_inline as copy size since it covers both SGEs
+	 * and inline data, which share the flex array.
+	 */
+	memcpy(&qp->req.send_wqe.wqe, wqe,
+	       sizeof(*wqe) + qp->sq.max_inline);
+	qp->req.shared_wqe = wqe;
+	qp->req.send_wqe_valid = true;
+
+	wqe = &qp->req.send_wqe.wqe;
 	wqe->mask = wr_opcode_mask(wqe->wr.opcode, qp);
 	return wqe;
 }
@@ -582,9 +616,13 @@ static void update_state(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
 {
 	qp->req.opcode = pkt->opcode;
 
-	if (pkt->mask & RXE_END_MASK)
+	rxe_req_writeback_wqe(qp);
+
+	if (pkt->mask & RXE_END_MASK) {
 		qp->req.wqe_index = queue_next_index(qp->sq.queue,
 						     qp->req.wqe_index);
+		qp->req.send_wqe_valid = false;
+	}
 
 	qp->need_req_skb = 0;
 
@@ -634,7 +672,9 @@ static int rxe_do_local_ops(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
 
 	wqe->state = wqe_state_done;
 	wqe->status = IB_WC_SUCCESS;
+	rxe_req_writeback_wqe(qp);
 	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
+	qp->req.send_wqe_valid = false;
 
 	return 0;
 }
@@ -666,6 +706,7 @@ int rxe_requester(struct rxe_qp *qp)
 		wqe = __req_next_wqe(qp);
 		spin_unlock_irqrestore(&qp->state_lock, flags);
 		if (wqe) {
+			qp->req.send_wqe_valid = false;
 			wqe->status = IB_WC_WR_FLUSH_ERR;
 			goto err;
 		} else {
@@ -681,6 +722,7 @@ int rxe_requester(struct rxe_qp *qp)
 		qp->req.wait_psn = 0;
 		qp->req.need_retry = 0;
 		qp->req.wait_for_rnr_timer = 0;
+		qp->req.send_wqe_valid = false;
 		spin_unlock_irqrestore(&qp->state_lock, flags);
 		goto exit;
 	}
@@ -695,6 +737,7 @@ int rxe_requester(struct rxe_qp *qp)
 	if (unlikely(qp->req.need_retry && !qp->req.wait_for_rnr_timer)) {
 		req_retry(qp);
 		qp->req.need_retry = 0;
+		qp->req.send_wqe_valid = false;
 	}
 
 	wqe = req_next_wqe(qp);
@@ -772,10 +815,12 @@ int rxe_requester(struct rxe_qp *qp)
 			wqe->last_psn = qp->req.psn;
 			qp->req.psn = (qp->req.psn + 1) & BTH_PSN_MASK;
 			qp->req.opcode = IB_OPCODE_UD_SEND_ONLY;
-			qp->req.wqe_index = queue_next_index(qp->sq.queue,
-						       qp->req.wqe_index);
 			wqe->state = wqe_state_done;
 			wqe->status = IB_WC_SUCCESS;
+			rxe_req_writeback_wqe(qp);
+			qp->req.wqe_index = queue_next_index(qp->sq.queue,
+						       qp->req.wqe_index);
+			qp->req.send_wqe_valid = false;
 			goto done;
 		}
 		payload = mtu;
@@ -839,8 +884,10 @@ int rxe_requester(struct rxe_qp *qp)
 	goto out;
 err:
 	/* update wqe_index for each wqe completion */
-	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
 	wqe->state = wqe_state_error;
+	rxe_req_writeback_wqe(qp);
+	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
+	qp->req.send_wqe_valid = false;
 	rxe_qp_error(qp);
 exit:
 	ret = -EAGAIN;
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index 0f5ffd94643f9..a22dfc6e5ae3c 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -114,6 +114,12 @@ struct rxe_req_info {
 	int			wait_for_rnr_timer;
 	int			noack_pkts;
 	int			again;
+	struct rxe_send_wqe	*shared_wqe;
+	bool			send_wqe_valid;
+	struct {
+		struct rxe_send_wqe	wqe;
+		struct ib_sge		sge[RXE_MAX_SGE];
+	} send_wqe;
 };
 
 struct rxe_comp_info {
-- 
2.47.3

Re: [PATCH v5 1/2] RDMA/rxe: copy send WQE to kernel buffer before processing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] rxe: kernel heap leak via unvalidated inline WQE fields
- [High] rxe: out-of-bounds read in wr_opcode_mask() via untrusted userspace WQE opcode
- [Medium] rdma/rxe: Missing memory barrier in IB_QPS_ERR flush path violates concurrency model
--

commit ffc0feb820a2586eb777f9f2b8fd3b594b8c06e6
Author: Tristan Madani <tristan@talencesecurity.com>

RDMA/rxe: copy send WQE to kernel buffer before processing

The rxe send queue is mapped into userspace via mmap. The requester
processes Work Queue Entries (WQEs) directly from this shared buffer
without first copying them to kernel memory. Userspace can modify WQE
fields (num_sge, sge_offset, SGE entries) between kernel reads,
leading to inconsistent state in copy_data().

> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index 53f7c1e7fe55b..5bd6bcad2ab97 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c

[ ... ]

> @@ -193,6 +213,20 @@ static struct rxe_send_wqe *req_next_wqe(struct rxe_qp *qp)
>  	}
>  	spin_unlock_irqrestore(&qp->state_lock, flags);
>  
> +	/* Reuse the existing kernel-private copy if still valid */
> +	if (qp->req.send_wqe_valid && qp->req.shared_wqe == wqe)
> +		return &qp->req.send_wqe.wqe;
> +
> +	/* Copy WQE from userspace-mapped shared queue to kernel-private
> +	 * buffer. Use max_inline as copy size since it covers both SGEs
> +	 * and inline data, which share the flex array.
> +	 */
> +	memcpy(&qp->req.send_wqe.wqe, wqe,
> +	       sizeof(*wqe) + qp->sq.max_inline);
> +	qp->req.shared_wqe = wqe;
> +	qp->req.send_wqe_valid = true;
> +
> +	wqe = &qp->req.send_wqe.wqe;
>  	wqe->mask = wr_opcode_mask(wqe->wr.opcode, qp);

[Severity: High]
Does this code perform an out-of-bounds read of rxe_wr_opcode_info[]?

The opcode in the WQE comes directly from userspace without validation. It is
passed to wr_opcode_mask():

drivers/infiniband/sw/rxe/rxe_loc.h:wr_opcode_mask() {
    return rxe_wr_opcode_info[opcode].mask[qp->ibqp.qp_type];
}

If a malicious user provides an out-of-bounds opcode (e.g. 0xFFFFFFFF), could
it read unmapped kernel memory or use garbage memory as an opcode mask?

[ ... ]

> @@ -666,6 +706,7 @@ int rxe_requester(struct rxe_qp *qp)
>  		wqe = __req_next_wqe(qp);
>  		spin_unlock_irqrestore(&qp->state_lock, flags);
>  		if (wqe) {
> +			qp->req.send_wqe_valid = false;
>  			wqe->status = IB_WC_WR_FLUSH_ERR;
>  			goto err;

[Severity: Medium]
Does this bypass the required memory barriers for updating the WQE in shared
memory?

Since qp->req.send_wqe_valid is set to false here, the subsequent call to
rxe_req_writeback_wqe() at the err label will be a no-op. The WQE state and
status are directly modified on the shared memory WQE:

err:
	wqe->state = wqe_state_error;
	rxe_req_writeback_wqe(qp);

Without the intended smp_store_release() or WRITE_ONCE() macros, could
the completer or userspace observe wqe_state_error before the status update
is visible?

>  		} else {
>  			goto exit;
>  		}

[ ... ]

> @@ -695,6 +737,7 @@ int rxe_requester(struct rxe_qp *qp)
>  	if (unlikely(qp->req.need_retry && !qp->req.wait_for_rnr_timer)) {
>  		req_retry(qp);
>  		qp->req.need_retry = 0;
> +		qp->req.send_wqe_valid = false;
>  	}
>  
>  	wqe = req_next_wqe(qp);

[Severity: High]
Does this code allow an out-of-bounds read of the kernel heap memory when
processing IB_SEND_INLINE WQEs?

Just below this in rxe_requester(), num_sge and cur_sge are bounds checked,
but sge_offset and resid are not validated against qp->sq.max_inline:

drivers/infiniband/sw/rxe/rxe_req.c:rxe_requester() {
	...
	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");
	...
}

Because this patch relocates the WQE into a kmalloc'd kernel buffer, can
a malicious user supply large values for sge_offset and resid to leak
adjacent kernel heap memory when the payload is copied to the network packet?

drivers/infiniband/sw/rxe/rxe_req.c:finish_packet() {
        ...
		if (wqe->wr.send_flags & IB_SEND_INLINE) {
			u8 *tmp = &wqe->dma.inline_data[wqe->dma.sge_offset];

			memcpy(payload_addr(pkt), tmp, payload);
        ...
}

[ ... ]

> @@ -839,8 +884,10 @@ int rxe_requester(struct rxe_qp *qp)
>  	goto out;
>  err:
>  	/* update wqe_index for each wqe completion */
> -	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
>  	wqe->state = wqe_state_error;
> +	rxe_req_writeback_wqe(qp);
> +	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
> +	qp->req.send_wqe_valid = false;
>  	rxe_qp_error(qp);
>  exit:
>  	ret = -EAGAIN;

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

[PATCH v5 2/2] RDMA/rxe: copy send WQE to kernel buffer in completer path

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The completer reads send WQEs from the same userspace-mapped shared
queue as the requester. Even after the requester copies the WQE to a
kernel-private buffer (previous patch), the completer still reads
directly from shared memory, leaving it exposed to the same TOCTOU
races.

Fix by copying the WQE in get_wqe() to a completer-private buffer
using the full queue element data size (max_inline bytes). The local
copy is reused when the same WQE is being processed with unchanged
state, which preserves DMA progress across multi-packet RDMA READ
responses. A fresh copy is taken when the state changes or a new WQE
appears.

State transitions from the requester are observed through an
smp_load_acquire() / smp_store_release() pair. Completion status and
rd_atomic state are written back through WRITE_ONCE() before the
consumer index advances.

The copy is invalidated on QP reset and when the completion advances
to the next WQE.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 drivers/infiniband/sw/rxe/rxe_comp.c  | 53 ++++++++++++++++++++++++++-
 drivers/infiniband/sw/rxe/rxe_qp.c    |  1 +
 drivers/infiniband/sw/rxe/rxe_verbs.h |  6 +++
 3 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
index 1390e861bd1d7..5d8b692114a33 100644
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -137,22 +137,69 @@ void rxe_comp_queue_pkt(struct rxe_qp *qp, struct sk_buff *skb)
 	rxe_sched_task(&qp->send_task);
 }
 
+/* Write back completer WQE fields to shared memory */
+static void rxe_comp_writeback_wqe(struct rxe_qp *qp)
+{
+	struct rxe_send_wqe *shared = qp->comp.shared_wqe;
+	struct rxe_send_wqe *local = &qp->comp.comp_wqe.wqe;
+
+	if (!qp->comp.comp_wqe_valid || !shared)
+		return;
+
+	WRITE_ONCE(shared->status, local->status);
+	WRITE_ONCE(shared->has_rd_atomic, local->has_rd_atomic);
+}
+
 static inline enum comp_state get_wqe(struct rxe_qp *qp,
 				      struct rxe_pkt_info *pkt,
 				      struct rxe_send_wqe **wqe_p)
 {
 	struct rxe_send_wqe *wqe;
+	u32 state;
 
 	/* we come here whether or not we found a response packet to see if
 	 * there are any posted WQEs
 	 */
 	wqe = queue_head(qp->sq.queue, QUEUE_TYPE_FROM_CLIENT);
-	*wqe_p = wqe;
 
 	/* no WQE or requester has not started it yet */
-	if (!wqe || wqe->state == wqe_state_posted)
+	if (!wqe) {
+		*wqe_p = NULL;
 		return pkt ? COMPST_DONE : COMPST_EXIT;
+	}
+
+	/* Pairs with smp_store_release() in rxe_req_writeback_wqe() */
+	state = smp_load_acquire(&wqe->state);
+	if (state == wqe_state_posted) {
+		*wqe_p = wqe;
+		return pkt ? COMPST_DONE : COMPST_EXIT;
+	}
+
+	/* Reuse existing local copy if still processing same WQE
+	 * with unchanged state (preserves DMA progress for multi-packet ops)
+	 */
+	if (qp->comp.comp_wqe_valid && qp->comp.shared_wqe == wqe &&
+	    qp->comp.comp_wqe.wqe.state == state) {
+		wqe = &qp->comp.comp_wqe.wqe;
+		*wqe_p = wqe;
+		goto check_state;
+	}
+
+	/* Copy shared WQE to kernel-private buffer. Use max_inline
+	 * as copy size since it covers both SGEs and inline data,
+	 * which share the flex array.
+	 */
+	qp->comp.shared_wqe = wqe;
+	memcpy(&qp->comp.comp_wqe.wqe, wqe,
+	       sizeof(*wqe) + qp->sq.max_inline);
+	qp->comp.comp_wqe_valid = true;
+	if (qp->comp.comp_wqe.wqe.dma.num_sge > qp->sq.max_sge)
+		qp->comp.comp_wqe.wqe.dma.num_sge = qp->sq.max_sge;
+
+	wqe = &qp->comp.comp_wqe.wqe;
+	*wqe_p = wqe;
 
+check_state:
 	/* WQE does not require an ack */
 	if (wqe->state == wqe_state_done)
 		return COMPST_COMP_WQE;
@@ -454,6 +501,8 @@ static void do_complete(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
 	if (post)
 		make_send_cqe(qp, wqe, &cqe);
 
+	rxe_comp_writeback_wqe(qp);
+	qp->comp.comp_wqe_valid = false;
 	queue_advance_consumer(qp->sq.queue, QUEUE_TYPE_FROM_CLIENT);
 
 	if (post)
diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index 77606c4a039b1..450254a131017 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -582,6 +582,7 @@ static void rxe_qp_reset(struct rxe_qp *qp)
 	qp->req.wait_for_rnr_timer = 0;
 	qp->req.noack_pkts = 0;
 	qp->req.send_wqe_valid = false;
+	qp->comp.comp_wqe_valid = false;
 	qp->resp.msn = 0;
 	qp->resp.opcode = -1;
 	qp->resp.drop_msg = 0;
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index a22dfc6e5ae3c..6205dbc29dff6 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -130,6 +130,12 @@ struct rxe_comp_info {
 	int			started_retry;
 	u32			retry_cnt;
 	u32			rnr_retry;
+	struct rxe_send_wqe	*shared_wqe;
+	bool			comp_wqe_valid;
+	struct {
+		struct rxe_send_wqe	wqe;
+		struct ib_sge		sge[RXE_MAX_SGE];
+	} comp_wqe;
 };
 
 /* responder states */
-- 
2.47.3

Re: [PATCH v5 2/2] RDMA/rxe: copy send WQE to kernel buffer in completer path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] RDMA/rxe: Out-of-bounds array access due to unvalidated dma.cur_sge in copied WQE
- [High] RDMA/rxe: Incomplete TOCTOU fix leaks shared WQE pointer when wqe_state_posted
- [High] RDMA/rxe: Broken RDMA READ retry logic due to omitted dma.resid writeback
- [High] rdma/rxe: FORTIFY_SOURCE field-spanning write warning in get_wqe()
--

commit cf86a77297c64ab74c16b59fe4e3de0489c867e4
Author: Tristan Madani <tristan@talencesecurity.com>

RDMA/rxe: copy send WQE to kernel buffer in completer path

This commit attempts to fix TOCTOU races by copying send WQEs from the
shared queue to a kernel-private buffer in the completer path. It
introduces local copies of the WQE to preserve DMA progress and updates
writeback logic before advancing the consumer index.

> diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
> index 1390e861bd1d7..5d8b692114a33 100644
> --- a/drivers/infiniband/sw/rxe/rxe_comp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_comp.c
[ ... ]
> +/* Write back completer WQE fields to shared memory */
> +static void rxe_comp_writeback_wqe(struct rxe_qp *qp)
> +{
> +	struct rxe_send_wqe *shared = qp->comp.shared_wqe;
> +	struct rxe_send_wqe *local = &qp->comp.comp_wqe.wqe;
> +
> +	if (!qp->comp.comp_wqe_valid || !shared)
> +		return;
> +
> +	WRITE_ONCE(shared->status, local->status);
> +	WRITE_ONCE(shared->has_rd_atomic, local->has_rd_atomic);

[Severity: High]
Does this code fail to write back dma.resid and dma.sge_offset?

If a timeout occurs, the requester's retry logic (req_retry()) reads the
unmodified dma.resid from the shared WQE (or its own unmodified private
copy) to calculate the retry offset.

This causes it to retry from the beginning (offset 0) but with an advanced
PSN, resulting in the completer writing redundant remote data into advanced
local offsets.

> +}
> +
>  static inline enum comp_state get_wqe(struct rxe_qp *qp,
>  				      struct rxe_pkt_info *pkt,
>  				      struct rxe_send_wqe **wqe_p)
>  {
>  	struct rxe_send_wqe *wqe;
> +	u32 state;
>  
>  	/* we come here whether or not we found a response packet to see if
>  	 * there are any posted WQEs
>  	 */
>  	wqe = queue_head(qp->sq.queue, QUEUE_TYPE_FROM_CLIENT);
>  
>  	/* no WQE or requester has not started it yet */
> -	if (!wqe || wqe->state == wqe_state_posted)
> +	if (!wqe) {
> +		*wqe_p = NULL;
>  		return pkt ? COMPST_DONE : COMPST_EXIT;
> +	}
> +
> +	/* Pairs with smp_store_release() in rxe_req_writeback_wqe() */
> +	state = smp_load_acquire(&wqe->state);
> +	if (state == wqe_state_posted) {
> +		*wqe_p = wqe;

[Severity: High]
Does this code assign the userspace-mapped shared memory pointer to *wqe_p
instead of forcing a local copy or returning NULL?

If the completer drops into COMPST_ERROR_RETRY (e.g., following a timeout),
it evaluates wqe->state == wqe_state_posted using this shared pointer.

Userspace can concurrently modify wqe->state to bypass this check, causing
the completer to erroneously process un-started WQEs.

> +		return pkt ? COMPST_DONE : COMPST_EXIT;
> +	}
> +
> +	/* Reuse existing local copy if still processing same WQE
> +	 * with unchanged state (preserves DMA progress for multi-packet ops)
> +	 */
> +	if (qp->comp.comp_wqe_valid && qp->comp.shared_wqe == wqe &&
> +	    qp->comp.comp_wqe.wqe.state == state) {
> +		wqe = &qp->comp.comp_wqe.wqe;
> +		*wqe_p = wqe;
> +		goto check_state;
> +	}
> +
> +	/* Copy shared WQE to kernel-private buffer. Use max_inline
> +	 * as copy size since it covers both SGEs and inline data,
> +	 * which share the flex array.
> +	 */
> +	qp->comp.shared_wqe = wqe;
> +	memcpy(&qp->comp.comp_wqe.wqe, wqe,
> +	       sizeof(*wqe) + qp->sq.max_inline);

[Severity: High]
Can this memcpy trigger a FORTIFY_SOURCE field-spanning write warning?

This memcpy targets the wqe sub-object but copies a length that spans into
the adjacent sge field.

The compiler's bounds check restricts the write destination field to exactly
sizeof(*wqe), so this triggers a fortify warning (and panic if panic_on_warn
is enabled) when max_inline > 0.

> +	qp->comp.comp_wqe_valid = true;
> +	if (qp->comp.comp_wqe.wqe.dma.num_sge > qp->sq.max_sge)
> +		qp->comp.comp_wqe.wqe.dma.num_sge = qp->sq.max_sge;

[Severity: Critical]
Does this code neglect to validate cur_sge after the copy from userspace?

A malicious userspace application can set cur_sge to an arbitrarily large
value.

During RDMA read or Atomic response processing, copy_data() indexes the
dma->sge array using this unvalidated cur_sge, accessing memory outside
the comp_wqe buffer.

> +
> +	wqe = &qp->comp.comp_wqe.wqe;
> +	*wqe_p = wqe;
>  
> +check_state:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008093740.3034881-1-tristmd@gmail.com?part=2