[PATCH for-rc 0/7] RDMA/bnxt_re: Bug fixes for RCFW completion handling and verbs

[PATCH for-rc 0/7] RDMA/bnxt_re: Bug fixes for RCFW completion handling and verbs

[Selvin Xavier]

Hi,

This series contains a set of fixes for the bnxt_re driver.

Patches 1-4 harden the RCFW command path: they clear a stale response
pointer when a wait ends, honor a completion that races with a wait
timeout, ignore CQ error notifications that are not command completions,
and drop completions for cookies with no outstanding command.

Patches 5-7 are independent verbs fixes: reset the cqe pointer on each
poll_cq iteration, remove the shared page mmap entry when
alloc_ucontext fails, and bound the FRMR page list by max_num_sg.

Please review.

Thanks,
Selvin Xavier

Saravanan Vajravel (1):
  RDMA/bnxt_re: Drop RCFW completions for cookies with no outstanding
    command

Selvin Xavier (6):
  RDMA/bnxt_re: Clear stale RCFW resp pointer when wait ends
  RDMA/bnxt_re: Honor a completion that races with a wait timeout
  RDMA/bnxt_re: Do not treat CQ error notification as a command
    completion
  RDMA/bnxt_re: Reset cqe pointer each poll_cq iteration
  RDMA/bnxt_re: Remove shared page mmap entry on alloc_ucontext failure
  RDMA/bnxt_re: Bound FRMR page list by max_num_sg

 drivers/infiniband/hw/bnxt_re/ib_verbs.c   |  9 ++-
 drivers/infiniband/hw/bnxt_re/ib_verbs.h   |  1 +
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 73 +++++++++++++++++-----
 3 files changed, 66 insertions(+), 17 deletions(-)

-- 
2.39.3

[PATCH for-rc 1/7] RDMA/bnxt_re: Clear stale RCFW resp pointer when wait ends

[Selvin Xavier]

If the wait ends with ERR_DEVICE_DETACHED, bnxt_qplib_map_rc() returns 0
for destroy-type opcodes, so the caller sees a synthetic success without
a CREQ. is_waiter_alive and resp are left set, resp keeps pointing at
the caller's stack, and a late CREQ is copied into a dead frame.

After the wait, take hwq->lock and, if the slot still holds this
command's response buffer, clear is_waiter_alive and resp. This also
covers the timeout case, where it is only defensive since is_waiter_alive
was already cleared there. Return success without checking evnt->status
when the slot still holds the buffer, since no completion was processed.

Also reset the slot in the completion handler, in the same critical
section that clears is_in_used, so that a second CREQ for the same cookie
cannot be copied into the caller's buffer. With this, resp == evnt under
the lock means no completion was processed.

Clear crsqe->resp when __send_message_no_waiter() takes a slot as well, so
a stale waiter can never match an internal command's slot by its own
response buffer pointer.

