* [PATCH v6 0/2] RDMA/rxe: fix TOCTOU races in send WQE processing
@ 2026-10-26 0:14 Tristan Madani
2026-10-26 0:14 ` [PATCH v6 1/2] RDMA/rxe: copy send WQE to kernel buffer before processing Tristan Madani
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Tristan Madani @ 2026-10-26 0:14 UTC (permalink / raw)
To: linux-rdma; +Cc: jgg, leon, zyjzyj2000, yanjun.zhu, rpearsonhpe
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.
v6 adds post-copy sanitization of attacker-controlled fields to address
the automated review findings from v4/v5:
- wr_opcode_mask() OOB: both patches now reject opcodes >
IB_WR_ATOMIC_WRITE before the array access.
- num_sge clamp: both patches now clamp dma.num_sge against
qp->sq.max_sge immediately after copy, before any SGE walk.
- sge_offset / resid / cur_sge: the requester already had a
num_sge + cur_sge bounds check after req_next_wqe() (lines
754-760 of rxe_req.c). With the copy, these fields are now
frozen, so the existing check is sufficient.
- FORTIFY_SOURCE: struct rxe_send_wqe ends with a flex array
(__DECLARE_FLEX_ARRAY in rxe_dma_info), so __builtin_object_size
returns (size_t)-1 for the memcpy. This is identical to the
merged receive-path fix (d6ab440240a04) and is a tooling
limitation, not a bug.
- Flush/retry/posted-state paths: these write directly to shared
memory (flush, posted) or invalidate the cache and take a fresh
copy (retry), identical to pre-patch behavior. Not affected by
this series.
Changes since v5:
- Added opcode bounds check after copy in both requester and
completer to prevent OOB access via wr_opcode_mask().
- Added num_sge clamp in requester after copy (completer already
had it in v5).
- Resent as standalone patchset from the correct sender address.
Changes since v4:
- Resent from the correct sender address. No code changes vs v4.
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 | 60 +++++++++++++++++++++++++-
drivers/infiniband/sw/rxe/rxe_qp.c | 2 +
drivers/infiniband/sw/rxe/rxe_req.c | 64 +++++++++++++++++++++++++--
drivers/infiniband/sw/rxe/rxe_verbs.h | 12 +++++
4 files changed, 132 insertions(+), 6 deletions(-)
--
2.47.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v6 1/2] RDMA/rxe: copy send WQE to kernel buffer before processing
2026-10-26 0:14 [PATCH v6 0/2] RDMA/rxe: fix TOCTOU races in send WQE processing Tristan Madani
@ 2026-10-26 0:14 ` Tristan Madani
2026-10-26 1:43 ` sashiko-bot
2026-10-26 0:14 ` [PATCH v6 2/2] RDMA/rxe: copy send WQE to kernel buffer in completer path Tristan Madani
2026-10-26 16:15 ` [PATCH v6 0/2] RDMA/rxe: fix TOCTOU races in send WQE processing Zhu Yanjun
2 siblings, 1 reply; 6+ messages in thread
From: Tristan Madani @ 2026-10-26 0:14 UTC (permalink / raw)
To: linux-rdma; +Cc: jgg, leon, zyjzyj2000, yanjun.zhu, rpearsonhpe
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.
After copying, sanitize attacker-controlled fields: reject out-of-range
opcodes before wr_opcode_mask() to prevent an OOB array access, and
clamp num_sge against the QP maximum so copy_data() never walks past
the SGE array.
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 | 64 +++++++++++++++++++++++++--
drivers/infiniband/sw/rxe/rxe_verbs.h | 6 +++
3 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index 311f285d78a6be..77606c4a039b17 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 24f5c044363f77..dac7eae54838be 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,29 @@ 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;
+
+ /* Sanitize attacker-controlled fields from the copy */
+ if (unlikely(wqe->wr.opcode > IB_WR_ATOMIC_WRITE)) {
+ qp->req.send_wqe_valid = false;
+ return NULL;
+ }
+ if (wqe->dma.num_sge > qp->sq.max_sge)
+ wqe->dma.num_sge = qp->sq.max_sge;
+
wqe->mask = wr_opcode_mask(wqe->wr.opcode, qp);
return wqe;
}
@@ -582,9 +625,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 +681,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 +715,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 +731,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 +746,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 +824,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 +893,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 0f5ffd94643f92..a22dfc6e5ae3cf 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] 6+ messages in thread
* [PATCH v6 2/2] RDMA/rxe: copy send WQE to kernel buffer in completer path
2026-10-26 0:14 [PATCH v6 0/2] RDMA/rxe: fix TOCTOU races in send WQE processing Tristan Madani
2026-10-26 0:14 ` [PATCH v6 1/2] RDMA/rxe: copy send WQE to kernel buffer before processing Tristan Madani
@ 2026-10-26 0:14 ` Tristan Madani
2026-10-26 1:39 ` sashiko-bot
2026-10-26 16:15 ` [PATCH v6 0/2] RDMA/rxe: fix TOCTOU races in send WQE processing Zhu Yanjun
2 siblings, 1 reply; 6+ messages in thread
From: Tristan Madani @ 2026-10-26 0:14 UTC (permalink / raw)
To: linux-rdma; +Cc: jgg, leon, zyjzyj2000, yanjun.zhu, rpearsonhpe
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.
After copying, sanitize attacker-controlled fields: reject out-of-range
opcodes to prevent an OOB array access, and clamp num_sge against the
QP maximum so subsequent DMA operations stay within bounds.
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 | 60 ++++++++++++++++++++++++++-
drivers/infiniband/sw/rxe/rxe_qp.c | 1 +
drivers/infiniband/sw/rxe/rxe_verbs.h | 6 +++
3 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
index 1390e861bd1d74..6e72cfeb10ec8d 100644
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -137,22 +137,76 @@ 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;
+
+ /* Sanitize attacker-controlled fields from the copy */
+ if (unlikely(qp->comp.comp_wqe.wqe.wr.opcode > IB_WR_ATOMIC_WRITE)) {
+ qp->comp.comp_wqe_valid = false;
+ *wqe_p = NULL;
+ return pkt ? COMPST_DONE : COMPST_EXIT;
+ }
+ 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 +508,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 77606c4a039b17..450254a1310171 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 a22dfc6e5ae3cf..6205dbc29dff6d 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v6 2/2] RDMA/rxe: copy send WQE to kernel buffer in completer path
2026-10-26 0:14 ` [PATCH v6 2/2] RDMA/rxe: copy send WQE to kernel buffer in completer path Tristan Madani
@ 2026-10-26 1:39 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-10-26 1:39 UTC (permalink / raw)
To: Tristan Madani, linux-rdma
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] RDMA/rxe: missing dma.cur_sge bounds check in get_wqe() allows OOB read
- [High] RDMA/rxe: wrong opcode bound in get_wqe() breaks IB_WR_REG_MR and wedges SQ
- [High] RDMA/rxe: stale comp_wqe and unshared DMA state in get_wqe() break READ retry
- [High] RDMA/rxe: TOCTOU races on wqe->state and shared wqe pointer in get_wqe()
- [High] RDMA/rxe: field-spanning memcpy() in get_wqe() triggers FORTIFY_SOURCE warn
--
commit 85eed619ea208c64c4708d3eee07f43c50d2a7e2
Author: Tristan Madani <tristmd@gmail.com>
RDMA/rxe: copy send WQE to kernel buffer in completer path
This commit copies send WQEs from the shared queue to a completer-private
kernel buffer in get_wqe() and sanitizes fields before processing
completions. It also writes back completion status and rd_atomic state
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..6e72cfeb10ec8 100644
> --- a/drivers/infiniband/sw/rxe/rxe_comp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_comp.c
> @@ -137,22 +137,76 @@ 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;
> + }
[Severity: High]
How does this reuse check interact with retries of partially completed
multi-packet RDMA READs?
During a multi-packet RDMA READ response, do_read() calls copy_data() on
&wqe->dma, which updates dma.resid, dma.cur_sge, and dma.sge_offset only in
qp->comp.comp_wqe.wqe. Because rxe_comp_writeback_wqe() only writes back
status and has_rd_atomic inside do_complete(), qp->comp.shared_wqe->dma in
qp->sq.queue is never updated as packets arrive.
If a timeout or NAK triggers a retry, req_retry() in
drivers/infiniband/sw/rxe/rxe_req.c reads wqe->dma.resid from the shared WQE
in qp->sq.queue:
if (mask & WR_READ_MASK) {
npsn = (wqe->dma.length - wqe->dma.resid) /
qp->mtu;
wqe->iova += npsn * qp->mtu;
}
Since shared_wqe->dma.resid was never decremented, npsn is 0, wqe->iova is
not advanced, and rxe_requester() re-requests the entire read from offset 0
with new first_psn and last_psn values while setting shared_wqe->state back
to wqe_state_pending.
In addition, qp->comp.comp_wqe_valid is not invalidated on retry. When the
retried read response arrives, get_wqe() sees:
qp->comp.comp_wqe_valid && qp->comp.shared_wqe == wqe &&
qp->comp.comp_wqe.wqe.state == state
(both wqe_state_pending) and reuses the pre-retry qp->comp.comp_wqe.wqe with
the stale first_psn and last_psn. Doesn't check_ack() then fail the PSN
comparison against wqe->first_psn and return COMPST_ERROR, moving the QP to
IB_QPS_ERR?
> +
> + /* 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;
[Severity: High]
Are there remaining TOCTOU windows on wqe->state and the shared wqe pointer
in get_wqe()?
1. When state == wqe_state_posted above, get_wqe() sets *wqe_p = wqe
(pointing directly to the uncopied, unsanitized shared queue element) and
returns COMPST_EXIT when pkt is NULL. In rxe_completer():
case COMPST_EXIT:
if (qp->comp.timeout_retry && wqe) {
state = COMPST_ERROR_RETRY;
break;
}
...
case COMPST_ERROR_RETRY:
/* there is nothing to retry in this case */
if (!wqe || (wqe->state == wqe_state_posted))
goto exit;
If wqe->state in shared memory changes after get_wqe() read it (for example,
via concurrent userspace modification of the mmap'd send queue) and
qp->comp.retry_cnt == 0, COMPST_ERROR_RETRY transitions to COMPST_ERROR and
calls do_complete(qp, wqe) directly on the uncopied shared WQE. Should
*wqe_p be set to NULL when state == wqe_state_posted?
2. When state != wqe_state_posted, memcpy() overwrites
qp->comp.comp_wqe.wqe.state with a second read from shared memory rather
than preserving the local state variable loaded via smp_load_acquire(). If
userspace mutates wqe->state in shared memory between smp_load_acquire() and
memcpy(), check_state can observe wqe_state_posted or wqe_state_error (with
wqe->status == IB_WC_SUCCESS, triggering WARN_ON_ONCE in COMPST_ERROR). Also,
across multi-packet responses, userspace can flip shared_wqe->state in the
mmap'd ring to make qp->comp.comp_wqe.wqe.state == state false and force
get_wqe() to overwrite comp_wqe mid-operation.
[Severity: High]
Also, in struct rxe_comp_info in drivers/infiniband/sw/rxe/rxe_verbs.h,
comp_wqe is defined as:
struct {
struct rxe_send_wqe wqe;
struct ib_sge sge[RXE_MAX_SGE];
} comp_wqe;
Because memcpy() here (and in req_next_wqe() in
drivers/infiniband/sw/rxe/rxe_req.c from the previous patch) passes the inner
member &qp->comp.comp_wqe.wqe instead of the outer struct &qp->comp.comp_wqe,
__member_size(&qp->comp.comp_wqe.wqe) is sizeof(struct rxe_send_wqe). Will
copying sizeof(*wqe) + qp->sq.max_inline bytes trigger a
CONFIG_FORTIFY_SOURCE field-spanning write warning whenever
qp->sq.max_inline > 0?
> +
> + /* Sanitize attacker-controlled fields from the copy */
> + if (unlikely(qp->comp.comp_wqe.wqe.wr.opcode > IB_WR_ATOMIC_WRITE)) {
> + qp->comp.comp_wqe_valid = false;
> + *wqe_p = NULL;
> + return pkt ? COMPST_DONE : COMPST_EXIT;
> + }
[Severity: High]
Won't checking wr.opcode > IB_WR_ATOMIC_WRITE reject kernel-issued
IB_WR_REG_MR work requests and wedge the send queue?
In include/rdma/ib_verbs.h, enum ib_wr_opcode defines:
IB_WR_FLUSH = IB_UVERBS_WR_FLUSH,
IB_WR_ATOMIC_WRITE = IB_UVERBS_WR_ATOMIC_WRITE,
/* These are kernel only and can not be issued by userspace */
IB_WR_REG_MR = 0x20,
IB_WR_ATOMIC_WRITE is 15, while IB_WR_REG_MR is 0x20 (32), which is why
wr_opcode_mask() in drivers/infiniband/sw/rxe/rxe_loc.h checks
opcode > IB_WR_REG_MR and wr_to_wc_opcode() in
drivers/infiniband/sw/rxe/rxe_comp.c handles IB_WR_REG_MR. Checking
wr.opcode > IB_WR_ATOMIC_WRITE here (as well as in req_next_wqe() in
drivers/infiniband/sw/rxe/rxe_req.c) rejects every IB_WR_REG_MR WQE posted by
kernel ULPs.
Furthermore, when this check triggers in get_wqe(), it sets *wqe_p = NULL and
returns COMPST_DONE or COMPST_EXIT without advancing the SQ consumer index or
transitioning to COMPST_ERROR. Because the WQE remains at the head of
qp->sq.queue forever (and COMPST_EXIT sees wqe == NULL, skipping
COMPST_ERROR_RETRY and calling reset_retry_timer(qp)), doesn't this
permanently wedge the send queue and re-arm retrans_timer in an infinite loop?
> + 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 get_wqe() also need to validate dma.cur_sge from the copied WQE?
In a user QP, shared_wqe->dma.cur_sge resides in the userspace-mapped send
queue buffer and can be modified by userspace before get_wqe() copies the WQE.
While dma.num_sge is clamped to qp->sq.max_sge here, dma.cur_sge is left
unchecked. When an RDMA READ or ATOMIC response arrives, do_read() or
do_atomic() passes &wqe->dma to copy_data() in
drivers/infiniband/sw/rxe/rxe_mr.c:
struct rxe_sge *sge = &dma->sge[dma->cur_sge];
u32 offset = dma->sge_offset;
u32 resid = dma->resid;
...
if (sge->length && (offset < sge->length)) {
mr = lookup_mr(pd, access, sge->lkey, RXE_LOOKUP_LOCAL);
Can an out-of-bounds dma.cur_sge index past qp->comp.comp_wqe.sge[RXE_MAX_SGE]
and read arbitrary kernel memory in copy_data()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261026001405.1698455-1-tristan@talencesecurity.com?part=2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6 1/2] RDMA/rxe: copy send WQE to kernel buffer before processing
2026-10-26 0:14 ` [PATCH v6 1/2] RDMA/rxe: copy send WQE to kernel buffer before processing Tristan Madani
@ 2026-10-26 1:43 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-10-26 1:43 UTC (permalink / raw)
To: Tristan Madani, linux-rdma
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] RDMA/rxe: out-of-bounds read from qp->req.send_wqe in finish_packet()
- [High] RDMA/rxe: req_next_wqe() rejects IB_WR_REG_MR and wedges send queue
- [High] RDMA/rxe: field-spanning memcpy() into send_wqe.wqe in req_next_wqe()
- [High] RDMA/rxe: missing dma.resid writeback in rxe_req_writeback_wqe() breaks IB_WR_FLUSH
- [High] RDMA/rxe: clamping num_sge in req_next_wqe() bypasses rxe_requester() check
- [High] RDMA/rxe: out-of-bounds SGE walk in req_retry() and shared state read in req_next_wqe()
--
commit cba7cc46eb582b40f6f5480cd729885e3630e9ad
Author: Tristan Madani <tristmd@gmail.com>
RDMA/rxe: copy send WQE to kernel buffer before processing
This commit copies send WQEs from the userspace-mapped send queue into a
kernel-private buffer in req_next_wqe() before processing, sanitizes opcode
and num_sge fields on the local copy, and writes updated WQE state fields
back to the shared queue.
> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index 4f353abd76f55..9df1776ee9ad7 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);
> +}
[Severity: High]
Should rxe_req_writeback_wqe() also write local->dma.resid back to
shared->dma.resid so that IB_WR_FLUSH requests with a non-zero flush length
can complete?
When rxe_requester() processes an IB_WR_FLUSH WQE, init_req_packet() writes
wqe->dma.resid into the RETH header and finish_packet() then zeroes
wqe->dma.resid on the local copy:
drivers/infiniband/sw/rxe/rxe_req.c:finish_packet() {
...
} else if (pkt->mask & RXE_FLUSH_MASK) {
/* oA19-2: shall have no payload. */
wqe->dma.resid = 0;
}
}
Because update_state() -> rxe_req_writeback_wqe() does not write dma.resid
back to qp->req.shared_wqe, shared->dma.resid in qp->sq.queue remains at its
original non-zero length (including when the completer later copies the WQE
in commit "RDMA/rxe: copy send WQE to kernel buffer in completer path").
When the responder replies with a zero-payload
IB_OPCODE_RC_RDMA_READ_RESPONSE_ONLY packet, rxe_completer() -> do_read()
calls copy_data() with payload_size(pkt) == 0, which returns immediately
without modifying wqe->dma.resid, and then checks:
drivers/infiniband/sw/rxe/rxe_comp.c:do_read() {
...
if (wqe->dma.resid == 0 && (pkt->mask & RXE_END_MASK))
return COMPST_COMP_ACK;
return COMPST_UPDATE_COMP;
}
With wqe->dma.resid still non-zero in the shared queue, do_read() returns
COMPST_UPDATE_COMP instead of COMPST_COMP_ACK, leaving the IB_WR_FLUSH WQE
uncompleted and hanging the send queue.
> +
> static struct rxe_send_wqe *__req_next_wqe(struct rxe_qp *qp)
> {
> struct rxe_queue *q = qp->sq.queue;
> @@ -193,6 +213,29 @@ 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);
[Severity: High]
Will passing &qp->req.send_wqe.wqe as the destination to memcpy() trigger a
CONFIG_FORTIFY_SOURCE field-spanning write warning whenever
qp->sq.max_inline > 0?
In struct rxe_req_info (drivers/infiniband/sw/rxe/rxe_verbs.h), send_wqe is
defined as:
struct {
struct rxe_send_wqe wqe;
struct ib_sge sge[RXE_MAX_SGE];
} send_wqe;
Because .wqe is the inner struct member rather than the enclosing
&qp->req.send_wqe container, __member_size(&qp->req.send_wqe.wqe) evaluates
to sizeof(struct rxe_send_wqe). For any QP with qp->sq.max_inline > 0,
sizeof(*wqe) + qp->sq.max_inline exceeds sizeof(struct rxe_send_wqe) and
spans across .wqe into .sge, triggering fortify_memcpy_chk(). Should the
destination be &qp->req.send_wqe instead, matching the receive-side copies?
> + qp->req.shared_wqe = wqe;
> + qp->req.send_wqe_valid = true;
> +
> + wqe = &qp->req.send_wqe.wqe;
[Severity: Critical]
Can copying only sizeof(*wqe) + qp->sq.max_inline bytes into
qp->req.send_wqe cause finish_packet() to read out of bounds past
qp->req.send_wqe.sge into struct rxe_qp and adjacent kernel heap memory on an
IB_WR_ATOMIC_WRITE request?
For a user RC QP (qp->is_user == true), WQEs are read directly from the
mmap'd send queue ring without going through validate_send_wr(). In
rxe_requester(), payload for RXE_ATOMIC_WRITE_MASK is taken from the
user-controlled wqe->dma.resid and capped only at mtu (up to 4096 bytes):
drivers/infiniband/sw/rxe/rxe_req.c:rxe_requester() {
...
mtu = get_mtu(qp);
payload = (mask & (RXE_WRITE_OR_SEND_MASK | RXE_ATOMIC_WRITE_MASK)) ?
wqe->dma.resid : 0;
if (payload > mtu) {
...
payload = mtu;
}
}
Then finish_packet() copies payload bytes directly from wqe->dma.atomic_wr
into the packet payload:
drivers/infiniband/sw/rxe/rxe_req.c:finish_packet() {
...
if (pkt->mask & RXE_ATOMIC_WRITE_MASK) {
memcpy(payload_addr(pkt), wqe->dma.atomic_wr, payload);
wqe->dma.resid -= payload;
}
}
Because wqe now points to &qp->req.send_wqe.wqe inside struct rxe_qp, where
only qp->sq.max_inline bytes were copied and sge[RXE_MAX_SGE] is 512 bytes, a
user WQE with wr.opcode = IB_WR_ATOMIC_WRITE, dma.num_sge = 0, and
dma.resid = 4096 causes finish_packet() to copy up to 3584 bytes past
qp->req.send_wqe into the transmitted network packet (or uninitialized bytes
from qp->req.send_wqe.sge when payload > qp->sq.max_inline).
> +
> + /* Sanitize attacker-controlled fields from the copy */
> + if (unlikely(wqe->wr.opcode > IB_WR_ATOMIC_WRITE)) {
> + qp->req.send_wqe_valid = false;
> + return NULL;
> + }
[Severity: High]
Doesn't rejecting wqe->wr.opcode > IB_WR_ATOMIC_WRITE here break fast memory
registration (IB_WR_REG_MR) for kernel ULPs and permanently wedge the send
queue whenever an opcode above IB_WR_ATOMIC_WRITE is encountered?
In include/rdma/ib_verbs.h, IB_WR_ATOMIC_WRITE is 15 while the kernel-only
IB_WR_REG_MR opcode is 0x20 (32):
include/rdma/ib_verbs.h:enum ib_wr_opcode {
...
IB_WR_FLUSH = IB_UVERBS_WR_FLUSH,
IB_WR_ATOMIC_WRITE = IB_UVERBS_WR_ATOMIC_WRITE,
/* These are kernel only and can not be issued by userspace */
IB_WR_REG_MR = 0x20,
};
Because req_next_wqe() runs for both user and kernel QPs, any IB_WR_REG_MR
WQE posted by a kernel ULP matches wqe->wr.opcode > IB_WR_ATOMIC_WRITE,
making the IB_WR_REG_MR handling in rxe_do_local_ops() -> rxe_reg_fast_mr()
unreachable.
In addition, when req_next_wqe() returns NULL on this check, rxe_requester()
takes the !wqe path:
drivers/infiniband/sw/rxe/rxe_req.c:rxe_requester() {
...
wqe = req_next_wqe(qp);
if (unlikely(!wqe))
goto exit;
}
That exits with -EAGAIN without setting wqe->status = IB_WC_LOC_QP_OP_ERR,
without advancing qp->req.wqe_index, and without calling rxe_qp_error(qp),
leaving the rejected WQE at the head of the send queue indefinitely. Note
that wr_opcode_mask() in drivers/infiniband/sw/rxe/rxe_loc.h already bounds
checks opcode < 0 || opcode > IB_WR_REG_MR and returns 0 so that
rxe_requester() fails invalid opcodes via next_opcode() -> goto err.
> + if (wqe->dma.num_sge > qp->sq.max_sge)
> + wqe->dma.num_sge = qp->sq.max_sge;
[Severity: High]
If req_next_wqe() silently clamps wqe->dma.num_sge to qp->sq.max_sge on the
local copy, doesn't that bypass the error check in rxe_requester() and allow
malformed WQEs to be processed with an unclamped num_sge in shared memory?
In rxe_requester(), commit 126c757e4cd4 ("RDMA/rxe: Validate num_sge/cur_sge
before indexing wqe->dma.sge[]") added:
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");
wqe->status = IB_WC_LOC_QP_OP_ERR;
goto err;
}
}
Because req_next_wqe() clamps wqe->dma.num_sge before returning wqe to
rxe_requester(), the wqe->dma.num_sge > qp->sq.max_sge check in
rxe_requester() becomes unreachable dead code. Instead of failing the WQE
with IB_WC_LOC_QP_OP_ERR and moving the QP to error state, rxe_requester()
transmits the WQE with a truncated SGE list and transitions shared->state to
wqe_state_processing / wqe_state_pending while shared->dma.num_sge in
qp->sq.queue remains unclamped.
> +
> wqe->mask = wr_opcode_mask(wqe->wr.opcode, qp);
> return wqe;
> }
[ ... ]
> @@ -702,6 +753,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;
> }
[Severity: High]
Can req_retry() still walk out of bounds past the end of qp->sq.queue->buf
when operating on the shared WQE in userspace-mapped memory?
When a retry occurs on an in-flight SEND or RDMA WRITE WQE, req_retry()
accesses the shared WQE directly via queue_addr_from_index(qp->sq.queue,
wqe_index) and calls retry_first_write_send() -> advance_dma_data(&wqe->dma,
to_send):
drivers/infiniband/sw/rxe/rxe_req.c:req_retry() {
...
if (first) {
first = 0;
if (mask & WR_WRITE_OR_SEND_MASK) {
npsn = (qp->comp.psn - wqe->first_psn) &
BTH_PSN_MASK;
retry_first_write_send(qp, wqe, npsn);
}
}
drivers/infiniband/sw/rxe/rxe_mr.c:advance_dma_data() {
...
while (length) {
unsigned int bytes;
if (offset >= sge->length) {
sge++;
dma->cur_sge++;
offset = 0;
if (dma->cur_sge >= dma->num_sge)
return -ENOSPC;
}
}
Because req_next_wqe() only clamped dma.num_sge on the local copy (preventing
rxe_requester() from failing a WQE posted with dma.num_sge > qp->sq.max_sge)
or if userspace modifies the shared WQE before a retry, wqe->dma.num_sge in
qp->sq.queue->buf is unbounded in req_retry(). If the trailing SGEs have
sge->length == 0 (and retry_first_write_send() also ignores the -ENOSPC
return value across its npsn loop), sge++ walks past the WQE SGE array and
off the end of the vmalloc_user() send queue buffer. (The completer path had
a similar exposure prior to follow-up commit "RDMA/rxe: copy send WQE to
kernel buffer in completer path", whereas req_retry() continues to operate
directly on the shared queue.)
Also, in req_next_wqe(), the IB_QPS_SQD check reads wqe->state from the
shared userspace buffer before checking qp->req.send_wqe_valid:
drivers/infiniband/sw/rxe/rxe_req.c:req_next_wqe() {
...
spin_lock_irqsave(&qp->state_lock, flags);
if (unlikely((qp_state(qp) == IB_QPS_SQD) &&
(wqe->state != wqe_state_processing))) {
spin_unlock_irqrestore(&qp->state_lock, flags);
return NULL;
}
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;
}
Could userspace modify wqe->state in the shared buffer to stall an
in-progress multi-packet WQE during SQD or bypass SQD by writing
wqe_state_processing to a newly posted WQE?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261026001405.1698455-1-tristan@talencesecurity.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6 0/2] RDMA/rxe: fix TOCTOU races in send WQE processing
2026-10-26 0:14 [PATCH v6 0/2] RDMA/rxe: fix TOCTOU races in send WQE processing Tristan Madani
2026-10-26 0:14 ` [PATCH v6 1/2] RDMA/rxe: copy send WQE to kernel buffer before processing Tristan Madani
2026-10-26 0:14 ` [PATCH v6 2/2] RDMA/rxe: copy send WQE to kernel buffer in completer path Tristan Madani
@ 2026-10-26 16:15 ` Zhu Yanjun
2 siblings, 0 replies; 6+ messages in thread
From: Zhu Yanjun @ 2026-10-26 16:15 UTC (permalink / raw)
To: Tristan Madani, linux-rdma, yanjun.zhu@linux.dev
Cc: jgg, leon, zyjzyj2000, rpearsonhpe
在 2026/10/25 17:14, Tristan Madani 写道:
> 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.
>
> v6 adds post-copy sanitization of attacker-controlled fields to address
> the automated review findings from v4/v5:
>
> - wr_opcode_mask() OOB: both patches now reject opcodes >
> IB_WR_ATOMIC_WRITE before the array access.
>
> - num_sge clamp: both patches now clamp dma.num_sge against
> qp->sq.max_sge immediately after copy, before any SGE walk.
>
> - sge_offset / resid / cur_sge: the requester already had a
> num_sge + cur_sge bounds check after req_next_wqe() (lines
> 754-760 of rxe_req.c). With the copy, these fields are now
> frozen, so the existing check is sufficient.
>
> - FORTIFY_SOURCE: struct rxe_send_wqe ends with a flex array
> (__DECLARE_FLEX_ARRAY in rxe_dma_info), so __builtin_object_size
> returns (size_t)-1 for the memcpy. This is identical to the
> merged receive-path fix (d6ab440240a04) and is a tooling
> limitation, not a bug.
>
> - Flush/retry/posted-state paths: these write directly to shared
> memory (flush, posted) or invalidate the cache and take a fresh
> copy (retry), identical to pre-patch behavior. Not affected by
> this series.
>
I’ve reviewed the feedback from Sashico, and some of the comments still
make sense. Some of the more interesting points deserve a closer look so
we can better understand the issues and address them.
Yanjun Zhu
> Changes since v5:
> - Added opcode bounds check after copy in both requester and
> completer to prevent OOB access via wr_opcode_mask().
> - Added num_sge clamp in requester after copy (completer already
> had it in v5).
> - Resent as standalone patchset from the correct sender address.
>
> Changes since v4:
> - Resent from the correct sender address. No code changes vs v4.
>
> 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 | 60 +++++++++++++++++++++++++-
> drivers/infiniband/sw/rxe/rxe_qp.c | 2 +
> drivers/infiniband/sw/rxe/rxe_req.c | 64 +++++++++++++++++++++++++--
> drivers/infiniband/sw/rxe/rxe_verbs.h | 12 +++++
> 4 files changed, 132 insertions(+), 6 deletions(-)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-26 16:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-26 0:14 [PATCH v6 0/2] RDMA/rxe: fix TOCTOU races in send WQE processing Tristan Madani
2026-10-26 0:14 ` [PATCH v6 1/2] RDMA/rxe: copy send WQE to kernel buffer before processing Tristan Madani
2026-10-26 1:43 ` sashiko-bot
2026-10-26 0:14 ` [PATCH v6 2/2] RDMA/rxe: copy send WQE to kernel buffer in completer path Tristan Madani
2026-10-26 1:39 ` sashiko-bot
2026-10-26 16:15 ` [PATCH v6 0/2] RDMA/rxe: fix TOCTOU races in send WQE processing Zhu Yanjun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox