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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] RDMA/rxe: out-of-bounds read from qp->req.send_wqe in finish_packet()
--

commit 7ddcf04d6a32c8caa9ab909cc0cf868ac66e4fb8
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, writing updated
status, state, and DMA progress 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..11c9c3259220b 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c
[ ... ]
> @@ -193,7 +213,28 @@ static struct rxe_send_wqe *req_next_wqe(struct rxe_qp *qp)
>  	}
>  	spin_unlock_irqrestore(&qp->state_lock, flags);
>  
> -	wqe->mask = wr_opcode_mask(wqe->wr.opcode, qp);
> +	/* Reuse the existing kernel-private copy if still valid */
> +	if (qp->req.send_wqe_valid && qp->req.shared_wqe == wqe)
> +		return &qp->req.send_wqe.wqe;
> +
> +	/* Copy WQE from userspace-mapped shared queue to kernel-private
> +	 * buffer. Copy size matches the queue element data area.
> +	 */
> +	memcpy(&qp->req.send_wqe, wqe, sizeof(*wqe) +
> +	       max_t(int, qp->sq.max_sge * sizeof(struct ib_sge),
> +		    qp->sq.max_inline));
> +	qp->req.shared_wqe = wqe;
> +	qp->req.send_wqe_valid = true;
> +
> +	wqe = &qp->req.send_wqe.wqe;

[Severity: Critical]
As reported by Sashiko on v6 [1], can redirecting wqe to qp->req.send_wqe.wqe
cause an out-of-bounds read from qp->req.send_wqe in finish_packet() for an
IB_WR_ATOMIC_WRITE work request?

In rxe_verbs.h, qp->req.send_wqe reserves RXE_MAX_SGE entries (512 bytes)
after wqe:

	struct {
		struct rxe_send_wqe	wqe;
		struct ib_sge		sge[RXE_MAX_SGE];
	} send_wqe;

When a user QP posts a WQE in the mmap'd send queue with wqe->wr.opcode set to
IB_WR_ATOMIC_WRITE, wqe->dma.num_sge = 0, wqe->dma.cur_sge = 0, and
wqe->dma.resid = 4096, the num_sge and cur_sge check in rxe_requester() passes
when qp->sq.max_sge >= 1.

rxe_requester() then computes payload from wqe->dma.resid and only caps it at
mtu (up to 4096 bytes):

	mtu = get_mtu(qp);
	payload = (mask & (RXE_WRITE_OR_SEND_MASK | RXE_ATOMIC_WRITE_MASK)) ?
			wqe->dma.resid : 0;
	if (payload > mtu) {
		...
		payload = mtu;
	}

When rxe_requester() calls finish_packet(), the RXE_ATOMIC_WRITE_MASK branch
copies payload bytes directly from wqe->dma.atomic_wr without checking that
payload is sizeof(u64) (8 bytes) or fits within the copied buffer:

	if (pkt->mask & RXE_ATOMIC_WRITE_MASK) {
		memcpy(payload_addr(pkt), wqe->dma.atomic_wr, payload);
		wqe->dma.resid -= payload;
	}

If payload exceeds the copied tail size, memcpy() reads uninitialized bytes
from qp->req.send_wqe.sge, and if payload exceeds 512 bytes, it reads past the
end of qp->req.send_wqe into the rest of struct rxe_qp and adjacent slab
memory before transmitting the packet. Should finish_packet() or
rxe_requester() validate that payload == 8 for RXE_ATOMIC_WRITE_MASK and fits
within the copied WQE data area?

[1] https://lore.kernel.org/r/20261026001405.1698455-2-tristan@talencesecurity.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030151558.1458444-1-tristan@talencesecurity.com?part=1