[PATCH v2 1/2] RDMA/erdma: initialize ret for empty receive WR lists

[PATCH v2 1/2] RDMA/erdma: initialize ret for empty receive WR lists

[Ruoyu Wang]

erdma_post_recv() returns ret after walking the receive work request list.
If the caller passes an empty list, the loop is skipped and ret is not
assigned.

Initialize ret to 0 so an empty receive work request list returns success
instead of stack data.

Fixes: 155055771704 ("RDMA/erdma: Add verbs implementation")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
v2:
- Split the erdma and mana_ib changes into separate patches.
- Add a driver-specific Fixes tag.

 drivers/infiniband/hw/erdma/erdma_qp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/erdma/erdma_qp.c b/drivers/infiniband/hw/erdma/erdma_qp.c
index 25f6c49aec779..e002343832f74 100644
--- a/drivers/infiniband/hw/erdma/erdma_qp.c
+++ b/drivers/infiniband/hw/erdma/erdma_qp.c
@@ -734,7 +734,7 @@ int erdma_post_recv(struct ib_qp *ibqp, const struct ib_recv_wr *recv_wr,
 	const struct ib_recv_wr *wr = recv_wr;
 	struct erdma_qp *qp = to_eqp(ibqp);
 	unsigned long flags;
-	int ret;
+	int ret = 0;
 
 	spin_lock_irqsave(&qp->lock, flags);
 
-- 
2.51.0

[PATCH v2 2/2] RDMA/mana_ib: initialize err for empty send WR lists

[Ruoyu Wang]

mana_ib_post_send() returns err after walking the send work request list.
If the caller passes an empty list, the loop is skipped and err is not
assigned.

Initialize err to 0 so an empty send work request list returns success
instead of stack data.

Fixes: c8017f5b4856 ("RDMA/mana_ib: UD/GSI work requests")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
v2:
- Split the erdma and mana_ib changes into separate patches.
- Add a driver-specific Fixes tag.

 drivers/infiniband/hw/mana/wr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mana/wr.c b/drivers/infiniband/hw/mana/wr.c
index 1813567d3b16c..36a1d506f08f6 100644
--- a/drivers/infiniband/hw/mana/wr.c
+++ b/drivers/infiniband/hw/mana/wr.c
@@ -144,7 +144,7 @@ static int mana_ib_post_send_ud(struct mana_ib_qp *qp, const struct ib_ud_wr *wr
 int mana_ib_post_send(struct ib_qp *ibqp, const struct ib_send_wr *wr,
 		      const struct ib_send_wr **bad_wr)
 {
-	int err;
+	int err = 0;
 	struct mana_ib_qp *qp = container_of(ibqp, struct mana_ib_qp, ibqp);
 
 	for (; wr; wr = wr->next) {
-- 
2.51.0

Re: [PATCH v2 1/2] RDMA/erdma: initialize ret for empty receive WR lists

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The receive work request queue (RQ) can silently overflow because `erdma_post_recv` and `erdma_post_recv_one` lack a bounds check against the queue capacity, and `rq_ci` is not properly tracked during completion polling.
- [High] In `erdma_post_send`, when the send queue is full, `*bad_send_wr` is incorrectly assigned the head of the list (`send_wr`) instead of the failing work request (`wr`).
- [High] Missing `dma_wmb()` before updating the doorbell records (`sq_dbrec` and `rq_dbrec`) in host memory.
--

commit 7fd711bd13dff4553304f89e7b55436f427b3b9d
Author: Ruoyu Wang <ruoyuw560@gmail.com>

RDMA/erdma: initialize ret for empty receive WR lists

erdma_post_recv() returns ret after walking the receive work request list.
If the caller passes an empty list, the loop is skipped and ret is not
assigned. Initialize ret to 0 so an empty receive work request list returns
success instead of stack data.

> diff --git a/drivers/infiniband/hw/erdma/erdma_qp.c b/drivers/infiniband/hw/erdma/erdma_qp.c
> index 25f6c49aec779..e002343832f74 100644
> --- a/drivers/infiniband/hw/erdma/erdma_qp.c
> +++ b/drivers/infiniband/hw/erdma/erdma_qp.c
> @@ -734,7 +734,7 @@ int erdma_post_recv(struct ib_qp *ibqp, const struct ib_recv_wr *recv_wr,
>  	const struct ib_recv_wr *wr = recv_wr;
>  	struct erdma_qp *qp = to_eqp(ibqp);
>  	unsigned long flags;
> -	int ret;
> +	int ret = 0;
>  
>  	spin_lock_irqsave(&qp->lock, flags);
>  

[Severity: High]
This is a pre-existing issue, but does the receive work request queue (RQ)
have a risk of silent overflow?

While erdma_post_send() tracks send queue capacity, erdma_post_recv()
unconditionally posts receive work requests and advances rq_pi without a
bounds check against the queue capacity:

drivers/infiniband/hw/erdma/erdma_qp.c:erdma_post_recv() {
    ...
	while (wr) {
		ret = erdma_post_recv_one(qp, wr);
    ...
}

drivers/infiniband/hw/erdma/erdma_qp.c:erdma_post_recv_one() {
    ...
	qp->kern_qp.rq_pi++;
    ...
}

Additionally, receive queue completions don't seem to update kern_qp->rq_ci:

drivers/infiniband/hw/erdma/erdma_cq.c:erdma_poll_one_cqe() {
    ...
	} else {
		id_table = kern_qp->rwr_tbl;
		depth = qp->attrs.rq_size;
	}
    ...
}

If a kernel consumer posts more receives than the queue capacity, could the
ring buffer wrap around and overwrite active, unpolled work requests, leading
to data corruption and invalid WR IDs during polling?

[Severity: High]
This is a pre-existing issue, but is the failing work request assigned
correctly when the send queue is full?

In erdma_post_send(), if the queue becomes full after some work requests have
already been successfully posted, *bad_send_wr is assigned the head of the
list instead of the failing request:

drivers/infiniband/hw/erdma/erdma_qp.c:erdma_post_send() {
    ...
		if ((u16)(sq_pi - qp->kern_qp.sq_ci) >= qp->attrs.sq_size) {
			ret = -ENOMEM;
			*bad_send_wr = send_wr;
			break;
		}
    ...
}

Does this signal to the caller that the entire batch failed from the start,
prompting a retry of the entire list and potentially leading to duplicate
execution?

[Severity: High]
This is a pre-existing issue, but should there be a memory barrier before
updating the doorbell records in host memory?

The driver updates doorbell records after constructing work queue elements
(WQEs/RQEs):

drivers/infiniband/hw/erdma/erdma_qp.c:erdma_post_recv_one() {
    ...
	*(u64 *)qp->kern_qp.rq_dbrec = *(u64 *)rqe;
    ...
}

drivers/infiniband/hw/erdma/erdma_qp.c:kick_sq_db() {
    ...
	*(u64 *)qp->kern_qp.sq_dbrec = db_data;
    ...
}

While the subsequent hardware doorbell ring via writeq() includes a barrier
before the MMIO operation, could the CPU reorder the memory stores of the WQE
with the memory store of the dbrec?

If the device polls the updated dbrec before the WQE stores are visible,
might it fetch a stale or partially written WQE via DMA?

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