[PATCH for-rc 0/8] RDMA/bnxt_re: Fix input validation and lifetime bugs

[PATCH for-rc 0/8] RDMA/bnxt_re: Fix input validation and lifetime bugs

[Selvin Xavier]

This series fixes some of the bugs found while auditing bnxt_re's handling
of user-supplied values and object/device lifetimes.

Please review and apply the series.

Thanks,
Selvin Xavier

Selvin Xavier (8):
  RDMA/bnxt_re: Reject executable mappings of the DBR and toggle pages
  RDMA/bnxt_re: Detect wrong sge_len passed for inline
  RDMA/bnxt_re: Validate num_sge in bnxt_re_post_srq_recv()
  RDMA/bnxt_re: Validate SRQ max_sge at create time
  RDMA/bnxt_re: Fix rdev lifetime races in  suspend/resume/shutdown
  RDMA/bnxt_re: Fix the PD and DPI table size
  RDMA/bnxt_re: Use bnxt_ext_stats_supported for counter count selection
  RDMA/bnxt_re: Check is_in_used before trusting RCFW completion

 drivers/infiniband/hw/bnxt_re/hw_counters.c |  6 ++--
 drivers/infiniband/hw/bnxt_re/ib_verbs.c    | 28 ++++++++++++++----
 drivers/infiniband/hw/bnxt_re/main.c        | 32 ++++++++++++++++-----
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c  | 12 +++++---
 drivers/infiniband/hw/bnxt_re/qplib_res.c   |  9 +++---
 drivers/infiniband/hw/bnxt_re/qplib_sp.c    |  2 +-
 6 files changed, 65 insertions(+), 24 deletions(-)

-- 
2.39.3

[PATCH for-rc 1/8] RDMA/bnxt_re: Reject executable mappings of the DBR and toggle pages

[Selvin Xavier]

The BNXT_RE_MMAP_DBR_PAGE and BNXT_RE_MMAP_TOGGLE_PAGE cases of
bnxt_re_mmap() hand out kernel pages that userspace is only supposed to
read. VM_WRITE was already rejected, but VM_EXEC was not, so userspace
could map these kernel pages executable. Reject VM_EXEC as well.
Also, return EPERM instead of EFAULT.

Fixes: 9b66c9af7172 ("RDMA/bnxt_re: Clear VM_MAYWRITE on DBR/toggle page mmap")
CC: Yousef Alhouseen <alhouseenyousef@gmail.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index ccd2702db78b..e39f99434923 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -5057,11 +5057,11 @@ int bnxt_re_mmap(struct ib_ucontext *ib_uctx, struct vm_area_struct *vma)
 		break;
 	case BNXT_RE_MMAP_DBR_PAGE:
 	case BNXT_RE_MMAP_TOGGLE_PAGE:
