[PATCH v5 5/5] IB/isert: do not build a new isert_device for a device being removed
From: Yehyeong Lee
Date: Tue Nov 03 2026 - 08:17:02 EST
isert_device_get() looks the device up by node_guid and has no reason to
refuse one. A CONNECT_REQUEST arriving while isert_remove_one() runs
therefore builds a new isert_device on an ib_device the core is about to
free, and nothing waits for its QP, CQ or PD. cma_process_remove() waits
only for id destruction, and the core takes no device reference for them,
so ib_dealloc_device() can run while they are still being released.
Keep the struct as a marker when the last reference of a device being
removed goes away, refuse isert_device_get() for an ib_device that has one,
and clear it from an ib_client add callback. The lookup runs in two
passes, so a marker cannot hide a live device whose ib_device pointer was
reused. The marker uses __GFP_NOFAIL because isert_remove_one() cannot
fail, and preallocating in the add callback does not work:
add_client_context() erases the client data and never calls remove on a
failed add.
A test-only delay before isert_device_get() opens the window. It opened in
one of twelve runs of patches 1 to 4 of this series, and that run built a
second isert_device and took the ten warnings below. It opened in six of
twelve runs with this patch, which built none and took none.
WARNING: drivers/infiniband/core/cq.c:370 at ib_cq_pool_cleanup+0x189/0x250, CPU#1: rdma/201
Modules linked in:
CPU: 1 UID: 0 PID: 201 Comm: rdma Not tainted 7.4.0-rc1-NW4-00006-ga65579195c15 #125 PREEMPT(full)
Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
RIP: 0010:ib_cq_pool_cleanup+0x189/0x250
Code: 85 19 ff ff ff 48 8b 04 24 49 83 c4 10 49 39 c4 0f 85 cd fe ff ff 48 83 c4 18 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 90 <0f> 0b 90 e9 20 ff ff ff 48 89 44 24 08 e8 15 16 0b fe 48 8b 44 24
RSP: 0018:ffff88810b867308 EFLAGS: 00010206
RAX: ffff888109a7ec40 RBX: ffff888109ac5c00 RCX: 0000000000000007
RDX: 000000000000010c RSI: 0000000000000000 RDI: ffff888109ac5c2c
RBP: dffffc0000000000 R08: 0000000000000001 R09: 00000000000000bf
R10: 0000000000000003 R11: 000000000a739171 R12: ffff888104954ad0
R13: dead000000000122 R14: dead000000000100 R15: ffff888109a7ec00
FS: 00007f9437ecb740(0000) GS:ffff88818cade000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f5331739000 CR3: 0000000109a19004 CR4: 0000000000770ef0
PKRU: 55555554
Call Trace:
<TASK>
? remove_client_context+0xb7/0xf0
disable_device+0x13c/0x240
? __pfx_disable_device+0x10/0x10
? __pfx_mutex_lock+0x10/0x10
__ib_unregister_device+0x226/0x3f0
? __pfx_mutex_unlock+0x10/0x10
ib_unregister_device_and_put+0x50/0x70
nldev_dellink+0x28e/0x3d0
? kernel_text_address+0xa7/0x130
? __pfx_nldev_dellink+0x10/0x10
? __pfx_down_read+0x10/0x10
rdma_nl_rcv_msg+0x29c/0x4e0
? __pfx_rdma_nl_rcv_msg+0x10/0x10
rdma_nl_rcv_skb.constprop.0.isra.0+0x21d/0x370
? __pfx_rdma_nl_rcv_skb.constprop.0.isra.0+0x10/0x10
netlink_unicast+0x68f/0x940
? __pfx_selinux_socket_getpeersec_dgram+0x10/0x10
? __pfx_netlink_unicast+0x10/0x10
netlink_sendmsg+0x810/0xf20
? __pfx_netlink_sendmsg+0x10/0x10
? __pfx__raw_spin_lock_irqsave+0x10/0x10
? selinux_socket_sendmsg+0x8c/0x270
? __sys_sendto+0x337/0x390
__sys_sendto+0x337/0x390
? __pfx___sys_sendto+0x10/0x10
? fdget+0x53/0x3b0
__x64_sys_sendto+0xdb/0x1b0
? fpregs_assert_state_consistent+0x56/0xe0
do_syscall_64+0xdd/0x4a0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f9437ffa0d7
Code: c7 c0 ff ff ff ff eb be 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 80 3d 75 ef 0d 00 00 41 89 ca 74 10 b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 69 c3 55 48 89 e5 53 48 83 ec 38 44 89 4d d0
RSP: 002b:00007ffd602e2628 EFLAGS: 00000202 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 0000561d7b001390 RCX: 00007f9437ffa0d7
RDX: 0000000000000018 RSI: 0000561d7b0002a0 RDI: 0000000000000004
RBP: 00007ffd602e2660 R08: 00007f94380f1200 R09: 000000000000000c
R10: 0000000000000000 R11: 0000000000000202 R12: 00007ffd602e2850
R13: 00007ffd602e2634 R14: 00007ffd602e2870 R15: 00007ffd602e2850
</TASK>
Fixes: 63b268d232b8 ("IB/isert: Properly release resources on DEVICE_REMOVAL")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Yehyeong Lee <yhlee@xxxxxxxxxxxxxxxxxx>
---
drivers/infiniband/ulp/isert/ib_isert.c | 73 +++++++++++++++++++++----
1 file changed, 61 insertions(+), 12 deletions(-)
diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 712d6165150e3c..4bcd14f5045ac6 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -260,10 +260,12 @@ isert_device_put(struct isert_device *device)
isert_info("device %p refcount %d\n", device, device->refcount);
if (!device->refcount) {
isert_free_device_ib_res(device);
- list_del(&device->dev_node);
- if (device->removal_comp)
+ if (device->removing) {
complete(device->removal_comp);
- kfree(device);
+ } else {
+ list_del(&device->dev_node);
+ kfree(device);
+ }
}
mutex_unlock(&device_list_mutex);
}
@@ -276,14 +278,21 @@ isert_device_get(struct rdma_cm_id *cma_id)
mutex_lock(&device_list_mutex);
list_for_each_entry(device, &device_list, dev_node) {
- if (device->ib_device->node_guid == cma_id->device->node_guid) {
- device->refcount++;
- isert_info("Found iser device %p refcount %d\n",
- device, device->refcount);
+ if (device->removing && device->ib_device == cma_id->device) {
mutex_unlock(&device_list_mutex);
- return device;
+ return ERR_PTR(-ENODEV);
}
}
+ list_for_each_entry(device, &device_list, dev_node) {
+ if (device->removing ||
+ device->ib_device->node_guid != cma_id->device->node_guid)
+ continue;
+ device->refcount++;
+ isert_info("Found iser device %p refcount %d\n",
+ device, device->refcount);
+ mutex_unlock(&device_list_mutex);
+ return device;
+ }
device = kzalloc_obj(struct isert_device);
if (!device) {
@@ -2792,20 +2801,29 @@ isert_remove_one(struct ib_device *ib_device, void *client_data)
mutex_lock(&device_list_mutex);
list_for_each_entry(device, &device_list, dev_node) {
- if (device->ib_device != ib_device)
+ if (device->ib_device != ib_device || device->removing)
continue;
device->refcount++;
device->removal_comp = &removal_comp;
found = device;
break;
}
- mutex_unlock(&device_list_mutex);
-
- if (!found)
+ if (!found) {
+ device = kzalloc_obj(struct isert_device,
+ GFP_KERNEL | __GFP_NOFAIL);
+ INIT_LIST_HEAD(&device->dev_node);
+ device->ib_device = ib_device;
+ device->removing = true;
+ list_add_tail(&device->dev_node, &device_list);
+ mutex_unlock(&device_list_mutex);
return;
+ }
+ mutex_unlock(&device_list_mutex);
mutex_lock(&isert_conn_mutex);
+ mutex_lock(&device_list_mutex);
found->removing = true;
+ mutex_unlock(&device_list_mutex);
list_for_each_entry(isert_conn, &isert_conn_list, conn_list) {
if (isert_conn->device != found)
continue;
@@ -2828,8 +2846,38 @@ isert_remove_one(struct ib_device *ib_device, void *client_data)
wait_for_completion(&removal_comp);
}
+static int isert_add_one(struct ib_device *ib_device)
+{
+ struct isert_device *device, *tmp;
+
+ mutex_lock(&device_list_mutex);
+ list_for_each_entry_safe(device, tmp, &device_list, dev_node) {
+ if (device->ib_device != ib_device || !device->removing ||
+ device->refcount)
+ continue;
+ list_del(&device->dev_node);
+ kfree(device);
+ }
+ mutex_unlock(&device_list_mutex);
+
+ return 0;
+}
+
+static void isert_free_markers(void)
+{
+ struct isert_device *device, *tmp;
+
+ mutex_lock(&device_list_mutex);
+ list_for_each_entry_safe(device, tmp, &device_list, dev_node) {
+ list_del(&device->dev_node);
+ kfree(device);
+ }
+ mutex_unlock(&device_list_mutex);
+}
+
static struct ib_client isert_ib_client = {
.name = "isert",
+ .add = isert_add_one,
.remove = isert_remove_one,
};
@@ -2881,6 +2929,7 @@ static int __init isert_init(void)
static void __exit isert_exit(void)
{
ib_unregister_client(&isert_ib_client);
+ isert_free_markers();
flush_workqueue(isert_login_wq);
destroy_workqueue(isert_release_wq);
destroy_workqueue(isert_comp_wq);
--
2.43.0