[PATCH rdma-next 0/6] RDMA: Fix restrack UAF in QP/CQ/SRQ destroy

[PATCH rdma-next 0/6] RDMA: Fix restrack UAF in QP/CQ/SRQ destroy

[Edward Srouji]

The resource-tracking (restrack) database is the back-end for the netlink
"rdma resource show" interface which pins objects with
rdma_restrack_get().
The QP/CQ/SRQ destroy flows call rdma_restrack_del() at the end of
ib_destroy_*_user(), after device->ops.destroy_*() had already freed the 
vendor object. Therefore, a concurrent netlink dump could look the
object up and touch freed memory, causing a use-after-free via
ib_query_qp() for instance.

Fix this by splitting the delete into a begin/commit/abort sequence:
begin_del() parks the entry as XA_ZERO_ENTRY (so lookups return NULL),
drops the birth reference and waits for in-flight readers to drain,
while keeping the index reserved. The destroy paths run begin_del()
first, then commit_del() on success or abort_del() on error.
abort_del() re-inserts into the reserved slot, so it needs no allocation
and cannot fail.

The first two patches remove DCT and raw RSS QP restrack tracking as
they have never worked (their ID is unset/reserved at create time).

Signed-off-by: Edward Srouji <edwards@nvidia.com>
---
Patrisious Haddad (6):
      RDMA/mlx5: Remove DCT restrack tracking
      RDMA/mlx5: Remove raw RSS QP restrack tracking
      RDMA/core: Add rdma_restrack_begin/abort/commit_del() operations
      RDMA/core: Fix use after free in ib_query_qp()
      RDMA/core: Fix potential use after free in ib_destroy_cq_user()
      RDMA/core: Fix potential use after free in ib_destroy_srq_user()

 drivers/infiniband/core/restrack.c    | 120 ++++++++++++++++++++++++++++++----
 drivers/infiniband/core/restrack.h    |   3 +
 drivers/infiniband/core/verbs.c       |  21 ++++--
 drivers/infiniband/hw/mlx5/qp.c       |   2 +
 drivers/infiniband/hw/mlx5/restrack.c |   3 -
 5 files changed, 130 insertions(+), 19 deletions(-)
---
base-commit: d6ab440240a04b8737ee4c7bb21af9182e451733
change-id: 20260607-restrack-uaf-fix-d3e0bccf0be1

Best regards,
-- 
Edward Srouji <edwards@nvidia.com>

[PATCH rdma-next 1/6] RDMA/mlx5: Remove DCT restrack tracking

[Edward Srouji]

From: Patrisious Haddad <phaddad@nvidia.com>

DCT restrack tracking wasn't working to begin with as it was only
tracking the first DCT which was added, since at creation the DCT number
isn't yet initialized because the DCT FW object is only created during
modify. The following DCT additions were failing silently.

Since the fix isn't trivial and there were no users that required or
complained about this issue we are dropping this for now instead of fixing.

