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

[PATCH 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 local copy is reused across multi-packet sends
to preserve DMA progress state (cur_sge, sge_offset, resid). State
changes are written back to the shared queue so the completer can observe
them (e.g., wqe_state_pending for RC acknowledgment processing).

The copy is invalidated when:
 - The WQE index advances (last packet sent, error, local ops, UD oversized)
 - A retry occurs (req_retry resets WQE state in shared memory)

The num_sge field is validated against qp->sq.max_sge on copy-in to
reject corrupted values early.

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

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 12d03f390b097..499f398b674c7 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -161,6 +161,20 @@ static void req_check_sq_drain_done(struct rxe_qp *qp)
 	spin_unlock_irqrestore(&qp->state_lock, flags);
 }
 
+/* Write local WQE copy back to shared queue so the completer
+ * can observe state transitions (e.g., wqe_state_pending).
+ */
+static void rxe_req_writeback_wqe(struct rxe_qp *qp)
+{
+	if (qp->req.send_wqe_valid && qp->req.shared_wqe) {
+		unsigned int num_sge = qp->req.send_wqe.wqe.dma.num_sge;
+		size_t size = sizeof(struct rxe_send_wqe) +
+			      num_sge * sizeof(struct rxe_sge);
+
+		memcpy(qp->req.shared_wqe, &qp->req.send_wqe.wqe, size);
+	}
+}
+
 static struct rxe_send_wqe *__req_next_wqe(struct rxe_qp *qp)
 {
 	struct rxe_queue *q = qp->sq.queue;
@@ -178,6 +192,8 @@ static struct rxe_send_wqe *req_next_wqe(struct rxe_qp *qp)
 {
 	struct rxe_send_wqe *wqe;
 	unsigned long flags;
+	unsigned int num_sge;
+	size_t copy_size;
 
 	req_check_sq_drain_done(qp);
 
@@ -193,6 +209,29 @@ static struct rxe_send_wqe *req_next_wqe(struct rxe_qp *qp)
 	}
 	spin_unlock_irqrestore(&qp->state_lock, flags);
 
+	/* If we already have a valid local copy of this WQE, use it.
+	 * This preserves DMA progress state across multi-packet sends.
+	 */
+	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. Userspace can concurrently modify num_sge, SGE entries,
+	 * or inline data offsets, leading to inconsistent state in
+	 * copy_data(). This is the send-path variant of the receive-path
+	 * fix in rxe_get_recv_wqe().
+	 */
+	num_sge = wqe->dma.num_sge;
+	if (unlikely(num_sge > qp->sq.max_sge)) {
+		rxe_dbg_qp(qp, "invalid num_sge in send WQE\n");
+		return NULL;
+	}
+	copy_size = sizeof(*wqe) + num_sge * sizeof(struct rxe_sge);
+	memcpy(&qp->req.send_wqe.wqe, wqe, copy_size);
+	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 +621,14 @@ static void update_state(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
 {
 	qp->req.opcode = pkt->opcode;
 
-	if (pkt->mask & RXE_END_MASK)
+	/* Write back local WQE state before possibly advancing index */
+	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;
 
@@ -635,6 +679,7 @@ 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;
 	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
+	qp->req.send_wqe_valid = false;
 
 	return 0;
 }
@@ -695,6 +740,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);
@@ -761,6 +807,7 @@ int rxe_requester(struct rxe_qp *qp)
 						       qp->req.wqe_index);
 			wqe->state = wqe_state_done;
 			wqe->status = IB_WC_SUCCESS;
+			qp->req.send_wqe_valid = false;
 			goto done;
 		}
 		payload = mtu;
@@ -820,12 +867,18 @@ int rxe_requester(struct rxe_qp *qp)
 	 * will continue looping and return to rxe_requester
 	 */
 done:
+	/* Write back local WQE for paths that skip update_state()
+	 * (local ops, UD oversized packets).
+	 */
+	rxe_req_writeback_wqe(qp);
 	ret = 0;
 	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.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

[PATCH 2/2] RDMA/rxe: copy completer WQE to kernel buffer before processing

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The completer reads WQE fields (dma.num_sge, dma.sge[], dma.cur_sge,
wr.opcode, first_psn, last_psn, state, status) directly from the
userspace-mapped shared send queue via queue_head(). Userspace can
modify these fields between completer reads, causing inconsistent
state in do_read() and do_atomic() which call copy_data().

This is the completer-path counterpart to the preceding patch which
fixed the requester path. Although rxe_sender() calls the requester
and completer sequentially (not concurrently), the requester may have
already advanced wqe_index and invalidated its kernel-private copy by
the time the completer processes the READ/ATOMIC response, so the
completer needs its own copy.

Fix by:
 1. Adding a kernel-private WQE copy buffer (comp_wqe) to rxe_comp_info
 2. Copying the WQE from shared memory in get_wqe() before any
    processing, with num_sge validation against qp->sq.max_sge
 3. Writing back status/state changes to the shared queue entry in
    do_complete() so CQE generation and retry logic see the correct
    completion status

The flush_send_queue() error path is not modified as it runs during
QP teardown (ERR/RESET state) and does not call copy_data().

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  | 36 +++++++++++++++++++++++++--
 drivers/infiniband/sw/rxe/rxe_verbs.h |  5 ++++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
