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

[Zhu Yanjun <yanjun.zhu@linux.dev>]

在 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(-)
>