Fixes: fd3af5e21866 ("RDMA/mlx5: Track DCT, DCI and REG_UMR QPs as diver_detail resources.")
Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
Reviewed-by: Michael Guralnik <michaelgur@nvidia.com>
Signed-off-by: Edward Srouji <edwards@nvidia.com>
---
 drivers/infiniband/hw/mlx5/qp.c       | 1 +
 drivers/infiniband/hw/mlx5/restrack.c | 3 ---
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index e8d34d54b43527e0595ec9e2fb93dc7e9bedba92..a16da733d99fa1f6fdb9ee864465acf45a6abb3d 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -3135,6 +3135,7 @@ static int create_qp(struct mlx5_ib_dev *dev, struct ib_pd *pd,
 
 	switch (qp->type) {
 	case MLX5_IB_QPT_DCT:
+		rdma_restrack_no_track(&qp->ibqp.res);
 		err = create_dct(dev, pd, qp, params);
 		break;
 	case MLX5_IB_QPT_DCI:
diff --git a/drivers/infiniband/hw/mlx5/restrack.c b/drivers/infiniband/hw/mlx5/restrack.c
index 67841922c7b8770c86fb5a47588e09560d0004f5..00a9bcb2603f0b094bcef8a4ffe6564699a85769 100644
--- a/drivers/infiniband/hw/mlx5/restrack.c
+++ b/drivers/infiniband/hw/mlx5/restrack.c
@@ -178,9 +178,6 @@ static int fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ibqp)
 		ret = nla_put_string(msg, RDMA_NLDEV_ATTR_RES_SUBTYPE,
 				     "REG_UMR");
 		break;
-	case MLX5_IB_QPT_DCT:
-		ret = nla_put_string(msg, RDMA_NLDEV_ATTR_RES_SUBTYPE, "DCT");
-		break;
 	case MLX5_IB_QPT_DCI:
 		ret = nla_put_string(msg, RDMA_NLDEV_ATTR_RES_SUBTYPE, "DCI");
 		break;

-- 
2.49.0

[PATCH rdma-next 2/6] RDMA/mlx5: Remove raw RSS QP restrack tracking

[Edward Srouji]

From: Patrisious Haddad <phaddad@nvidia.com>

Raw RSS QP restrack tracking wasn't working to begin with as it was
only tracking the first raw RSS QP which was added, since at creation
the raw RSS QP number is reserved so the QP number for this qp type
was always zero.
The following raw RSS QP additions were always failing silently.

Since the fix isn't trivial and there were no users that required or
complained about this issue we are dropping this for now instead of fixing.

Fixes: 968f0b6f9c01 ("RDMA/mlx5: Consolidate into special function all create QP calls")
Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
Reviewed-by: Michael Guralnik <michaelgur@nvidia.com>
Signed-off-by: Edward Srouji <edwards@nvidia.com>
---
 drivers/infiniband/hw/mlx5/qp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index a16da733d99fa1f6fdb9ee864465acf45a6abb3d..d7fffc0d818f39ca9c75b386811fb016f547a32c 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -3129,6 +3129,7 @@ static int create_qp(struct mlx5_ib_dev *dev, struct ib_pd *pd,
 	int err;
 
 	if (params->is_rss_raw) {
+		rdma_restrack_no_track(&qp->ibqp.res);
 		err = create_rss_raw_qp_tir(dev, pd, qp, params);
 		goto out;
 	}

-- 
2.49.0

[PATCH rdma-next 3/6] RDMA/core: Add rdma_restrack_begin/abort/commit_del() operations

[Edward Srouji]

From: Patrisious Haddad <phaddad@nvidia.com>

Add rdma_restrack_abort_del(), rdma_restrack_begin_del() and
rdma_restrack_commit_del() functions to allow deleting a resource from
the xarray to effectively prevent future access to it and wait for all
current users to finish while preserving its index in the xarray to
allow to re-insert it if needed with guaranteed success.

This is a preparatory change for subsequent patches in the series
which will use these functions to fix the cleanup flow.

Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
Reviewed-by: Michael Guralnik <michaelgur@nvidia.com>
Signed-off-by: Edward Srouji <edwards@nvidia.com>
---
 drivers/infiniband/core/restrack.c | 120 +++++++++++++++++++++++++++++++++----
 drivers/infiniband/core/restrack.h |   3 +
 2 files changed, 112 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index ac3688952cabbff1ebb899bacb78421f2515231b..97c8991081a49ff4f74d65c6c1bd8ca647aa72d1 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -71,6 +71,8 @@ int rdma_restrack_count(struct ib_device *dev, enum rdma_restrack_type type,
 
 	xa_lock(&rt->xa);
 	xas_for_each(&xas, e, U32_MAX) {
+		if (xa_is_zero(e))
+			continue;
 		if (xa_get_mark(&rt->xa, e->id, RESTRACK_DD) && !show_details)
 			continue;
 		cnt++;
@@ -127,6 +129,16 @@ static void rdma_restrack_attach_task(struct rdma_restrack_entry *res,
 	res->user = true;
 }
 
+static struct rdma_restrack_root *res_to_rt(struct rdma_restrack_entry *res)
+{
+	struct ib_device *dev = res_to_dev(res);
+
+	if (WARN_ON(!dev))
+		return NULL;
+
+	return &dev->res[res->type];
+}
+
 /**
  * rdma_restrack_set_name() - set the task for this resource
  * @res:  resource entry
@@ -180,17 +192,15 @@ EXPORT_SYMBOL(rdma_restrack_new);
  */
 void rdma_restrack_add(struct rdma_restrack_entry *res)
 {
-	struct ib_device *dev = res_to_dev(res);
 	struct rdma_restrack_root *rt;
 	int ret = 0;
 
-	if (!dev)
-		return;
-
 	if (res->no_track)
 		goto out;
 
-	rt = &dev->res[res->type];
+	rt = res_to_rt(res);
+	if (!rt)
+		return;
 
 	if (res->type == RDMA_RESTRACK_QP) {
 		/* Special case to ensure that LQPN points to right QP */
@@ -227,6 +237,37 @@ void rdma_restrack_add(struct rdma_restrack_entry *res)
 }
 EXPORT_SYMBOL(rdma_restrack_add);
 
+/**
+ * rdma_restrack_abort_del() - readd object to the resource tracking database
+ * it can only be used after rdma_restrack_begin_del().
+ * @res:  resource entry
+ */
+void rdma_restrack_abort_del(struct rdma_restrack_entry *res)
+{
+	struct rdma_restrack_entry *old;
+	struct rdma_restrack_root *rt;
+
+	if (!res->valid)
+		return;
+
+	if (res->no_track) {
+		rdma_restrack_new(res, res->type);
+		return;
+	}
+
+	rt = res_to_rt(res);
+	if (!rt)
+		return;
+
+	rdma_restrack_new(res, res->type);
+	old = xa_cmpxchg(&rt->xa, res->id, XA_ZERO_ENTRY, res, 0);
+	/* The only way this can fail if someone called this function
+	 * without first calling rdma_restrack_begin_del().
+	 */
+	WARN_ON(old);
+}
+EXPORT_SYMBOL(rdma_restrack_abort_del);
+
 int __must_check rdma_restrack_get(struct rdma_restrack_entry *res)
 {
 	return kref_get_unless_zero(&res->kref);
@@ -263,7 +304,7 @@ static void restrack_release(struct kref *kref)
 	struct rdma_restrack_entry *res;
 
 	res = container_of(kref, struct rdma_restrack_entry, kref);
-	if (res->task) {
+	if (res->task && !res->valid) {
 		put_task_struct(res->task);
 		res->task = NULL;
 	}
@@ -284,7 +325,6 @@ void rdma_restrack_del(struct rdma_restrack_entry *res)
 {
 	struct rdma_restrack_entry *old;
 	struct rdma_restrack_root *rt;
-	struct ib_device *dev;
 
 	if (!res->valid) {
 		if (res->task) {
@@ -297,12 +337,10 @@ void rdma_restrack_del(struct rdma_restrack_entry *res)
 	if (res->no_track)
 		goto out;
 
-	dev = res_to_dev(res);
-	if (WARN_ON(!dev))
+	rt = res_to_rt(res);
+	if (!rt)
 		return;
 
-	rt = &dev->res[res->type];
-
 	old = xa_erase(&rt->xa, res->id);
 	WARN_ON(old != res);
 
@@ -310,5 +348,65 @@ void rdma_restrack_del(struct rdma_restrack_entry *res)
 	res->valid = false;
 	rdma_restrack_put(res);
 	wait_for_completion(&res->comp);
+	if (res->task) {
+		put_task_struct(res->task);
+		res->task = NULL;
+	}
 }
 EXPORT_SYMBOL(rdma_restrack_del);
+
+/**
+ * rdma_restrack_begin_del() - invalidate the object from the resource tracking
+ * database but preserve its index in the array.
+ * @res:  resource entry
+ */
+void rdma_restrack_begin_del(struct rdma_restrack_entry *res)
+{
+	struct rdma_restrack_entry *old;
+	struct rdma_restrack_root *rt;
+
+	if (!res->valid)
+		return;
+
+	if (res->no_track)
+		goto out;
+
+	rt = res_to_rt(res);
+	if (!rt)
+		return;
+
+	old = xa_cmpxchg(&rt->xa, res->id, res, XA_ZERO_ENTRY, 0);
+	WARN_ON(old != res);
+
+out:
+	rdma_restrack_put(res);
+	wait_for_completion(&res->comp);
+}
+EXPORT_SYMBOL(rdma_restrack_begin_del);
+
+/**
+ * rdma_restrack_commit_del() - delete object from the resource tracking
+ * database and free the task.
+ * @res:  resource entry
+ */
+void rdma_restrack_commit_del(struct rdma_restrack_entry *res)
+{
+	struct rdma_restrack_root *rt;
+
+	if (!res->valid || res->no_track)
+		goto out;
+
+	rt = res_to_rt(res);
+	if (!rt)
+		return;
+
+	xa_erase(&rt->xa, res->id);
+
+out:
+	res->valid = false;
+	if (res->task) {
+		put_task_struct(res->task);
+		res->task = NULL;
+	}
+}
+EXPORT_SYMBOL(rdma_restrack_commit_del);
diff --git a/drivers/infiniband/core/restrack.h b/drivers/infiniband/core/restrack.h
index 6a04fc41f738010a90d96f88dbcc88bc36d3a289..45f2f06825f402324304113014fa90da03ec6f88 100644
--- a/drivers/infiniband/core/restrack.h
+++ b/drivers/infiniband/core/restrack.h
@@ -26,7 +26,10 @@ struct rdma_restrack_root {
 int rdma_restrack_init(struct ib_device *dev);
 void rdma_restrack_clean(struct ib_device *dev);
 void rdma_restrack_add(struct rdma_restrack_entry *res);
+void rdma_restrack_abort_del(struct rdma_restrack_entry *res);
 void rdma_restrack_del(struct rdma_restrack_entry *res);
+void rdma_restrack_begin_del(struct rdma_restrack_entry *res);
+void rdma_restrack_commit_del(struct rdma_restrack_entry *res);
 void rdma_restrack_new(struct rdma_restrack_entry *res,
 		       enum rdma_restrack_type type);
 void rdma_restrack_set_name(struct rdma_restrack_entry *res,

-- 
2.49.0

[PATCH rdma-next 4/6] RDMA/core: Fix use after free in ib_query_qp()

[Edward Srouji]

From: Patrisious Haddad <phaddad@nvidia.com>

When querying a QP via the netlink flow the only synchronization
mechanism for the said QP is rdma_restrack_get(), meanwhile during the
QP destroy path rdma_restrack_del() is called at the end of the
ib_destroy_qp_user() function which is too late, since by then the
vendor specific resources for said QP would already be destroyed, and
until the rdma_restrack_del() is called this QP can still be accessed,
which could cause the use after free below.

Fix this by moving the rdma_restrack_begin_del() to the start of the
ib_destroy_qp_user(), which in turn waits for all usages of the QP to be
done then removes it from the database to prevent access to it while it
is being destroyed.

RIP: 0010:ib_query_qp+0x15/0x50 [ib_core]
Code: 48 83 05 5d 8e b9 ff 01 eb b5 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 c7 46 40 00 00 00 00 48 c7 46 78 00 00 00 00 <48> 8b 07 48 8b 80 88 01 00 00 48 85 c0 74 1a 48 83 05 54 91 b9 ff
RSP: 0018:ff11000108a8f2f0 EFLAGS: 00010202
RAX: 0000000000000000 RBX: ff11000108a8f370 RCX: ff11000108a8f370
RDX: 0000000000000000 RSI: ff11000108a8f3d8 RDI: 0000000000000000
RBP: ff1100010de5a000 R08: 0000000000000e80 R09: 0000000000000004
R10: ff110001057a604c R11: 0000000000000000 R12: ff11000108a8f370
R13: ff110001090e8000 R14: 0000000000000000 R15: ff110001057a602c
FS:  00007f2ffd8db6c0(0000) GS:ff110008dc90b000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000010b9a7004 CR4: 0000000000373eb0
Call Trace:
 <TASK>
 mlx5_ib_gsi_query_qp+0x21/0x50 [mlx5_ib]
 mlx5_ib_query_qp+0x689/0x9d0 [mlx5_ib]
 ib_query_qp+0x35/0x50 [ib_core]
 fill_res_qp_entry_query.isra.0+0x47/0x280 [ib_core]
 ? __wake_up+0x40/0x50
 ? netlink_broadcast_filtered+0x15a/0x550
 ? kobject_uevent_env+0x562/0x710
 ? ep_poll_callback+0x242/0x270
 ? __nla_put+0xc/0x20
 ? nla_put+0x28/0x40
 ? nla_put_string+0x2e/0x40 [ib_core]
 fill_res_qp_entry+0x138/0x190 [ib_core]
 res_get_common_dumpit+0x4a5/0x800 [ib_core]
 ? fill_res_qp_entry_query.isra.0+0x280/0x280 [ib_core]
 nldev_res_get_qp_dumpit+0x1e/0x30 [ib_core]
 netlink_dump+0x16f/0x450
 __netlink_dump_start+0x1ce/0x2e0
 rdma_nl_rcv_msg+0x1d3/0x330 [ib_core]
 ? nldev_res_get_qp_raw_dumpit+0x30/0x30 [ib_core]
 rdma_nl_rcv_skb.constprop.0.isra.0+0x108/0x180 [ib_core]
 rdma_nl_rcv+0x12/0x20 [ib_core]
 netlink_unicast+0x255/0x380
 ? __alloc_skb+0xfa/0x1e0
 netlink_sendmsg+0x1f3/0x420
 __sock_sendmsg+0x38/0x60
 ____sys_sendmsg+0x1e8/0x230
 ? copy_msghdr_from_user+0xea/0x170
 ___sys_sendmsg+0x7c/0xb0
 ? __futex_wait+0x95/0xf0
 ? __futex_wake_mark+0x40/0x40
 ? futex_wait+0x67/0x100
 ? futex_wake+0xac/0x1b0
 __sys_sendmsg+0x5f/0xb0
 do_syscall_64+0x55/0xb90
 entry_SYSCALL_64_after_hwframe+0x4b/0x53

Fixes: 514aee660df4 ("RDMA: Globally allocate and release QP memory")
Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
Reviewed-by: Michael Guralnik <michaelgur@nvidia.com>
Signed-off-by: Edward Srouji <edwards@nvidia.com>
---
 drivers/infiniband/core/verbs.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index de7d19fabd75951f0c546accbbb97348e756c235..8bd39cfcf41bce3a20cfbc41be6f51a1f7f95a8a 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -2157,6 +2157,8 @@ int ib_destroy_qp_user(struct ib_qp *qp, struct ib_udata *udata)
 	if (qp->real_qp != qp)
 		return __ib_destroy_shared_qp(qp);
 
+	rdma_restrack_begin_del(&qp->res);
+
 	sec  = qp->qp_sec;
 	if (sec)
 		ib_destroy_qp_security_begin(sec);
@@ -2169,6 +2171,7 @@ int ib_destroy_qp_user(struct ib_qp *qp, struct ib_udata *udata)
 	if (ret) {
 		if (sec)
 			ib_destroy_qp_security_abort(sec);
+		rdma_restrack_abort_del(&qp->res);
 		return ret;
 	}
 
@@ -2181,7 +2184,7 @@ int ib_destroy_qp_user(struct ib_qp *qp, struct ib_udata *udata)
 	if (sec)
 		ib_destroy_qp_security_end(sec);
 
-	rdma_restrack_del(&qp->res);
+	rdma_restrack_commit_del(&qp->res);
 	kfree(qp);
 	return ret;
 }

-- 
2.49.0

[PATCH rdma-next 5/6] RDMA/core: Fix potential use after free in ib_destroy_cq_user()

[Edward Srouji]

From: Patrisious Haddad <phaddad@nvidia.com>

When accessing a CQ via the netlink path the only synchronization
mechanism for the said CQ is rdma_restrack_get().
Currently, rdma_restrack_del() is invoked at the end of
ib_destroy_cq_user(), which is too late, since by that point
vendor-specific resources associated with the CQ might already be
freed. This can leave a short window where the CQ remains accessible
through restrack, leading to a potential use-after-free.

Fix this by moving the rdma_restrack_begin_del() call to the start of
ib_destroy_cq_user(), ensuring that the CQ is removed from restrack
before its internal resources are released. This guarantees that no new
users hold references to a CQ that is in the process of destruction.

In addition, this change preserves the intended asymmetric behavior
between create and destroy routines: resources are added to
restrack at the end of successful creation, and hence shall be removed
from the restrack first thing during the destruction flow, which keeps
the lifecycle management consistent and predictable.

Fixes: 08f294a1524b ("RDMA/core: Add resource tracking for create and destroy CQs")
Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
Reviewed-by: Michael Guralnik <michaelgur@nvidia.com>
Signed-off-by: Edward Srouji <edwards@nvidia.com>
---
 drivers/infiniband/core/verbs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 8bd39cfcf41bce3a20cfbc41be6f51a1f7f95a8a..bca0e48f6805e87554e77139ce6812d6b7236802 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -2250,11 +2250,15 @@ int ib_destroy_cq_user(struct ib_cq *cq, struct ib_udata *udata)
 	if (atomic_read(&cq->usecnt))
 		return -EBUSY;
 
+	rdma_restrack_begin_del(&cq->res);
+
 	ret = cq->device->ops.destroy_cq(cq, udata);
-	if (ret)
+	if (ret) {
+		rdma_restrack_abort_del(&cq->res);
 		return ret;
+	}
 
-	rdma_restrack_del(&cq->res);
+	rdma_restrack_commit_del(&cq->res);
 	kfree(cq);
 	return ret;
 }

-- 
2.49.0

[PATCH rdma-next 6/6] RDMA/core: Fix potential use after free in ib_destroy_srq_user()

[Edward Srouji]

From: Patrisious Haddad <phaddad@nvidia.com>

When accessing a SRQ via the netlink path the only synchronization
mechanism for the said SRQ is rdma_restrack_get().
Currently, rdma_restrack_del() is invoked at the end of
ib_destroy_srq_user(), which is too late, since by that point
vendor-specific resources associated with the SRQ might already be
freed. This can leave a short window where the SRQ remains accessible
through restrack, leading to a potential use-after-free.

Fix this by moving the rdma_restrack_begin_del() call to the start of
ib_destroy_srq_user(), ensuring that the SRQ is removed from restrack
before its internal resources are released. This guarantees that no new
users hold references to a SRQ that is in the process of destruction.

In addition, this change preserves the intended asymmetric behavior
between create and destroy routines: resources are added to
restrack at the end of successful creation, and hence shall be removed
from the restrack first thing during the destruction flow, which keeps
the lifecycle management consistent and predictable.

Fixes: 48f8a70e899f ("RDMA/restrack: Add support to get resource tracking for SRQ")
Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
Reviewed-by: Michael Guralnik <michaelgur@nvidia.com>
Signed-off-by: Edward Srouji <edwards@nvidia.com>
---
 drivers/infiniband/core/verbs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index bca0e48f6805e87554e77139ce6812d6b7236802..12b79ed046ee81ba2e7b199f39aa40c7bda9d892 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -1139,16 +1139,20 @@ int ib_destroy_srq_user(struct ib_srq *srq, struct ib_udata *udata)
 	if (atomic_read(&srq->usecnt))
 		return -EBUSY;
 
+	rdma_restrack_begin_del(&srq->res);
+
 	ret = srq->device->ops.destroy_srq(srq, udata);
-	if (ret)
+	if (ret) {
+		rdma_restrack_abort_del(&srq->res);
 		return ret;
+	}
 
 	atomic_dec(&srq->pd->usecnt);
 	if (srq->srq_type == IB_SRQT_XRC && srq->ext.xrc.xrcd)
 		atomic_dec(&srq->ext.xrc.xrcd->usecnt);
 	if (ib_srq_has_cq(srq->srq_type))
 		atomic_dec(&srq->ext.cq->usecnt);
-	rdma_restrack_del(&srq->res);
+	rdma_restrack_commit_del(&srq->res);
 	kfree(srq);
 
 	return ret;

-- 
2.49.0

Re: [PATCH rdma-next 0/6] RDMA: Fix restrack UAF in QP/CQ/SRQ destroy

[Jason Gunthorpe]

On Sun, Jun 07, 2026 at 09:18:07PM +0300, Edward Srouji wrote:
> The resource-tracking (restrack) database is the back-end for the netlink
> "rdma resource show" interface which pins objects with
> rdma_restrack_get().
> The QP/CQ/SRQ destroy flows call rdma_restrack_del() at the end of
> ib_destroy_*_user(), after device->ops.destroy_*() had already freed the 
> vendor object. Therefore, a concurrent netlink dump could look the
> object up and touch freed memory, causing a use-after-free via
> ib_query_qp() for instance.
> 
> Fix this by splitting the delete into a begin/commit/abort sequence:
> begin_del() parks the entry as XA_ZERO_ENTRY (so lookups return NULL),
> drops the birth reference and waits for in-flight readers to drain,
> while keeping the index reserved. The destroy paths run begin_del()
> first, then commit_del() on success or abort_del() on error.
> abort_del() re-inserts into the reserved slot, so it needs no allocation
> and cannot fail.
> 
> The first two patches remove DCT and raw RSS QP restrack tracking as
> they have never worked (their ID is unset/reserved at create time).
> 
> Signed-off-by: Edward Srouji <edwards@nvidia.com>
> ---
> Patrisious Haddad (6):
>       RDMA/mlx5: Remove DCT restrack tracking
>       RDMA/mlx5: Remove raw RSS QP restrack tracking
>       RDMA/core: Add rdma_restrack_begin/abort/commit_del() operations
>       RDMA/core: Fix use after free in ib_query_qp()
>       RDMA/core: Fix potential use after free in ib_destroy_cq_user()
>       RDMA/core: Fix potential use after free in ib_destroy_srq_user()

The pre-existing sashiko issues look real too, can you fix them also:

https://sashiko.dev/#/patchset/20260607-restrack-uaf-fix-v1-0-d72e45eb76c2%40nvidia.com

The sashiko notes about XA_ZERO_ENTRY seems to be really obviously
wrong:

void *__xa_cmpxchg(struct xarray *xa, unsigned long index,
			void *old, void *entry, gfp_t gfp)
{
	return xa_zero_to_null(__xa_cmpxchg_raw(xa, index, old, entry, gfp));
}
EXPORT_SYMBOL(__xa_cmpxchg);

This looks legit:

For instance, in drivers/infiniband/core/cq.c:ib_free_cq():
    ret = cq->device->ops.destroy_cq(cq, NULL);
    WARN_ONCE(ret, "Destroy of kernel CQ shouldn't fail");
    rdma_restrack_del(&cq->res);

and so on

Please send a series switching more/all places to commit/abort,
probably there should be very few/no calls to a naked del left.

This doesn't apply on top of the restrack_sync addition, please rebase
it.

You should probably be refactoring rdma_restrack_sync() and using its
parts in this implementation since it does the same things.

I don't think this should NULL the task on abort either, it doesn't
seem necessary.

Jason

Re: [PATCH rdma-next 0/6] RDMA: Fix restrack UAF in QP/CQ/SRQ destroy

[Patrisious Haddad]

On 6/11/2026 10:11 PM, Jason Gunthorpe wrote:
> On Sun, Jun 07, 2026 at 09:18:07PM +0300, Edward Srouji wrote:
>> The resource-tracking (restrack) database is the back-end for the netlink
>> "rdma resource show" interface which pins objects with
>> rdma_restrack_get().
>> The QP/CQ/SRQ destroy flows call rdma_restrack_del() at the end of
>> ib_destroy_*_user(), after device->ops.destroy_*() had already freed the
>> vendor object. Therefore, a concurrent netlink dump could look the
>> object up and touch freed memory, causing a use-after-free via
>> ib_query_qp() for instance.
>>
>> Fix this by splitting the delete into a begin/commit/abort sequence:
>> begin_del() parks the entry as XA_ZERO_ENTRY (so lookups return NULL),
>> drops the birth reference and waits for in-flight readers to drain,
>> while keeping the index reserved. The destroy paths run begin_del()
>> first, then commit_del() on success or abort_del() on error.
>> abort_del() re-inserts into the reserved slot, so it needs no allocation
>> and cannot fail.
>>
>> The first two patches remove DCT and raw RSS QP restrack tracking as
>> they have never worked (their ID is unset/reserved at create time).
>>
>> Signed-off-by: Edward Srouji <edwards@nvidia.com>
>> ---
>> Patrisious Haddad (6):
>>        RDMA/mlx5: Remove DCT restrack tracking
>>        RDMA/mlx5: Remove raw RSS QP restrack tracking
>>        RDMA/core: Add rdma_restrack_begin/abort/commit_del() operations
>>        RDMA/core: Fix use after free in ib_query_qp()
>>        RDMA/core: Fix potential use after free in ib_destroy_cq_user()
>>        RDMA/core: Fix potential use after free in ib_destroy_srq_user()
> The pre-existing sashiko issues look real too, can you fix them also:
Sure but one of them is a false-positive though:
Before destroy_qp() is called, the counter is unconditionally unbound:
         rdma_counter_unbind_qp(qp, qp->port, true);
         ret = qp->device->ops.destroy_qp(qp, udata);
If destroy_qp() fails and we abort destruction here, the kref on the
counter was dropped in rdma_counter_unbind_qp(), but qp->counter is never
set to NULL.

This is actually wrong the qp->counter is actually set to NULL(inside 
the driver though not the core) so subsequent calls will hit the NULL 
check and return safely.
>
> https://sashiko.dev/#/patchset/20260607-restrack-uaf-fix-v1-0-d72e45eb76c2%40nvidia.com
>
> The sashiko notes about XA_ZERO_ENTRY seems to be really obviously
> wrong:
>
> void *__xa_cmpxchg(struct xarray *xa, unsigned long index,
> 			void *old, void *entry, gfp_t gfp)
> {
> 	return xa_zero_to_null(__xa_cmpxchg_raw(xa, index, old, entry, gfp));
> }
> EXPORT_SYMBOL(__xa_cmpxchg);
>
> This looks legit:
>
> For instance, in drivers/infiniband/core/cq.c:ib_free_cq():
>      ret = cq->device->ops.destroy_cq(cq, NULL);
>      WARN_ONCE(ret, "Destroy of kernel CQ shouldn't fail");
>      rdma_restrack_del(&cq->res);
Agreed I seem to have missed this one and the counter_release_one().
>
> and so on
>
> Please send a series switching more/all places to commit/abort,
> probably there should be very few/no calls to a naked del left.

I wonder what about places where switching to commit/abort doesnt fix an 
actual bug.

I had a bigger series that changes all places but decided to send only 
critical ones instead.

For instance "ib_dealloc_pd_user()" I didnt fix since there is no 
rdma_restrack_get() for it currently so it cant hit this issue - should 
I fix that as well ?

and ib_dereg_mr_user() actually calls the delete at start so it doesnt 
have this issue (but it also doesnt readd it to restrack when driver OP 
fails) - but here I think thats by design since the MR would be in weird 
state and we dont want to track it ?
>
> This doesn't apply on top of the restrack_sync addition, please rebase
> it.
>
> You should probably be refactoring rdma_restrack_sync() and using its
> parts in this implementation since it does the same things.
>
> I don't think this should NULL the task on abort either, it doesn't
> seem necessary.
I dont NULL the task on abort(I do NULL it on commit_del() though , or 
were you talking about the restrack_sync() ?
>
> Jason

Re: [PATCH rdma-next 0/6] RDMA: Fix restrack UAF in QP/CQ/SRQ destroy

[Jason Gunthorpe]

On Fri, Jun 12, 2026 at 11:53:23AM +0300, Patrisious Haddad wrote:
> 
> On 6/11/2026 10:11 PM, Jason Gunthorpe wrote:
> > On Sun, Jun 07, 2026 at 09:18:07PM +0300, Edward Srouji wrote:
> > > The resource-tracking (restrack) database is the back-end for the netlink
> > > "rdma resource show" interface which pins objects with
> > > rdma_restrack_get().
> > > The QP/CQ/SRQ destroy flows call rdma_restrack_del() at the end of
> > > ib_destroy_*_user(), after device->ops.destroy_*() had already freed the
> > > vendor object. Therefore, a concurrent netlink dump could look the
> > > object up and touch freed memory, causing a use-after-free via
> > > ib_query_qp() for instance.
> > > 
> > > Fix this by splitting the delete into a begin/commit/abort sequence:
> > > begin_del() parks the entry as XA_ZERO_ENTRY (so lookups return NULL),
> > > drops the birth reference and waits for in-flight readers to drain,
> > > while keeping the index reserved. The destroy paths run begin_del()
> > > first, then commit_del() on success or abort_del() on error.
> > > abort_del() re-inserts into the reserved slot, so it needs no allocation
> > > and cannot fail.
> > > 
> > > The first two patches remove DCT and raw RSS QP restrack tracking as
> > > they have never worked (their ID is unset/reserved at create time).
> > > 
> > > Signed-off-by: Edward Srouji <edwards@nvidia.com>
> > > ---
> > > Patrisious Haddad (6):
> > >        RDMA/mlx5: Remove DCT restrack tracking
> > >        RDMA/mlx5: Remove raw RSS QP restrack tracking
> > >        RDMA/core: Add rdma_restrack_begin/abort/commit_del() operations
> > >        RDMA/core: Fix use after free in ib_query_qp()
> > >        RDMA/core: Fix potential use after free in ib_destroy_cq_user()
> > >        RDMA/core: Fix potential use after free in ib_destroy_srq_user()
> > The pre-existing sashiko issues look real too, can you fix them also:
> Sure but one of them is a false-positive though:
> Before destroy_qp() is called, the counter is unconditionally unbound:
>         rdma_counter_unbind_qp(qp, qp->port, true);
>         ret = qp->device->ops.destroy_qp(qp, udata);
> If destroy_qp() fails and we abort destruction here, the kref on the
> counter was dropped in rdma_counter_unbind_qp(), but qp->counter is never
> set to NULL.
> 
> This is actually wrong the qp->counter is actually set to NULL(inside the
> driver though not the core) so subsequent calls will hit the NULL check and
> return safely.

That doesn't sound very good why is it like that, why is a driver
making any permanent change on destroy failure?

> I wonder what about places where switching to commit/abort doesnt fix an
> actual bug.

I would change them all.

> and ib_dereg_mr_user() actually calls the delete at start so it doesnt have
> this issue (but it also doesnt readd it to restrack when driver OP fails) -
> but here I think thats by design since the MR would be in weird state and we
> dont want to track it ?

That doesn't sound right either.

> > I don't think this should NULL the task on abort either, it doesn't
> > seem necessary.
> I dont NULL the task on abort(I do NULL it on commit_del() though , or were
> you talking about the restrack_sync() ?

Since begin_del calls rdma_restrack_put() which calls through
rdma_restrack_del it NULL's it.

Jason

Re: [PATCH rdma-next 0/6] RDMA: Fix restrack UAF in QP/CQ/SRQ destroy

[Jason Gunthorpe]

On Sun, Jun 07, 2026 at 09:18:07PM +0300, Edward Srouji wrote:
> Patrisious Haddad (6):
>       RDMA/mlx5: Remove DCT restrack tracking
>       RDMA/mlx5: Remove raw RSS QP restrack tracking

Applied these two to for-next

Thanks,
Jason