index 1390e861bd1d7..0f647721a657e 100644
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -142,16 +142,40 @@ static inline enum comp_state get_wqe(struct rxe_qp *qp,
 				      struct rxe_send_wqe **wqe_p)
 {
 	struct rxe_send_wqe *wqe;
+	unsigned int num_sge;
+	size_t copy_size;
 
 	/* 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->state == wqe_state_posted) {
+		*wqe_p = NULL;
 		return pkt ? COMPST_DONE : COMPST_EXIT;
+	}
+
+	/* Copy WQE from userspace-mapped shared queue to kernel-private
+	 * buffer. Userspace can concurrently modify DMA state (num_sge,
+	 * sge[], cur_sge), leading to inconsistent state in do_read()
+	 * and do_atomic(). This is the completer-path counterpart to
+	 * the requester-path fix.
+	 */
+	num_sge = wqe->dma.num_sge;
+	if (unlikely(num_sge > qp->sq.max_sge)) {
+		rxe_dbg_qp(qp, "invalid num_sge in send WQE (comp)\n");
+		memcpy(&qp->comp.comp_wqe.wqe, wqe, sizeof(*wqe));
+		qp->comp.comp_wqe.wqe.status = IB_WC_LOC_LEN_ERR;
+		qp->comp.shared_wqe = wqe;
+		*wqe_p = &qp->comp.comp_wqe.wqe;
+		return COMPST_ERROR;
+	}
+	copy_size = sizeof(*wqe) + num_sge * sizeof(struct rxe_sge);
+	memcpy(&qp->comp.comp_wqe.wqe, wqe, copy_size);
+	qp->comp.shared_wqe = wqe;
+	*wqe_p = &qp->comp.comp_wqe.wqe;
+	wqe = *wqe_p;
 
 	/* WQE does not require an ack */
 	if (wqe->state == wqe_state_done)
@@ -446,6 +470,14 @@ static void do_complete(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
 	struct rxe_cqe cqe;
 	bool post;
 
+	/* Write back status to the shared queue entry so the CQE
+	 * and any retry logic sees the correct completion status.
+	 */
+	if (qp->comp.shared_wqe) {
+		qp->comp.shared_wqe->status = wqe->status;
+		qp->comp.shared_wqe->state = wqe->state;
+	}
+
 	/* do we need to post a completion */
 	post = ((qp->sq_sig_type == IB_SIGNAL_ALL_WR) ||
 			(wqe->wr.send_flags & IB_SEND_SIGNALED) ||
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index a22dfc6e5ae3c..f1d647ddc1e05 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -130,6 +130,11 @@ struct rxe_comp_info {
 	int			started_retry;
 	u32			retry_cnt;
 	u32			rnr_retry;
+	struct rxe_send_wqe	*shared_wqe;
+	struct {
+		struct rxe_send_wqe	wqe;
+		struct ib_sge		sge[RXE_MAX_SGE];
+	} comp_wqe;
 };
 
 /* responder states */
-- 
2.47.3

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

[Zhu Yanjun]

在 2026/8/16 3:44, 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 local copy is reused across multi-packet sends
> to preserve DMA progress state (cur_sge, sge_offset, resid). State
> changes are written back to the shared queue so the completer can observe
> them (e.g., wqe_state_pending for RC acknowledgment processing).
>
> The copy is invalidated when:
>   - The WQE index advances (last packet sent, error, local ops, UD oversized)
>   - A retry occurs (req_retry resets WQE state in shared memory)
>
> The num_sge field is validated against qp->sq.max_sge on copy-in to
> reject corrupted values early.
>
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
>   drivers/infiniband/sw/rxe/rxe_req.c   | 55 ++++++++++++++++++++++++++-
>   drivers/infiniband/sw/rxe/rxe_verbs.h |  6 +++
>   2 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index 12d03f390b097..499f398b674c7 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c
> @@ -161,6 +161,20 @@ static void req_check_sq_drain_done(struct rxe_qp *qp)
>   	spin_unlock_irqrestore(&qp->state_lock, flags);
>   }
>   
> +/* Write local WQE copy back to shared queue so the completer
> + * can observe state transitions (e.g., wqe_state_pending).
> + */
> +static void rxe_req_writeback_wqe(struct rxe_qp *qp)
> +{
> +	if (qp->req.send_wqe_valid && qp->req.shared_wqe) {
> +		unsigned int num_sge = qp->req.send_wqe.wqe.dma.num_sge;
> +		size_t size = sizeof(struct rxe_send_wqe) +
> +			      num_sge * sizeof(struct rxe_sge);
> +
> +		memcpy(qp->req.shared_wqe, &qp->req.send_wqe.wqe, size);
> +	}
> +}
> +
rxe_req_writeback_wqe() uses memcpy() to write the local WQE state back
to qp->req.shared_wqe in user-mapped memory without synchronization.

This introduces two data race hazards:
1. Malicious or concurrent userspace writes during memcpy() can race
    with the kernel writeback, causing tearing or inconsistent shared state.
2. The kernel completer tasklet concurrently reads shared_wqe without
    holding a lock, risking torn reads of intermediate WQE states (e.g.
    wqe_state_pending) and triggering state machine corruption.

Writebacks to shared user-mapped memory must update state fields atomically
or execute under appropriate lock serialization rather than performing an

uncontrolled bulk memcpy().

Please fix the above problem with appropriate synchronize methods.

Thanks a lot.

Yanjun.Zhu

>   static struct rxe_send_wqe *__req_next_wqe(struct rxe_qp *qp)
>   {
>   	struct rxe_queue *q = qp->sq.queue;
> @@ -178,6 +192,8 @@ static struct rxe_send_wqe *req_next_wqe(struct rxe_qp *qp)
>   {
>   	struct rxe_send_wqe *wqe;
>   	unsigned long flags;
> +	unsigned int num_sge;
> +	size_t copy_size;
>   
>   	req_check_sq_drain_done(qp);
>   
> @@ -193,6 +209,29 @@ static struct rxe_send_wqe *req_next_wqe(struct rxe_qp *qp)
>   	}
>   	spin_unlock_irqrestore(&qp->state_lock, flags);
>   
> +	/* If we already have a valid local copy of this WQE, use it.
> +	 * This preserves DMA progress state across multi-packet sends.
> +	 */
> +	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. Userspace can concurrently modify num_sge, SGE entries,
> +	 * or inline data offsets, leading to inconsistent state in
> +	 * copy_data(). This is the send-path variant of the receive-path
> +	 * fix in rxe_get_recv_wqe().
> +	 */
> +	num_sge = wqe->dma.num_sge;
> +	if (unlikely(num_sge > qp->sq.max_sge)) {
> +		rxe_dbg_qp(qp, "invalid num_sge in send WQE\n");
> +		return NULL;
> +	}
> +	copy_size = sizeof(*wqe) + num_sge * sizeof(struct rxe_sge);
> +	memcpy(&qp->req.send_wqe.wqe, wqe, copy_size);
> +	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 +621,14 @@ static void update_state(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
>   {
>   	qp->req.opcode = pkt->opcode;
>   
> -	if (pkt->mask & RXE_END_MASK)
> +	/* Write back local WQE state before possibly advancing index */
> +	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;
>   
> @@ -635,6 +679,7 @@ 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;
>   	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
> +	qp->req.send_wqe_valid = false;
>   
>   	return 0;
>   }
> @@ -695,6 +740,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);
> @@ -761,6 +807,7 @@ int rxe_requester(struct rxe_qp *qp)
>   						       qp->req.wqe_index);
>   			wqe->state = wqe_state_done;
>   			wqe->status = IB_WC_SUCCESS;
> +			qp->req.send_wqe_valid = false;
>   			goto done;
>   		}
>   		payload = mtu;
> @@ -820,12 +867,18 @@ int rxe_requester(struct rxe_qp *qp)
>   	 * will continue looping and return to rxe_requester
>   	 */
>   done:
> +	/* Write back local WQE for paths that skip update_state()
> +	 * (local ops, UD oversized packets).
> +	 */
> +	rxe_req_writeback_wqe(qp);
>   	ret = 0;
>   	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.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 {

-- 
Best Regards,
Yanjun.Zhu

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

[Tristan Madani]

Hi Yanjun,

Thank you for the review.

Regarding concern 2: unless I am mistaken, rxe_sender() calls
rxe_requester() then rxe_completer() sequentially in the same
work item (rxe_req.c:836), and do_task() prevents re-entry, so
the requester and completer should not run concurrently on the
same QP. But please correct me if there is a scheduling path
I missed.

Either way, using WRITE_ONCE() for the state and status fields
instead of a bulk memcpy() is cleaner and eliminates any concern
about tearing from concurrent userspace writes (concern 1). I
will send v2 with targeted WRITE_ONCE() writebacks for just the
fields that need to be visible, dropping the bulk memcpy.

Best regards,
Tristan Madani

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

[Zhu Yanjun]

在 2026/8/18 6:33, Tristan Madani 写道:
> Hi Yanjun,
> 
> Thank you for the review.
> 
> Regarding concern 2: unless I am mistaken, rxe_sender() calls
> rxe_requester() then rxe_completer() sequentially in the same
> work item (rxe_req.c:836), and do_task() prevents re-entry, so
> the requester and completer should not run concurrently on the
> same QP. But please correct me if there is a scheduling path
> I missed.
> 
> Either way, using WRITE_ONCE() for the state and status fields
> instead of a bulk memcpy() is cleaner and eliminates any concern
> about tearing from concurrent userspace writes (concern 1). I
> will send v2 with targeted WRITE_ONCE() writebacks for just the
> fields that need to be visible, dropping the bulk memcpy.

Sounds good. Please go ahead and send v2 so we can look at the actual 
implementation and discuss based on the code.

Best regards,
Yanjun Zhu

> 
> Best regards,
> Tristan Madani

[PATCH v2 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 local copy is reused across multi-packet sends
to preserve DMA progress state (cur_sge, sge_offset, resid). Modified
fields are written back individually using WRITE_ONCE() with
smp_store_release() for the state field to ensure the completer
observes consistent values.

The copy is invalidated when:
 - The WQE index advances (last packet sent, error, local ops,
   UD oversized)
 - A retry occurs (req_retry resets WQE state in shared memory)

The num_sge field is validated against qp->sq.max_sge on copy-in to
reject corrupted values early.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
v1 -> v2:
 - Replace bulk memcpy() writeback with targeted WRITE_ONCE() for
   individual fields (cur_sge, sge_offset, resid, status) and
   smp_store_release() for the state field, addressing review feedback
   from Zhu Yanjun

 drivers/infiniband/sw/rxe/rxe_req.c   | 60 ++++++++++++++++++++++++++-
 drivers/infiniband/sw/rxe/rxe_verbs.h |  6 +++
 2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 12d03f390b097..59b22bb160908 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 modified WQE fields to the shared queue using
+ * WRITE_ONCE() so the completer observes consistent values.
+ * Only the fields modified by the requester are written back;
+ * state is written last as the publication field.
+ */
+static void rxe_req_writeback_wqe(struct rxe_qp *qp)
+{
+	if (qp->req.send_wqe_valid && qp->req.shared_wqe) {
+		struct rxe_send_wqe *shared = qp->req.shared_wqe;
+		struct rxe_send_wqe *local = &qp->req.send_wqe.wqe;
+
+		WRITE_ONCE(shared->dma.cur_sge, local->dma.cur_sge);
+		WRITE_ONCE(shared->dma.sge_offset, local->dma.sge_offset);
+		WRITE_ONCE(shared->dma.resid, local->dma.resid);
+		WRITE_ONCE(shared->status, local->status);
+		/* Ensure fields above are visible before state transition */
+		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;
@@ -178,6 +198,8 @@ static struct rxe_send_wqe *req_next_wqe(struct rxe_qp *qp)
 {
 	struct rxe_send_wqe *wqe;
 	unsigned long flags;
+	unsigned int num_sge;
+	size_t copy_size;
 
 	req_check_sq_drain_done(qp);
 
@@ -193,6 +215,28 @@ static struct rxe_send_wqe *req_next_wqe(struct rxe_qp *qp)
 	}
 	spin_unlock_irqrestore(&qp->state_lock, flags);
 
+	/* If we already have a valid local copy of this WQE, use it.
+	 * This preserves DMA progress state across multi-packet sends.
+	 */
+	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 to prevent TOCTOU races on num_sge, SGE entries, and
+	 * inline data offsets. This is the send-path counterpart to
+	 * rxe_get_recv_wqe().
+	 */
+	num_sge = wqe->dma.num_sge;
+	if (unlikely(num_sge > qp->sq.max_sge)) {
+		rxe_dbg_qp(qp, "invalid num_sge in send WQE\n");
+		return NULL;
+	}
+	copy_size = sizeof(*wqe) + num_sge * sizeof(struct rxe_sge);
+	memcpy(&qp->req.send_wqe.wqe, wqe, copy_size);
+	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 +626,14 @@ static void update_state(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
 {
 	qp->req.opcode = pkt->opcode;
 
-	if (pkt->mask & RXE_END_MASK)
+	/* Write back local WQE state before possibly advancing index */
+	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;
 
@@ -635,6 +684,7 @@ 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;
 	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
+	qp->req.send_wqe_valid = false;
 
 	return 0;
 }
@@ -695,6 +745,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);
@@ -761,6 +812,7 @@ int rxe_requester(struct rxe_qp *qp)
 						       qp->req.wqe_index);
 			wqe->state = wqe_state_done;
 			wqe->status = IB_WC_SUCCESS;
+			qp->req.send_wqe_valid = false;
 			goto done;
 		}
 		payload = mtu;
@@ -820,12 +872,18 @@ int rxe_requester(struct rxe_qp *qp)
 	 * will continue looping and return to rxe_requester
 	 */
 done:
+	/* Write back local WQE for paths that skip update_state()
+	 * (local ops, UD oversized packets).
+	 */
+	rxe_req_writeback_wqe(qp);
 	ret = 0;
 	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.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

[PATCH v2 2/2] RDMA/rxe: copy completer WQE to kernel buffer before processing

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The completer reads WQE fields (dma.num_sge, dma.sge[], dma.cur_sge,
wr.opcode, first_psn, last_psn, state, status) directly from the
userspace-mapped shared send queue via queue_head(). Userspace can
modify these fields between completer reads, causing inconsistent
state in do_read() and do_atomic() which call copy_data().

This is the completer-path counterpart to the preceding patch which
fixed the requester path. Although rxe_sender() calls the requester
and completer sequentially (not concurrently), the requester may have
already advanced wqe_index and invalidated its kernel-private copy by
the time the completer processes the READ/ATOMIC response, so the
completer needs its own copy.

Fix by:
 1. Adding a kernel-private WQE copy buffer (comp_wqe) to
    rxe_comp_info
 2. Copying the WQE from shared memory in get_wqe() before any
    processing, with num_sge validation against qp->sq.max_sge
 3. Writing back status/state changes to the shared queue entry in
    do_complete() using WRITE_ONCE() so userspace observes consistent
    completion status

The flush_send_queue() error path is not modified as it runs during
QP teardown (ERR/RESET state) and does not call copy_data().

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
v1 -> v2:
 - Use WRITE_ONCE() for status/state writeback in do_complete(),
   addressing review feedback from Zhu Yanjun

 drivers/infiniband/sw/rxe/rxe_comp.c  | 34 +++++++++++++++++++++++++--
 drivers/infiniband/sw/rxe/rxe_verbs.h |  5 ++++
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
index 1390e861bd1d7..2c57e7a0d4203 100644
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -142,16 +142,38 @@ static inline enum comp_state get_wqe(struct rxe_qp *qp,
 				      struct rxe_send_wqe **wqe_p)
 {
 	struct rxe_send_wqe *wqe;
+	unsigned int num_sge;
+	size_t copy_size;
 
 	/* 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->state == wqe_state_posted) {
+		*wqe_p = NULL;
 		return pkt ? COMPST_DONE : COMPST_EXIT;
+	}
+
+	/* Copy WQE from userspace-mapped shared queue to kernel-private
+	 * buffer to prevent TOCTOU races on DMA state fields.
+	 * This is the completer-path counterpart to the requester fix.
+	 */
+	num_sge = wqe->dma.num_sge;
+	if (unlikely(num_sge > qp->sq.max_sge)) {
+		rxe_dbg_qp(qp, "invalid num_sge in send WQE (comp)\n");
+		memcpy(&qp->comp.comp_wqe.wqe, wqe, sizeof(*wqe));
+		qp->comp.comp_wqe.wqe.status = IB_WC_LOC_LEN_ERR;
+		qp->comp.shared_wqe = wqe;
+		*wqe_p = &qp->comp.comp_wqe.wqe;
+		return COMPST_ERROR;
+	}
+	copy_size = sizeof(*wqe) + num_sge * sizeof(struct rxe_sge);
+	memcpy(&qp->comp.comp_wqe.wqe, wqe, copy_size);
+	qp->comp.shared_wqe = wqe;
+	*wqe_p = &qp->comp.comp_wqe.wqe;
+	wqe = *wqe_p;
 
 	/* WQE does not require an ack */
 	if (wqe->state == wqe_state_done)
@@ -446,6 +468,14 @@ static void do_complete(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
 	struct rxe_cqe cqe;
 	bool post;
 
+	/* Write back status to the shared queue entry so the CQE
+	 * and any retry logic sees the correct completion status.
+	 */
+	if (qp->comp.shared_wqe) {
+		WRITE_ONCE(qp->comp.shared_wqe->status, wqe->status);
+		WRITE_ONCE(qp->comp.shared_wqe->state, wqe->state);
+	}
+
 	/* do we need to post a completion */
 	post = ((qp->sq_sig_type == IB_SIGNAL_ALL_WR) ||
 			(wqe->wr.send_flags & IB_SEND_SIGNALED) ||
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index a22dfc6e5ae3c..f1d647ddc1e05 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -130,6 +130,11 @@ struct rxe_comp_info {
 	int			started_retry;
 	u32			retry_cnt;
 	u32			rnr_retry;
+	struct rxe_send_wqe	*shared_wqe;
+	struct {
+		struct rxe_send_wqe	wqe;
+		struct ib_sge		sge[RXE_MAX_SGE];
+	} comp_wqe;
 };
 
 /* responder states */
-- 
2.47.3

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

[Zhu Yanjun]

在 2026/8/30 12:13, 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 local copy is reused across multi-packet sends
> to preserve DMA progress state (cur_sge, sge_offset, resid). Modified
> fields are written back individually using WRITE_ONCE() with
> smp_store_release() for the state field to ensure the completer
> observes consistent values.
> 
> The copy is invalidated when:
>   - The WQE index advances (last packet sent, error, local ops,
>     UD oversized)
>   - A retry occurs (req_retry resets WQE state in shared memory)
> 
> The num_sge field is validated against qp->sq.max_sge on copy-in to
> reject corrupted values early.
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
> v1 -> v2:
>   - Replace bulk memcpy() writeback with targeted WRITE_ONCE() for
>     individual fields (cur_sge, sge_offset, resid, status) and
>     smp_store_release() for the state field, addressing review feedback
>     from Zhu Yanjun
> 
>   drivers/infiniband/sw/rxe/rxe_req.c   | 60 ++++++++++++++++++++++++++-
>   drivers/infiniband/sw/rxe/rxe_verbs.h |  6 +++
>   2 files changed, 65 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index 12d03f390b097..59b22bb160908 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 modified WQE fields to the shared queue using
> + * WRITE_ONCE() so the completer observes consistent values.
> + * Only the fields modified by the requester are written back;
> + * state is written last as the publication field.
> + */
> +static void rxe_req_writeback_wqe(struct rxe_qp *qp)
> +{
> +	if (qp->req.send_wqe_valid && qp->req.shared_wqe) {
> +		struct rxe_send_wqe *shared = qp->req.shared_wqe;
> +		struct rxe_send_wqe *local = &qp->req.send_wqe.wqe;
> +
> +		WRITE_ONCE(shared->dma.cur_sge, local->dma.cur_sge);
> +		WRITE_ONCE(shared->dma.sge_offset, local->dma.sge_offset);
> +		WRITE_ONCE(shared->dma.resid, local->dma.resid);
> +		WRITE_ONCE(shared->status, local->status);
> +		/* Ensure fields above are visible before state transition */
> +		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;
> @@ -178,6 +198,8 @@ static struct rxe_send_wqe *req_next_wqe(struct rxe_qp *qp)
>   {
>   	struct rxe_send_wqe *wqe;
>   	unsigned long flags;
> +	unsigned int num_sge;
> +	size_t copy_size;
>   
>   	req_check_sq_drain_done(qp);
>   
> @@ -193,6 +215,28 @@ static struct rxe_send_wqe *req_next_wqe(struct rxe_qp *qp)
>   	}
>   	spin_unlock_irqrestore(&qp->state_lock, flags);
>   
> +	/* If we already have a valid local copy of this WQE, use it.
> +	 * This preserves DMA progress state across multi-packet sends.
> +	 */
> +	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 to prevent TOCTOU races on num_sge, SGE entries, and
> +	 * inline data offsets. This is the send-path counterpart to
> +	 * rxe_get_recv_wqe().
> +	 */
> +	num_sge = wqe->dma.num_sge;
> +	if (unlikely(num_sge > qp->sq.max_sge)) {
> +		rxe_dbg_qp(qp, "invalid num_sge in send WQE\n");
> +		return NULL;
> +	}

Since wqe->dma.num_sge resides in shared memory, accessing it without 
READ_ONCE() allows compiler re-fetches that create a TOCTOU race 
condition, potentially leading to a kernel buffer overflow if userspace 
modifies the value post-validation.

As such, please use the following.

num_sge = READ_ONCE(wqe->dma.num_sge);

Thanks a lot.
Zhu Yanjun

> +	copy_size = sizeof(*wqe) + num_sge * sizeof(struct rxe_sge);
> +	memcpy(&qp->req.send_wqe.wqe, wqe, copy_size);
> +	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 +626,14 @@ static void update_state(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
>   {
>   	qp->req.opcode = pkt->opcode;
>   
> -	if (pkt->mask & RXE_END_MASK)
> +	/* Write back local WQE state before possibly advancing index */
> +	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;
>   
> @@ -635,6 +684,7 @@ 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;
>   	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
> +	qp->req.send_wqe_valid = false;
>   
>   	return 0;
>   }
> @@ -695,6 +745,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);
> @@ -761,6 +812,7 @@ int rxe_requester(struct rxe_qp *qp)
>   						       qp->req.wqe_index);
>   			wqe->state = wqe_state_done;
>   			wqe->status = IB_WC_SUCCESS;
> +			qp->req.send_wqe_valid = false;
>   			goto done;
>   		}
>   		payload = mtu;
> @@ -820,12 +872,18 @@ int rxe_requester(struct rxe_qp *qp)
>   	 * will continue looping and return to rxe_requester
>   	 */
>   done:
> +	/* Write back local WQE for paths that skip update_state()
> +	 * (local ops, UD oversized packets).
> +	 */
> +	rxe_req_writeback_wqe(qp);
>   	ret = 0;
>   	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.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 {