Fixes: 3022cc151197 ("RDMA/bnxt_re: Avoid the command wait if firmware is inactive")
Fixes: 691eb7c6110f ("RDMA/bnxt_re: handle command completions after driver detect a timedout")
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 26 ++++++++++++++--------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 34d8aa12265d..707687a8f301 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -255,6 +255,7 @@ static void __send_message_no_waiter(struct bnxt_qplib_rcfw *rcfw,
 	 */
 	crsqe->is_internal_cmd = true;
 	crsqe->is_waiter_alive = false;
+	crsqe->resp = NULL;
 	crsqe->is_in_used = true;
 	crsqe->req_size = __get_cmdq_base_cmd_size(msg->req, msg->req_sz);
 
@@ -487,6 +488,7 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 {
 	struct creq_qp_event *evnt = (struct creq_qp_event *)msg->resp;
 	struct bnxt_qplib_crsqe *crsqe;
+	bool pending;
 	u16 cookie;
 	int rc;
 	u8 opcode;
@@ -511,15 +513,21 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 	else
 		rc = __poll_for_resp(rcfw, cookie);
 
-	if (rc) {
-		spin_lock_bh(&rcfw->cmdq.hwq.lock);
-		crsqe = &rcfw->crsqe_tbl[cookie];
+	spin_lock_bh(&rcfw->cmdq.hwq.lock);
+	crsqe = &rcfw->crsqe_tbl[cookie];
+	pending = false;
+	if (crsqe->resp == evnt) {
+		pending = true;
 		crsqe->is_waiter_alive = false;
-		if (rc == -ENODEV)
-			set_bit(FIRMWARE_STALL_DETECTED, &rcfw->cmdq.flags);
-		spin_unlock_bh(&rcfw->cmdq.hwq.lock);
-		return -ETIMEDOUT;
+		crsqe->resp = NULL;
 	}
+	if (rc == -ENODEV)
+		set_bit(FIRMWARE_STALL_DETECTED, &rcfw->cmdq.flags);
+	spin_unlock_bh(&rcfw->cmdq.hwq.lock);
+	if (rc)
+		return -ETIMEDOUT;
+	if (pending)
+		return 0;
 
 	if (evnt->status) {
 		/* failed with status */
@@ -703,8 +711,8 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 		is_waiter_alive = crsqe->is_waiter_alive;
 
 		crsqe->req_size = 0;
-		if (!is_waiter_alive)
-			crsqe->resp = NULL;
+		crsqe->resp = NULL;
+		crsqe->is_waiter_alive = false;
 
 		crsqe->is_in_used = false;
 
-- 
2.39.3

Re: [PATCH for-rc 1/7] RDMA/bnxt_re: Clear stale RCFW resp pointer when wait ends

[sashiko-bot]

> If the wait ends with ERR_DEVICE_DETACHED, bnxt_qplib_map_rc() returns 0
> for destroy-type opcodes, so the caller sees a synthetic success without
> a CREQ. is_waiter_alive and resp are left set, resp keeps pointing at
> the caller's stack, and a late CREQ is copied into a dead frame.
> 
> After the wait, take hwq->lock and, if the slot still holds this
> command's response buffer, clear is_waiter_alive and resp. This also
> covers the timeout case, where it is only defensive since is_waiter_alive
> was already cleared there. Return success without checking evnt->status
> when the slot still holds the buffer, since no completion was processed.
> 
> Also reset the slot in the completion handler, in the same critical
> section that clears is_in_used, so that a second CREQ for the same cookie
> cannot be copied into the caller's buffer. With this, resp == evnt under
> the lock means no completion was processed.
> 
> Clear crsqe->resp when __send_message_no_waiter() takes a slot as well, so
> a stale waiter can never match an internal command's slot by its own
> response buffer pointer.
> 
> Fixes: 3022cc151197 ("RDMA/bnxt_re: Avoid the command wait if firmware is inactive")
> Fixes: 691eb7c6110f ("RDMA/bnxt_re: handle command completions after driver detect a timedout")
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028113404.33289-1-selvin.xavier@broadcom.com?part=1

[PATCH for-rc 2/7] RDMA/bnxt_re: Honor a completion that races with a wait timeout

[Selvin Xavier]

If the wait helper gives up (timeout or firmware stall) and the CREQ
tasklet processes the command before the waiter takes hwq->lock, the
response is already copied into the caller's buffer and is_in_used is
clear. The waiter still returns -ETIMEDOUT and may set
FIRMWARE_STALL_DETECTED, and for CREATE_AH the handler skips
destroy_timedout_ah() because the waiter was alive, so the AH leaks in
firmware.

Under the lock, if the slot no longer holds this command's response
buffer, a completion was delivered. Use its status instead of failing.

To keep that check valid, never reuse a cookie slot that is still in
use: __send_message() returns -EAGAIN and __send_message_no_waiter()
drops the internal command.

A slot stays in use only while its completion is outstanding. If that
completion never arrives, the cause is a PCI error or firmware still
processing the command, so there is no point in posting more commands.
Continuing to return -EAGAIN until firmware completes the command in
order is intended; otherwise recovery is initiated.

An internal destroy_ah dropped for the same reason leaks the AH in
firmware; this is accepted and the comment in
bnxt_qplib_service_creq() is updated to say so.

Fixes: 691eb7c6110f ("RDMA/bnxt_re: handle command completions after driver detect a timedout")
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 31 ++++++++++++++++++----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 707687a8f301..167626246800 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -223,7 +223,7 @@ static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
  * Current design of this function is -
  * user must hold the completion queue hwq->lock.
  * user must have used existing completion and free the resources.
- * this function will not check queue full condition.
+ * the command is dropped if the cookie slot is in use or the queue is full.
  * this function will explicitly set is_waiter_alive=false.
  * current use case is - send destroy_ah if create_ah is return
  * after waiter of create_ah is lost. It can be extended for other
@@ -245,9 +245,17 @@ static void __send_message_no_waiter(struct bnxt_qplib_rcfw *rcfw,
 	u8 *preq;
 
 	cookie = cmdq->seq_num & RCFW_MAX_COOKIE_VALUE;
-	__set_cmdq_base_cookie(msg->req, msg->req_sz, cpu_to_le16(cookie));
 	crsqe = &rcfw->crsqe_tbl[cookie];
 
+	if (crsqe->is_in_used ||
+	    bnxt_qplib_get_cmd_slots(msg->req) >= HWQ_FREE_SLOTS(hwq)) {
+		dev_warn_ratelimited(&rcfw->pdev->dev,
+				     "CMDQ cookie %#x busy or CMDQ full, dropping internal command\n",
+				     cookie);
+		return;
+	}
+	__set_cmdq_base_cookie(msg->req, msg->req_sz, cpu_to_le16(cookie));
+
 	/* Set cmd_size in terms of 16B slots in req. */
 	bsize = bnxt_qplib_set_cmd_slots(msg->req);
 	/* GET_CMD_SIZE would return number of slots in either case of tlv
@@ -314,6 +322,12 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 		spin_unlock_bh(&hwq->lock);
 		return -EAGAIN;
 	}
+	if (crsqe->is_in_used) {
+		dev_info_ratelimited(&pdev->dev,
+				     "CMDQ cookie %#x still in use\n", cookie);
+		spin_unlock_bh(&hwq->lock);
+		return -EAGAIN;
+	}
 	if (msg->block)
 		cookie |= RCFW_CMD_IS_BLOCKING;
 	__set_cmdq_base_cookie(msg->req, msg->req_sz, cpu_to_le16(cookie));
@@ -520,6 +534,11 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 		pending = true;
 		crsqe->is_waiter_alive = false;
 		crsqe->resp = NULL;
+	} else if (rc) {
+		/* The completion was processed after the wait gave up but
+		 * before the lock was taken. The response is valid.
+		 */
+		rc = 0;
 	}
 	if (rc == -ENODEV)
 		set_bit(FIRMWARE_STALL_DETECTED, &rcfw->cmdq.flags);
@@ -726,9 +745,11 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 		 * so let's destroy the same address vector since it is
 		 * no more used in stack. We don't care about completion
 		 * in __send_message_no_waiter.
-		 * If destroy_ah is failued by firmware, there will be AH
-		 * resource leak and relatively not critical +  unlikely
-		 * scenario. Current design is not to handle such case.
+		 * If destroy_ah is failed by firmware, or is dropped locally
+		 * because the cookie slot is in use or the CMDQ is full,
+		 * there will be AH resource leak and relatively not
+		 * critical + unlikely scenario. Current design is not to
+		 * handle such case.
 		 */
 		if (!is_waiter_alive && !qp_event->status &&
 		    qp_event->event == CREQ_QP_EVENT_EVENT_CREATE_AH)
-- 
2.39.3

Re: [PATCH for-rc 2/7] RDMA/bnxt_re: Honor a completion that races with a wait timeout

[sashiko-bot]

> If the wait helper gives up (timeout or firmware stall) and the CREQ
> tasklet processes the command before the waiter takes hwq->lock, the
> response is already copied into the caller's buffer and is_in_used is
> clear. The waiter still returns -ETIMEDOUT and may set
> FIRMWARE_STALL_DETECTED, and for CREATE_AH the handler skips
> destroy_timedout_ah() because the waiter was alive, so the AH leaks in
> firmware.
> 
> Under the lock, if the slot no longer holds this command's response
> buffer, a completion was delivered. Use its status instead of failing.
> 
> To keep that check valid, never reuse a cookie slot that is still in
> use: __send_message() returns -EAGAIN and __send_message_no_waiter()
> drops the internal command.
> 
> A slot stays in use only while its completion is outstanding. If that
> completion never arrives, the cause is a PCI error or firmware still
> processing the command, so there is no point in posting more commands.
> Continuing to return -EAGAIN until firmware completes the command in
> order is intended; otherwise recovery is initiated.
> 
> An internal destroy_ah dropped for the same reason leaks the AH in
> firmware; this is accepted and the comment in
> bnxt_qplib_service_creq() is updated to say so.
> 
> Fixes: 691eb7c6110f ("RDMA/bnxt_re: handle command completions after driver detect a timedout")
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028113404.33289-1-selvin.xavier@broadcom.com?part=2

[PATCH for-rc 3/7] RDMA/bnxt_re: Do not treat CQ error notification as a command completion

[Selvin Xavier]

creq_cq_error_notification has the same type as creq_qp_event, and its
cq_err_reason (1-6) sits where creq_qp_event carries the cookie. With
no case for the event in bnxt_qplib_process_qp_event(), it is handled
as a completion for cookie 1-6 and can corrupt an outstanding command.

Handle the event explicitly and do not touch crsqe_tbl. The event is
intentionally not reported to the stack for now; that will be handled
in a later series.

Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 167626246800..da6c7779ff87 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -647,6 +647,7 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 				       struct creq_qp_event *qp_event,
 				       u32 *num_wait)
 {
+	struct creq_cq_error_notification *cq_err_event;
 	struct creq_qp_error_notification *err_event;
 	struct bnxt_qplib_hwq *hwq = &rcfw->cmdq.hwq;
 	struct bnxt_qplib_crsqe *crsqe;
@@ -661,6 +662,13 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 	pdev = rcfw->pdev;
 	print_hex_dump_bytes("event: ", DUMP_PREFIX_OFFSET, qp_event, sizeof(*qp_event));
 	switch (qp_event->event) {
+	case CREQ_QP_EVENT_EVENT_CQ_ERROR_NOTIFICATION:
+		cq_err_event = (struct creq_cq_error_notification *)qp_event;
+		dev_dbg(&pdev->dev,
+			"Received CQ error notification cqid 0x%x, reason 0x%x\n",
+			le32_to_cpu(cq_err_event->xid),
+			cq_err_event->cq_err_reason);
+		break;
 	case CREQ_QP_EVENT_EVENT_QP_ERROR_NOTIFICATION:
 		err_event = (struct creq_qp_error_notification *)qp_event;
 		qp_id = le32_to_cpu(err_event->xid);
-- 
2.39.3

Re: [PATCH for-rc 3/7] RDMA/bnxt_re: Do not treat CQ error notification as a command completion

[sashiko-bot]

> creq_cq_error_notification has the same type as creq_qp_event, and its
> cq_err_reason (1-6) sits where creq_qp_event carries the cookie. With
> no case for the event in bnxt_qplib_process_qp_event(), it is handled
> as a completion for cookie 1-6 and can corrupt an outstanding command.
> 
> Handle the event explicitly and do not touch crsqe_tbl. The event is
> intentionally not reported to the stack for now; that will be handled
> in a later series.
> 
> Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028113404.33289-1-selvin.xavier@broadcom.com?part=3

[PATCH for-rc 4/7] RDMA/bnxt_re: Drop RCFW completions for cookies with no outstanding command

[Selvin Xavier]

From: Saravanan Vajravel <saravanan.vajravel@broadcom.com>

A duplicate CREQ completion for an idle cookie slot is processed as a
command response. It can decrement timeout_send a second time and, for
CREATE_AH, send a spurious DESTROY_AH. Log and drop such completions.

Fixes: 691eb7c6110f ("RDMA/bnxt_re: handle command completions after driver detect a timedout")
Signed-off-by: Saravanan Vajravel <saravanan.vajravel@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index da6c7779ff87..7278e85a8a40 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -718,6 +718,14 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 			return rc;
 		}
 
+		if (!crsqe->is_in_used) {
+			dev_warn_ratelimited(&pdev->dev,
+					     "event %#x for cookie %#x with no outstanding command\n",
+					     qp_event->event, cookie);
+			spin_unlock(&hwq->lock);
+			return rc;
+		}
+
 		if (crsqe->is_internal_cmd && !qp_event->status)
 			atomic_dec(&rcfw->timeout_send);
 
-- 
2.39.3

Re: [PATCH for-rc 4/7] RDMA/bnxt_re: Drop RCFW completions for cookies with no outstanding command

[sashiko-bot]

> From: Saravanan Vajravel <saravanan.vajravel@broadcom.com>
> 
> A duplicate CREQ completion for an idle cookie slot is processed as a
> command response. It can decrement timeout_send a second time and, for
> CREATE_AH, send a spurious DESTROY_AH. Log and drop such completions.
> 
> Fixes: 691eb7c6110f ("RDMA/bnxt_re: handle command completions after driver detect a timedout")
> Signed-off-by: Saravanan Vajravel <saravanan.vajravel@broadcom.com>
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028113404.33289-1-selvin.xavier@broadcom.com?part=4

[PATCH for-rc 5/7] RDMA/bnxt_re: Reset cqe pointer each poll_cq iteration

[Selvin Xavier]

In bnxt_re_poll_cq(), cqe advances for every polled CQE, but budget is
decremented only for completions reported to the caller. Skipped CQEs
(looped-back QP1 receive, shadow-QP completions, unknown opcode) leave
cqe ahead of where budget implies, so the next poll can write past the
end of cq->cql[].

cql[] is only scratch space, so reset cqe to the start of the array on
each iteration of the outer loop.

Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index ac15372376a2..796ea795c5b7 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -4328,8 +4328,8 @@ int bnxt_re_poll_cq(struct ib_cq *ib_cq, int num_entries, struct ib_wc *wc)
 		ibdev_err(&cq->rdev->ibdev, "POLL CQ : no CQL to use");
 		goto exit;
 	}
-	cqe = &cq->cql[0];
 	while (budget) {
+		cqe = &cq->cql[0];
 		lib_qp = NULL;
 		ncqe = bnxt_qplib_poll_cq(&cq->qplib_cq, cqe, budget, &lib_qp);
 		if (lib_qp) {
-- 
2.39.3

Re: [PATCH for-rc 5/7] RDMA/bnxt_re: Reset cqe pointer each poll_cq iteration

[sashiko-bot]

> In bnxt_re_poll_cq(), cqe advances for every polled CQE, but budget is
> decremented only for completions reported to the caller. Skipped CQEs
> (looped-back QP1 receive, shadow-QP completions, unknown opcode) leave
> cqe ahead of where budget implies, so the next poll can write past the
> end of cq->cql[].
> 
> cql[] is only scratch space, so reset cqe to the start of the array on
> each iteration of the outer loop.
> 
> Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028113404.33289-1-selvin.xavier@broadcom.com?part=5

[PATCH for-rc 6/7] RDMA/bnxt_re: Remove shared page mmap entry on alloc_ucontext failure

[Selvin Xavier]

The uverbs core does not call dealloc_ucontext() when alloc_ucontext()
fails. If bnxt_re_alloc_ucontext() fails after inserting the shared page
mmap entry, the entry is leaked. Remove it in the failure path.

Fixes: 24ce94782c48 ("RDMA/bnxt_re: Use the common mmap helper functions")
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index 796ea795c5b7..1ca481806578 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -4909,6 +4909,8 @@ int bnxt_re_alloc_ucontext(struct ib_ucontext *ctx, struct ib_udata *udata)
 
 	return 0;
 cfail:
+	rdma_user_mmap_entry_remove(uctx->shpage_mmap);
+	uctx->shpage_mmap = NULL;
 	free_page((unsigned long)uctx->shpg);
 	uctx->shpg = NULL;
 fail:
-- 
2.39.3

Re: [PATCH for-rc 6/7] RDMA/bnxt_re: Remove shared page mmap entry on alloc_ucontext failure

[sashiko-bot]

> The uverbs core does not call dealloc_ucontext() when alloc_ucontext()
> fails. If bnxt_re_alloc_ucontext() fails after inserting the shared page
> mmap entry, the entry is leaked. Remove it in the failure path.
> 
> Fixes: 24ce94782c48 ("RDMA/bnxt_re: Use the common mmap helper functions")
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028113404.33289-1-selvin.xavier@broadcom.com?part=6

[PATCH for-rc 7/7] RDMA/bnxt_re: Bound FRMR page list by max_num_sg

[Selvin Xavier]

bnxt_re_set_page() checks against frpl->max_pg_ptrs, which is rounded up
to a power of two, while mr->pages[] is allocated with max_num_sg
entries. For a non-power-of-two max_num_sg, ib_sg_to_pages() can write
past the end of mr->pages[].

Track the allocated size in mr->max_pages and bound against it. Also
reject max_num_sg of 0.

Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 5 +++--
 drivers/infiniband/hw/bnxt_re/ib_verbs.h | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index 1ca481806578..811f59f02f75 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -4555,7 +4555,7 @@ static int bnxt_re_set_page(struct ib_mr *ib_mr, u64 addr)
 {
 	struct bnxt_re_mr *mr = container_of(ib_mr, struct bnxt_re_mr, ib_mr);
 
-	if (unlikely(mr->npages == mr->qplib_frpl.max_pg_ptrs))
+	if (unlikely(mr->npages == mr->max_pages))
 		return -ENOMEM;
 
 	mr->pages[mr->npages++] = addr;
@@ -4584,7 +4584,7 @@ struct ib_mr *bnxt_re_alloc_mr(struct ib_pd *ib_pd, enum ib_mr_type type,
 		ibdev_dbg(&rdev->ibdev, "MR type 0x%x not supported", type);
 		return ERR_PTR(-EINVAL);
 	}
-	if (max_num_sg > MAX_PBL_LVL_1_PGS)
+	if (!max_num_sg || max_num_sg > MAX_PBL_LVL_1_PGS)
 		return ERR_PTR(-EINVAL);
 
 	mr = kzalloc_obj(*mr);
@@ -4603,6 +4603,7 @@ struct ib_mr *bnxt_re_alloc_mr(struct ib_pd *ib_pd, enum ib_mr_type type,
 	mr->ib_mr.lkey = mr->qplib_mr.lkey;
 	mr->ib_mr.rkey = mr->ib_mr.lkey;
 
+	mr->max_pages = max_num_sg;
 	mr->pages = kzalloc_objs(u64, max_num_sg);
 	if (!mr->pages) {
 		rc = -ENOMEM;
diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.h b/drivers/infiniband/hw/bnxt_re/ib_verbs.h
index b7b33f6acf91..d12cadaef10c 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.h
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.h
@@ -124,6 +124,7 @@ struct bnxt_re_mr {
 	struct ib_umem		*ib_umem;
 	struct bnxt_qplib_mrw	qplib_mr;
 	u32			npages;
+	u32			max_pages;
 	u64			*pages;
 	struct bnxt_qplib_frpl	qplib_frpl;
 };
-- 
2.39.3

Re: [PATCH for-rc 7/7] RDMA/bnxt_re: Bound FRMR page list by max_num_sg

[sashiko-bot]

> bnxt_re_set_page() checks against frpl->max_pg_ptrs, which is rounded up
> to a power of two, while mr->pages[] is allocated with max_num_sg
> entries. For a non-power-of-two max_num_sg, ib_sg_to_pages() can write
> past the end of mr->pages[].
> 
> Track the allocated size in mr->max_pages and bound against it. Also
> reject max_num_sg of 0.
> 
> Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028113404.33289-1-selvin.xavier@broadcom.com?part=7