Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* [PATCH 1/2] RDMA/rxe: copy send WQE to kernel buffer before processing
@ 2026-08-16 10:44 Tristan Madani
  2026-08-16 10:44 ` [PATCH 2/2] RDMA/rxe: copy completer " Tristan Madani
  0 siblings, 1 reply; 2+ messages in thread
From: Tristan Madani @ 2026-08-16 10:44 UTC (permalink / raw)
  To: Zhu Yanjun, Jason Gunthorpe, Leon Romanovsky
  Cc: Moni Shoua, linux-rdma, stable, 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


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

* [PATCH 2/2] RDMA/rxe: copy completer WQE to kernel buffer before processing
  2026-08-16 10:44 [PATCH 1/2] RDMA/rxe: copy send WQE to kernel buffer before processing Tristan Madani
@ 2026-08-16 10:44 ` Tristan Madani
  0 siblings, 0 replies; 2+ messages in thread
From: Tristan Madani @ 2026-08-16 10:44 UTC (permalink / raw)
  To: Zhu Yanjun, Jason Gunthorpe, Leon Romanovsky
  Cc: Moni Shoua, linux-rdma, stable, 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


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

end of thread, other threads:[~2026-08-16 10:44 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-16 10:44 [PATCH 1/2] RDMA/rxe: copy send WQE to kernel buffer before processing Tristan Madani
2026-08-16 10:44 ` [PATCH 2/2] RDMA/rxe: copy completer " Tristan Madani

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