-		/* Driver doesn't expect write access for user space */
-		if (vma->vm_flags & VM_WRITE) {
-			ret = -EFAULT;
+		/* Driver doesn't expect write and exec access for user space */
+		if (vma->vm_flags & (VM_WRITE | VM_EXEC)) {
+			ret = -EPERM;
 		} else {
-			vm_flags_clear(vma, VM_MAYWRITE);
+			vm_flags_clear(vma, VM_MAYWRITE | VM_MAYEXEC);
 			ret = vm_insert_page(vma, vma->vm_start,
 					     virt_to_page((void *)bnxt_entry->mem_offset));
 		}
-- 
2.39.3

[PATCH for-rc 2/8] RDMA/bnxt_re: Detect wrong sge_len passed for inline

[Selvin Xavier]

Avoid handling wrong sge_len by adding extra check
to see if the passed length is more than the inline
size supported.

Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index e39f99434923..108be3a68b25 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -3168,8 +3168,9 @@ static int bnxt_re_copy_inline_data(struct bnxt_re_dev *rdev,
 				wr->sg_list[i].addr;
 		sge_len = wr->sg_list[i].length;
 
-		if ((sge_len + wqe->inline_len) >
-		    BNXT_QPLIB_SWQE_MAX_INLINE_LENGTH) {
+		if (sge_len > BNXT_QPLIB_SWQE_MAX_INLINE_LENGTH ||
+		    ((sge_len + wqe->inline_len) >
+		    BNXT_QPLIB_SWQE_MAX_INLINE_LENGTH)) {
 			ibdev_err(&rdev->ibdev,
 				  "Inline data size requested > supported value");
 			return -EINVAL;
-- 
2.39.3

[PATCH for-rc 3/8] RDMA/bnxt_re: Validate num_sge in bnxt_re_post_srq_recv()

[Selvin Xavier]

Add a check if max sge for SRQ is exceeding the device
maximum.

Fixes: 37cb11acf1f7 ("RDMA/bnxt_re: Add SRQ support for Broadcom adapters")
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index 108be3a68b25..ab5d06034c20 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -2389,6 +2389,13 @@ int bnxt_re_post_srq_recv(struct ib_srq *ib_srq, const struct ib_recv_wr *wr,
 	while (wr) {
 		/* Transcribe each ib_recv_wr to qplib_swqe */
 		wqe.num_sge = wr->num_sge;
+		if (wr->num_sge > srq->qplib_srq.max_sge) {
+			ibdev_err(&srq->rdev->ibdev,
+				  "Limit exceeded for SRQ Receive SGEs");
+			rc = -EINVAL;
+			*bad_wr = wr;
+			break;
+		}
 		bnxt_re_build_sgl(wr->sg_list, wqe.sg_list, wr->num_sge);
 		wqe.wr_id = wr->wr_id;
 		wqe.type = BNXT_QPLIB_SWQE_TYPE_RECV;
-- 
2.39.3

[PATCH for-rc 4/8] RDMA/bnxt_re: Validate SRQ max_sge at create time

[Selvin Xavier]

bnxt_re_create_srq() validates attr.max_wr but copies attr.max_sge
into srq->qplib_srq.max_sge unvalidated. This lets an unprivileged
user request an arbitrarily large max_sge, defeating the num_sge
check added in bnxt_re_post_srq_recv() and overflowing the fixed
13-entry (BNXT_VAR_MAX_SGE) sg_list[] array in bnxt_re_build_sgl().

Reject max_sge > dev_attr->max_srq_sges at create time, as create_qp
already does for QP send/recv SGEs. Also clamp max_srq_sges itself in
bnxt_qplib_get_dev_attr() to BNXT_STATIC_MAX_SGE (6), the actual max
SGEs an SRQ ring entry supports, since it was taken straight from
firmware with no bound.

Fixes: 37cb11acf1f7 ("RDMA/bnxt_re: Add SRQ support for Broadcom adapters")
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 8 ++++++++
 drivers/infiniband/hw/bnxt_re/qplib_sp.c | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index ab5d06034c20..b3aa3c9f8941 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -2235,6 +2235,14 @@ int bnxt_re_create_srq(struct ib_srq *ib_srq,
 		goto exit;
 	}
 
+	if (srq_init_attr->attr.max_sge > dev_attr->max_srq_sges) {
+		ibdev_err(&rdev->ibdev,
+			  "Create SRQ failed - max_sge %d exceeds supported %d",
+			  srq_init_attr->attr.max_sge, dev_attr->max_srq_sges);
+		rc = -EINVAL;
+		goto exit;
+	}
+
 	if (srq_init_attr->srq_type != IB_SRQT_BASIC) {
 		rc = -EOPNOTSUPP;
 		goto exit;
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_sp.c b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
index ec9eb52a8ebf..9aaa2b5204b8 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_sp.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
@@ -160,7 +160,7 @@ int bnxt_qplib_get_dev_attr(struct bnxt_qplib_rcfw *rcfw)
 
 	attr->max_srq = le16_to_cpu(sb->max_srq);
 	attr->max_srq_wqes = le32_to_cpu(sb->max_srq_wr) - 1;
-	attr->max_srq_sges = sb->max_srq_sge;
+	attr->max_srq_sges = min_t(u32, sb->max_srq_sge, BNXT_STATIC_MAX_SGE);
 	attr->max_pkey = 1;
 	attr->max_inline_data = attr->max_qp_sges * sizeof(struct sq_sge);
 	if (!bnxt_qplib_is_chip_gen_p7(rcfw->res->cctx))
-- 
2.39.3

[PATCH for-rc 5/8] RDMA/bnxt_re: Fix rdev lifetime races in suspend/resume/shutdown

[Selvin Xavier]

en_info->rdev is read by unlocked L2 ULP callbacks
(bnxt_re_stop_irq/start_irq/async_notifier), which rely on rdev
being cleared before it is freed, not after. bnxt_re_add_device()
cleared it too late on a bnxt_re_dev_init() failure and
bnxt_re_resume() didn't check the return value, risking a
use-after-free/NULL deref. bnxt_re_suspend() froze and freed rdev
before clearing en_info->rdev, opening the same UAF window.
bnxt_re_shutdown() took no lock, never checked rdev for NULL, never
cleared en_info->rdev, and never freed rdev at all.

Fix the ordering in bnxt_re_add_device()/bnxt_re_suspend(), add the
missing return-value check in bnxt_re_resume(), and rewrite
bnxt_re_shutdown() to take the lock, check for NULL, and reuse
bnxt_re_remove_device() with the same clear-before-free ordering.

Fixes: dee3da3422d5 ("RDMA/bnxt_re: Change aux driver data to en_info to hold more information")
Fixes: cc5b9b48d447 ("RDMA/bnxt_re: Recover the device when FW error is detected")
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/main.c | 32 ++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 17654a9e23fe..91c2edebdb93 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -2433,11 +2433,14 @@ static int bnxt_re_add_device(struct auxiliary_device *adev, u8 op_type)
 	bnxt_re_update_en_info_rdev(rdev, en_info, adev);
 
 	rc = bnxt_re_dev_init(rdev, op_type);
-	if (rc)
+	if (rc) {
+		bnxt_re_update_en_info_rdev(NULL, en_info, adev);
 		goto re_dev_dealloc;
+	}
 
 	rc = bnxt_re_ib_init(rdev);
 	if (rc) {
+		bnxt_re_update_en_info_rdev(NULL, en_info, adev);
 		pr_err("Failed to register with IB: %s",
 			aux_priv->aux_dev.name);
 		goto re_dev_uninit;
@@ -2448,7 +2451,6 @@ static int bnxt_re_add_device(struct auxiliary_device *adev, u8 op_type)
 	return 0;
 
 re_dev_uninit:
-	bnxt_re_update_en_info_rdev(NULL, en_info, adev);
 	bnxt_re_dev_uninit(rdev, BNXT_RE_COMPLETE_REMOVE);
 re_dev_dealloc:
 	ib_dealloc_device(&rdev->ibdev);
@@ -2517,9 +2519,13 @@ static int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
 	struct bnxt_en_dev *en_dev;
 	struct bnxt_re_dev *rdev;
 
+	mutex_lock(&bnxt_re_mutex);
 	rdev = en_info->rdev;
+	if (!rdev) {
+		mutex_unlock(&bnxt_re_mutex);
+		return 0;
+	}
 	en_dev = en_info->en_dev;
-	mutex_lock(&bnxt_re_mutex);
 
 	ibdev_info(&rdev->ibdev, "Handle device suspend call");
 	/* Check the current device state from bnxt_en_dev and move the
@@ -2539,8 +2545,9 @@ static int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
 
 	ibdev_info(&rdev->ibdev, "%s: L2 driver notified to stop en_state 0x%lx",
 		   __func__, en_dev->en_state);
-	bnxt_re_remove_device(rdev, BNXT_RE_PRE_RECOVERY_REMOVE, adev);
+
 	bnxt_re_update_en_info_rdev(NULL, en_info, adev);
+	bnxt_re_remove_device(rdev, BNXT_RE_PRE_RECOVERY_REMOVE, adev);
 	mutex_unlock(&bnxt_re_mutex);
 
 	return 0;
@@ -2550,9 +2557,14 @@ static int bnxt_re_resume(struct auxiliary_device *adev)
 {
 	struct bnxt_re_en_dev_info *en_info = auxiliary_get_drvdata(adev);
 	struct bnxt_re_dev *rdev;
+	int rc;
 
 	mutex_lock(&bnxt_re_mutex);
-	bnxt_re_add_device(adev, BNXT_RE_POST_RECOVERY_INIT);
+	rc = bnxt_re_add_device(adev, BNXT_RE_POST_RECOVERY_INIT);
+	if (rc) {
+		mutex_unlock(&bnxt_re_mutex);
+		return rc;
+	}
 	rdev = en_info->rdev;
 	ibdev_info(&rdev->ibdev, "Device resume completed");
 	mutex_unlock(&bnxt_re_mutex);
@@ -2565,9 +2577,15 @@ static void bnxt_re_shutdown(struct auxiliary_device *adev)
 	struct bnxt_re_en_dev_info *en_info = auxiliary_get_drvdata(adev);
 	struct bnxt_re_dev *rdev;
 
+	mutex_lock(&bnxt_re_mutex);
 	rdev = en_info->rdev;
-	ib_unregister_device(&rdev->ibdev);
-	bnxt_re_dev_uninit(rdev, BNXT_RE_COMPLETE_REMOVE);
+	if (!rdev)
+		goto out;
+
+	bnxt_re_update_en_info_rdev(NULL, en_info, adev);
+	bnxt_re_remove_device(rdev, BNXT_RE_COMPLETE_REMOVE, adev);
+out:
+	mutex_unlock(&bnxt_re_mutex);
 }
 
 static const struct auxiliary_device_id bnxt_re_id_table[] = {
-- 
2.39.3

[PATCH for-rc 6/8] RDMA/bnxt_re: Fix the PD and DPI table size

[Selvin Xavier]

The PD and DPI bitmaps were sized as max >> 3 (bytes),
but bitmap ops (set_bit(), clear_bit(), find_first_bit(),
test_and_set_bit()) operate on whole unsigned long words,
so whenever max isn't a multiple of BITS_PER_LONG,
the buffer under-allocates and the top word's bitops
read/write past the end of the kmalloc()'d buffer.
Most exposed on the DPI table, since dpit->max comes from the
firmware-reported dev_attr->max_dpi with no alignment guarantee.

Fix the size both allocations with BITS_TO_LONGS(max) * sizeof(unsigned long).

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_res.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_res.c b/drivers/infiniband/hw/bnxt_re/qplib_res.c
index 756f8b5f042a..7ff587ce9126 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_res.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_res.c
@@ -45,6 +45,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/if_vlan.h>
 #include <linux/vmalloc.h>
+#include <linux/bitops.h>
 #include <rdma/ib_verbs.h>
 #include <rdma/iter.h>
 
@@ -668,9 +669,9 @@ static int bnxt_qplib_alloc_pd_tbl(struct bnxt_qplib_res *res,
 {
 	u32 bytes;
 
-	bytes = max >> 3;
+	bytes = BITS_TO_LONGS(max) * sizeof(unsigned long);
 	if (!bytes)
-		bytes = 1;
+		bytes = sizeof(unsigned long);
 	pdt->tbl = kmalloc(bytes, GFP_KERNEL);
 	if (!pdt->tbl)
 		return -ENOMEM;
@@ -848,9 +849,9 @@ static int bnxt_qplib_alloc_dpi_tbl(struct bnxt_qplib_res *res,
 	if (!dpit->app_tbl)
 		return -ENOMEM;
 
-	bytes = dpit->max >> 3;
+	bytes = BITS_TO_LONGS(dpit->max) * sizeof(unsigned long);
 	if (!bytes)
-		bytes = 1;
+		bytes = sizeof(unsigned long);
 
 	dpit->tbl = kmalloc(bytes, GFP_KERNEL);
 	if (!dpit->tbl) {
-- 
2.39.3

[PATCH for-rc 7/8] RDMA/bnxt_re: Use bnxt_ext_stats_supported for counter count selection

[Selvin Xavier]

Selecting between BNXT_RE_NUM_EXT_COUNTERS and
BNXT_RE_NUM_STD_COUNTERS relied only on
bnxt_qplib_is_chip_gen_p5_p7(), which doesn't account for the
extended stats capability flag or the PF/VF restriction. Use
bnxt_ext_stats_supported() instead, matching the check already
used to populate the extended stats, so the counter count stays
consistent with what gets filled in.

Fixes: 8238c7bd8420 ("RDMA/bnxt_re: Fix the statistics for Gen P7 VF")
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/hw_counters.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/hw_counters.c b/drivers/infiniband/hw/bnxt_re/hw_counters.c
index 651cf9d0e0c7..575b97030587 100644
--- a/drivers/infiniband/hw/bnxt_re/hw_counters.c
+++ b/drivers/infiniband/hw/bnxt_re/hw_counters.c
@@ -413,7 +413,8 @@ int bnxt_re_ib_get_hw_stats(struct ib_device *ibdev,
 	}
 
 done:
-	return bnxt_qplib_is_chip_gen_p5_p7(rdev->chip_ctx) ?
+	return bnxt_ext_stats_supported(rdev->chip_ctx, rdev->dev_attr->dev_cap_flags,
+					 rdev->is_virtfn) ?
 		BNXT_RE_NUM_EXT_COUNTERS : BNXT_RE_NUM_STD_COUNTERS;
 }
 
@@ -423,7 +424,8 @@ struct rdma_hw_stats *bnxt_re_ib_alloc_hw_port_stats(struct ib_device *ibdev,
 	struct bnxt_re_dev *rdev = to_bnxt_re_dev(ibdev, ibdev);
 	int num_counters = 0;
 
-	if (bnxt_qplib_is_chip_gen_p5_p7(rdev->chip_ctx))
+	if (bnxt_ext_stats_supported(rdev->chip_ctx, rdev->dev_attr->dev_cap_flags,
+				     rdev->is_virtfn))
 		num_counters = BNXT_RE_NUM_EXT_COUNTERS;
 	else
 		num_counters = BNXT_RE_NUM_STD_COUNTERS;
-- 
2.39.3

[PATCH for-rc 8/8] RDMA/bnxt_re: Check is_in_used before trusting RCFW completion

[Selvin Xavier]

Gate the crsqe->resp memcpy in bnxt_qplib_process_qp_event() on
is_in_used in addition to is_waiter_alive, since a late/duplicate
completion for an already-retired cookie could still write through a
stale resp pointer.

Also disarm the waiter and clear resp when a wait call bails out early
on ERR_DEVICE_DETACHED, and require is_in_used before re-triggering
__destroy_timedout_ah(), closing the same stale-cookie exposure
elsewhere in the completion path.

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 | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 9ee0d500436c..0339e55bea88 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -511,14 +511,16 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 	else
 		rc = __poll_for_resp(rcfw, cookie);
 
-	if (rc) {
+	if (rc || test_bit(ERR_DEVICE_DETACHED, &rcfw->cmdq.flags)) {
 		spin_lock_bh(&rcfw->cmdq.hwq.lock);
 		crsqe = &rcfw->crsqe_tbl[cookie];
 		crsqe->is_waiter_alive = false;
+		crsqe->resp = NULL;
 		if (rc == -ENODEV)
 			set_bit(FIRMWARE_STALL_DETECTED, &rcfw->cmdq.flags);
 		spin_unlock_bh(&rcfw->cmdq.hwq.lock);
-		return -ETIMEDOUT;
+		if (rc)
+			return -ETIMEDOUT;
 	}
 
 	if (evnt->status) {
@@ -627,6 +629,7 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 	struct bnxt_qplib_qp *qp;
 	u16 cookie, blocked = 0;
 	bool is_waiter_alive;
+	bool is_in_used;
 	struct pci_dev *pdev;
 	u32 wait_cmds = 0;
 	int rc = 0;
@@ -682,7 +685,7 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 		if (crsqe->is_internal_cmd && !qp_event->status)
 			atomic_dec(&rcfw->timeout_send);
 
-		if (crsqe->is_waiter_alive) {
+		if (crsqe->is_waiter_alive && crsqe->is_in_used) {
 			if (crsqe->resp) {
 				memcpy(crsqe->resp, qp_event, sizeof(*qp_event));
 				/* Insert write memory barrier to ensure that
@@ -697,6 +700,7 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 
 		req_size = crsqe->req_size;
 		is_waiter_alive = crsqe->is_waiter_alive;
+		is_in_used = crsqe->is_in_used;
 
 		crsqe->req_size = 0;
 		if (!is_waiter_alive)
@@ -718,7 +722,7 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 		 * resource leak and relatively not critical +  unlikely
 		 * scenario. Current design is not to handle such case.
 		 */
-		if (!is_waiter_alive && !qp_event->status &&
+		if (is_in_used && !is_waiter_alive && !qp_event->status &&
 		    qp_event->event == CREQ_QP_EVENT_EVENT_CREATE_AH)
 			__destroy_timedout_ah(rcfw,
 					      (struct creq_create_ah_resp *)
-- 
2.39.3

Re: [PATCH for-rc 1/8] RDMA/bnxt_re: Reject executable mappings of the DBR and toggle pages

[Leon Romanovsky]

On Sun, Sep 06, 2026 at 04:06:53PM -0700, Selvin Xavier wrote:
> The BNXT_RE_MMAP_DBR_PAGE and BNXT_RE_MMAP_TOGGLE_PAGE cases of
> bnxt_re_mmap() hand out kernel pages that userspace is only supposed to
> read. VM_WRITE was already rejected, but VM_EXEC was not, so userspace
> could map these kernel pages executable. Reject VM_EXEC as well.
> Also, return EPERM instead of EFAULT.
> 
> Fixes: 9b66c9af7172 ("RDMA/bnxt_re: Clear VM_MAYWRITE on DBR/toggle page mmap")
> CC: Yousef Alhouseen <alhouseenyousef@gmail.com>
> Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> ---
>  drivers/infiniband/hw/bnxt_re/ib_verbs.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> index ccd2702db78b..e39f99434923 100644
> --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> @@ -5057,11 +5057,11 @@ int bnxt_re_mmap(struct ib_ucontext *ib_uctx, struct vm_area_struct *vma)
>  		break;
>  	case BNXT_RE_MMAP_DBR_PAGE:
>  	case BNXT_RE_MMAP_TOGGLE_PAGE:
> -		/* Driver doesn't expect write access for user space */
> -		if (vma->vm_flags & VM_WRITE) {
> -			ret = -EFAULT;
> +		/* Driver doesn't expect write and exec access for user space */
> +		if (vma->vm_flags & (VM_WRITE | VM_EXEC)) {
> +			ret = -EPERM;
>  		} else {
> -			vm_flags_clear(vma, VM_MAYWRITE);
> +			vm_flags_clear(vma, VM_MAYWRITE | VM_MAYEXEC);

This is an opposite to 10bf13c33450 ("RDMA/mlx5: Remove MAYEXEC flag")
commit.

Thanks

>  			ret = vm_insert_page(vma, vma->vm_start,
>  					     virt_to_page((void *)bnxt_entry->mem_offset));
>  		}
> -- 
> 2.39.3
> 

Re: [PATCH for-rc 1/8] RDMA/bnxt_re: Reject executable mappings of the DBR and toggle pages

[Selvin Xavier]

[-- Attachment #1.1: Type: text/plain, Size: 2231 bytes --]

On Tue, Sep 8, 2026 at 7:15 PM Leon Romanovsky <leon@kernel.org> wrote:

> On Sun, Sep 06, 2026 at 04:06:53PM -0700, Selvin Xavier wrote:
> > The BNXT_RE_MMAP_DBR_PAGE and BNXT_RE_MMAP_TOGGLE_PAGE cases of
> > bnxt_re_mmap() hand out kernel pages that userspace is only supposed to
> > read. VM_WRITE was already rejected, but VM_EXEC was not, so userspace
> > could map these kernel pages executable. Reject VM_EXEC as well.
> > Also, return EPERM instead of EFAULT.
> >
> > Fixes: 9b66c9af7172 ("RDMA/bnxt_re: Clear VM_MAYWRITE on DBR/toggle page
> mmap")
> > CC: Yousef Alhouseen <alhouseenyousef@gmail.com>
> > Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > ---
> >  drivers/infiniband/hw/bnxt_re/ib_verbs.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > index ccd2702db78b..e39f99434923 100644
> > --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > @@ -5057,11 +5057,11 @@ int bnxt_re_mmap(struct ib_ucontext *ib_uctx,
> struct vm_area_struct *vma)
> >               break;
> >       case BNXT_RE_MMAP_DBR_PAGE:
> >       case BNXT_RE_MMAP_TOGGLE_PAGE:
> > -             /* Driver doesn't expect write access for user space */
> > -             if (vma->vm_flags & VM_WRITE) {
> > -                     ret = -EFAULT;
> > +             /* Driver doesn't expect write and exec access for user
> space */
> > +             if (vma->vm_flags & (VM_WRITE | VM_EXEC)) {
> > +                     ret = -EPERM;
> >               } else {
> > -                     vm_flags_clear(vma, VM_MAYWRITE);
> > +                     vm_flags_clear(vma, VM_MAYWRITE | VM_MAYEXEC);
>
> This is an opposite to 10bf13c33450 ("RDMA/mlx5: Remove MAYEXEC flag")
> commit.
>
> Thanks
>
ok.. will remove this and post v2.

>
> >                       ret = vm_insert_page(vma, vma->vm_start,
> >                                            virt_to_page((void
> *)bnxt_entry->mem_offset));
> >               }
> > --
> > 2.39.3
> >
>

[-- Attachment #1.2: Type: text/html, Size: 3287 bytes --]

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5473 bytes --]

Re: [PATCH for-rc 1/8] RDMA/bnxt_re: Reject executable mappings of the DBR and toggle pages

[Jason Gunthorpe]

On Tue, Sep 08, 2026 at 04:45:53PM +0300, Leon Romanovsky wrote:
> On Sun, Sep 06, 2026 at 04:06:53PM -0700, Selvin Xavier wrote:
> > The BNXT_RE_MMAP_DBR_PAGE and BNXT_RE_MMAP_TOGGLE_PAGE cases of
> > bnxt_re_mmap() hand out kernel pages that userspace is only supposed to
> > read. VM_WRITE was already rejected, but VM_EXEC was not, so userspace
> > could map these kernel pages executable. Reject VM_EXEC as well.
> > Also, return EPERM instead of EFAULT.
> > 
> > Fixes: 9b66c9af7172 ("RDMA/bnxt_re: Clear VM_MAYWRITE on DBR/toggle page mmap")
> > CC: Yousef Alhouseen <alhouseenyousef@gmail.com>
> > Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > ---
> >  drivers/infiniband/hw/bnxt_re/ib_verbs.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > index ccd2702db78b..e39f99434923 100644
> > --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > @@ -5057,11 +5057,11 @@ int bnxt_re_mmap(struct ib_ucontext *ib_uctx, struct vm_area_struct *vma)
> >  		break;
> >  	case BNXT_RE_MMAP_DBR_PAGE:
> >  	case BNXT_RE_MMAP_TOGGLE_PAGE:
> > -		/* Driver doesn't expect write access for user space */
> > -		if (vma->vm_flags & VM_WRITE) {
> > -			ret = -EFAULT;
> > +		/* Driver doesn't expect write and exec access for user space */
> > +		if (vma->vm_flags & (VM_WRITE | VM_EXEC)) {
> > +			ret = -EPERM;
> >  		} else {
> > -			vm_flags_clear(vma, VM_MAYWRITE);
> > +			vm_flags_clear(vma, VM_MAYWRITE | VM_MAYEXEC);
> 
> This is an opposite to 10bf13c33450 ("RDMA/mlx5: Remove MAYEXEC flag")
> commit.

Why did we do that? These MMIO mmap should never be executable

Jason

Re: [PATCH for-rc 1/8] RDMA/bnxt_re: Reject executable mappings of the DBR and toggle pages

[Leon Romanovsky]

On Wed, Sep 09, 2026 at 10:29:48AM -0300, Jason Gunthorpe wrote:
> On Tue, Sep 08, 2026 at 04:45:53PM +0300, Leon Romanovsky wrote:
> > On Sun, Sep 06, 2026 at 04:06:53PM -0700, Selvin Xavier wrote:
> > > The BNXT_RE_MMAP_DBR_PAGE and BNXT_RE_MMAP_TOGGLE_PAGE cases of
> > > bnxt_re_mmap() hand out kernel pages that userspace is only supposed to
> > > read. VM_WRITE was already rejected, but VM_EXEC was not, so userspace
> > > could map these kernel pages executable. Reject VM_EXEC as well.
> > > Also, return EPERM instead of EFAULT.
> > > 
> > > Fixes: 9b66c9af7172 ("RDMA/bnxt_re: Clear VM_MAYWRITE on DBR/toggle page mmap")
> > > CC: Yousef Alhouseen <alhouseenyousef@gmail.com>
> > > Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > > ---
> > >  drivers/infiniband/hw/bnxt_re/ib_verbs.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > > index ccd2702db78b..e39f99434923 100644
> > > --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > > +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > > @@ -5057,11 +5057,11 @@ int bnxt_re_mmap(struct ib_ucontext *ib_uctx, struct vm_area_struct *vma)
> > >  		break;
> > >  	case BNXT_RE_MMAP_DBR_PAGE:
> > >  	case BNXT_RE_MMAP_TOGGLE_PAGE:
> > > -		/* Driver doesn't expect write access for user space */
> > > -		if (vma->vm_flags & VM_WRITE) {
> > > -			ret = -EFAULT;
> > > +		/* Driver doesn't expect write and exec access for user space */
> > > +		if (vma->vm_flags & (VM_WRITE | VM_EXEC)) {
> > > +			ret = -EPERM;
> > >  		} else {
> > > -			vm_flags_clear(vma, VM_MAYWRITE);
> > > +			vm_flags_clear(vma, VM_MAYWRITE | VM_MAYEXEC);
> > 
> > This is an opposite to 10bf13c33450 ("RDMA/mlx5: Remove MAYEXEC flag")
> > commit.
> 
> Why did we do that? These MMIO mmap should never be executable

I don't remember the rationale, but if I understood my findings correctly,
clearing VM_MAYEXEC broke memcopy.

Thanks

Re: [PATCH for-rc 1/8] RDMA/bnxt_re: Reject executable mappings of the DBR and toggle pages

[Jason Gunthorpe]

On Thu, Sep 10, 2026 at 12:26:51PM +0300, Leon Romanovsky wrote:
> On Wed, Sep 09, 2026 at 10:29:48AM -0300, Jason Gunthorpe wrote:
> > On Tue, Sep 08, 2026 at 04:45:53PM +0300, Leon Romanovsky wrote:
> > > On Sun, Sep 06, 2026 at 04:06:53PM -0700, Selvin Xavier wrote:
> > > > The BNXT_RE_MMAP_DBR_PAGE and BNXT_RE_MMAP_TOGGLE_PAGE cases of
> > > > bnxt_re_mmap() hand out kernel pages that userspace is only supposed to
> > > > read. VM_WRITE was already rejected, but VM_EXEC was not, so userspace
> > > > could map these kernel pages executable. Reject VM_EXEC as well.
> > > > Also, return EPERM instead of EFAULT.
> > > > 
> > > > Fixes: 9b66c9af7172 ("RDMA/bnxt_re: Clear VM_MAYWRITE on DBR/toggle page mmap")
> > > > CC: Yousef Alhouseen <alhouseenyousef@gmail.com>
> > > > Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > > > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > > > ---
> > > >  drivers/infiniband/hw/bnxt_re/ib_verbs.c | 8 ++++----
> > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > > > index ccd2702db78b..e39f99434923 100644
> > > > --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > > > +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > > > @@ -5057,11 +5057,11 @@ int bnxt_re_mmap(struct ib_ucontext *ib_uctx, struct vm_area_struct *vma)
> > > >  		break;
> > > >  	case BNXT_RE_MMAP_DBR_PAGE:
> > > >  	case BNXT_RE_MMAP_TOGGLE_PAGE:
> > > > -		/* Driver doesn't expect write access for user space */
> > > > -		if (vma->vm_flags & VM_WRITE) {
> > > > -			ret = -EFAULT;
> > > > +		/* Driver doesn't expect write and exec access for user space */
> > > > +		if (vma->vm_flags & (VM_WRITE | VM_EXEC)) {
> > > > +			ret = -EPERM;
> > > >  		} else {
> > > > -			vm_flags_clear(vma, VM_MAYWRITE);
> > > > +			vm_flags_clear(vma, VM_MAYWRITE | VM_MAYEXEC);
> > > 
> > > This is an opposite to 10bf13c33450 ("RDMA/mlx5: Remove MAYEXEC flag")
> > > commit.
> > 
> > Why did we do that? These MMIO mmap should never be executable
> 
> I don't remember the rationale, but if I understood my findings correctly,
> clearing VM_MAYEXEC broke memcopy.

That doesn't make sense..

Jason