Re: [PATCH v5 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 3 potential issue(s) to consider:
- [High] rxe: kernel heap leak via unvalidated inline WQE fields
- [High] rxe: out-of-bounds read in wr_opcode_mask() via untrusted userspace WQE opcode
- [Medium] rdma/rxe: Missing memory barrier in IB_QPS_ERR flush path violates concurrency model
--

commit ffc0feb820a2586eb777f9f2b8fd3b594b8c06e6
Author: Tristan Madani <tristan@talencesecurity.com>

RDMA/rxe: copy send WQE to kernel buffer before processing

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

> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index 53f7c1e7fe55b..5bd6bcad2ab97 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c

[ ... ]

> @@ -193,6 +213,20 @@ 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;
>  	wqe->mask = wr_opcode_mask(wqe->wr.opcode, qp);

[Severity: High]
Does this code perform an out-of-bounds read of rxe_wr_opcode_info[]?

The opcode in the WQE comes directly from userspace without validation. It is
passed to wr_opcode_mask():

drivers/infiniband/sw/rxe/rxe_loc.h:wr_opcode_mask() {
    return rxe_wr_opcode_info[opcode].mask[qp->ibqp.qp_type];
}

If a malicious user provides an out-of-bounds opcode (e.g. 0xFFFFFFFF), could
it read unmapped kernel memory or use garbage memory as an opcode mask?

[ ... ]

> @@ -666,6 +706,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;

[Severity: Medium]
Does this bypass the required memory barriers for updating the WQE in shared
memory?

Since qp->req.send_wqe_valid is set to false here, the subsequent call to
rxe_req_writeback_wqe() at the err label will be a no-op. The WQE state and
status are directly modified on the shared memory WQE:

err:
	wqe->state = wqe_state_error;
	rxe_req_writeback_wqe(qp);

Without the intended smp_store_release() or WRITE_ONCE() macros, could
the completer or userspace observe wqe_state_error before the status update
is visible?

>  		} else {
>  			goto exit;
>  		}

[ ... ]

> @@ -695,6 +737,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);

[Severity: High]
Does this code allow an out-of-bounds read of the kernel heap memory when
processing IB_SEND_INLINE WQEs?

Just below this in rxe_requester(), num_sge and cur_sge are bounds checked,
but sge_offset and resid are not validated against qp->sq.max_inline:

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");
	...
}

Because this patch relocates the WQE into a kmalloc'd kernel buffer, can
a malicious user supply large values for sge_offset and resid to leak
adjacent kernel heap memory when the payload is copied to the network packet?

drivers/infiniband/sw/rxe/rxe_req.c:finish_packet() {
        ...
		if (wqe->wr.send_flags & IB_SEND_INLINE) {
			u8 *tmp = &wqe->dma.inline_data[wqe->dma.sge_offset];

			memcpy(payload_addr(pkt), tmp, payload);
        ...
}

[ ... ]

> @@ -839,8 +884,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;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008093740.3034881-1-tristmd@gmail.com?part=1