* [PATCH v7 1/6] IB/isert: do not publish connections being released on a second list
2026-11-04 1:22 [PATCH v7 0/6] IB/isert: fix the device removal use-after-free Yehyeong Lee
@ 2026-11-04 1:22 ` Yehyeong Lee
2026-11-04 1:45 ` sashiko-bot
2026-11-04 1:22 ` [PATCH v7 2/6] IB/isert: drain the QP without holding isert_conn->mutex Yehyeong Lee
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Yehyeong Lee @ 2026-11-04 1:22 UTC (permalink / raw)
To: Sagi Grimberg, Max Gurtovoy, Jason Gunthorpe, Leon Romanovsky
Cc: Martin K . Petersen, Mike Christie, linux-rdma, target-devel,
linux-kernel, Yehyeong Lee, stable
isert_connect_error() and isert_disconnected_handler() take a connection
off isert_np->accepted or isert_np->pending and release it, but they do not
check whether the other one got there first, and the connection also stays
reachable from the list that isert_free_np() walks. Two callers can
therefore release the same connection, and isert_free_np() can release one
a CM handler has already freed.
Make the list the single owner token. A caller that empties
isert_conn->node under isert_np->mutex owns the drain and the release; one
that finds it already empty stands back and returns zero, so cma keeps the
id and the owner destroys it. isert_connect_error() drains before it takes
the connection, which is the order mainline already uses, so no drain can
run on a QP whose id another handler is tearing down.
A test-only delay in isert_free_np(), right after it has taken a
connection, opens the window; it does not open on its own in this setup.
With it, two runs of 7.4-rc1 reported 22 use-after-frees each and then an
oops at a NULL instruction pointer in the rmdir task, which was killed.
Two runs with this patch reported neither, and the rmdir returned in seven
seconds.
BUG: KASAN: slab-use-after-free in isert_connect_release+0x5c1/0x620
Read of size 8 at addr ffff888109c5d160 by task rmdir/191
CPU: 1 UID: 0 PID: 191 Comm: rmdir Not tainted 7.4.0-rc1-NSR-00001-gffec6cbc0e7e #129 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
Call Trace:
<TASK>
dump_stack_lvl+0x53/0x70
print_report+0xd0/0x630
? __pfx__raw_spin_lock_irqsave+0x10/0x10
? isert_connect_release
? isert_connect_release+0x5c1/0x620
kasan_report+0xce/0x100
? isert_connect_release+0x5c1/0x620
isert_connect_release+0x5c1/0x620
isert_free_np+0x4cf/0xb20
? __pfx_isert_free_np+0x10/0x10
iscsit_del_np+0x163/0x340
iscsit_tpg_del_network_portal+0x3c3/0x530
? mutex_unlock+0x7b/0xd0
lio_target_call_delnpfromtpg+0x40/0x60
config_item_cleanup+0x115/0x190
configfs_rmdir+0x7c7/0x9f0
? __pfx_configfs_rmdir+0x10/0x10
vfs_rmdir+0x291/0x7b0
? lookup_one_qstr_excl+0x1a/0x1d0
filename_rmdir+0x390/0x560
? __pfx_filename_rmdir+0x10/0x10
? kasan_save_track+0x14/0x30
? __kasan_slab_alloc+0x6e/0x70
? strncpy_from_user+0x2b/0x1c0
__x64_sys_rmdir+0x41/0x60
do_syscall_64+0xdd/0x4a0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x463b6b
Code: 77 05 c3 0f 1f 40 00 48 c7 c2 e0 ff ff ff f7 d8 64 89 02 48 c7 c0 ff ff ff ff c3 0f 1f 40 00 f3 0f 1e fa b8 54 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 05 c3 0f 1f 40 00 48 c7 c2 e0 ff ff ff f7 d8
RSP: 002b:00007ffcc34f26c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000054
RAX: ffffffffffffffda RBX: 000000003b607218 RCX: 0000000000463b6b
RDX: 0000000000000000 RSI: 000000003b5ea9e0 RDI: 000000003b607218
RBP: 00007ffcc34f2700 R08: 0000000000000000 R09: 0000000000000000
R10: 00007ffcc34f2040 R11: 0000000000000246 R12: 000000003b607288
R13: 0000000000000000 R14: 0000000000000000 R15: 00000000005e6245
</TASK>
Allocated by task 72:
kasan_save_stack+0x33/0x60
kasan_save_track+0x14/0x30
__kasan_kmalloc+0x8f/0xa0
__kmalloc_cache_noprof+0x148/0x360
isert_cma_handler+0x1e3/0x28a0
cma_cm_event_handler+0x3e/0x240
cma_ib_req_handler+0x17d9/0x4480
cm_process_work+0x41/0x330
cm_work_handler+0x5727/0xc160
process_one_work+0x697/0x1060
worker_thread+0x45b/0xd10
kthread+0x2c8/0x3b0
ret_from_fork+0x36e/0x5a0
ret_from_fork_asm+0x1a/0x30
Freed by task 10:
kasan_save_stack+0x33/0x60
kasan_save_track+0x14/0x30
kasan_save_free_info+0x3b/0x60
__kasan_slab_free+0x43/0x70
kfree+0x121/0x390
isert_cma_handler+0x1148/0x28a0
cma_cm_event_handler+0x3e/0x240
cma_ib_handler+0x190/0x750
cm_process_work+0x41/0x330
cm_work_handler+0x1de2/0xc160
process_one_work+0x697/0x1060
worker_thread+0x45b/0xd10
kthread+0x2c8/0x3b0
ret_from_fork+0x36e/0x5a0
ret_from_fork_asm+0x1a/0x30
Fixes: 691b0480933f ("IB/isert: Fix dead lock in ib_isert")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
drivers/infiniband/ulp/isert/ib_isert.c | 81 ++++++++++++++++---------
1 file changed, 54 insertions(+), 27 deletions(-)
diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index efa8d7b7844c97..ef2edaa3218fee 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -521,13 +521,18 @@ isert_connected_handler(struct rdma_cm_id *cma_id)
isert_info("conn %p\n", isert_conn);
mutex_lock(&isert_conn->mutex);
+ mutex_lock(&isert_np->mutex);
+ if (list_empty(&isert_conn->node)) {
+ mutex_unlock(&isert_np->mutex);
+ mutex_unlock(&isert_conn->mutex);
+ isert_info("conn %p is being released already\n", isert_conn);
+ return;
+ }
isert_conn->state = ISER_CONN_UP;
kref_get(&isert_conn->kref);
- mutex_unlock(&isert_conn->mutex);
-
- mutex_lock(&isert_np->mutex);
list_move_tail(&isert_conn->node, &isert_np->pending);
mutex_unlock(&isert_np->mutex);
+ mutex_unlock(&isert_conn->mutex);
isert_info("np %p: Allow accept_np to continue\n", isert_np);
up(&isert_np->sem);
@@ -658,12 +663,20 @@ isert_connect_error(struct rdma_cm_id *cma_id)
{
struct isert_conn *isert_conn = cma_id->qp->qp_context;
struct isert_np *isert_np = cma_id->context;
+ bool owned;
ib_drain_qp(isert_conn->qp);
mutex_lock(&isert_np->mutex);
+ owned = !list_empty(&isert_conn->node);
list_del_init(&isert_conn->node);
mutex_unlock(&isert_np->mutex);
+
+ if (!owned) {
+ isert_info("conn %p is being released already\n", isert_conn);
+ return 0;
+ }
+
isert_conn->cm_id = NULL;
isert_put_conn(isert_conn);
@@ -2479,12 +2492,33 @@ isert_accept_np(struct iscsi_np *np, struct iscsit_conn *conn)
return 0;
}
+static struct isert_conn *
+isert_np_take_conn(struct isert_np *isert_np, bool *promoted)
+{
+ struct isert_conn *isert_conn;
+
+ lockdep_assert_held(&isert_np->mutex);
+
+ *promoted = true;
+ isert_conn = list_first_entry_or_null(&isert_np->pending,
+ struct isert_conn, node);
+ if (!isert_conn) {
+ *promoted = false;
+ isert_conn = list_first_entry_or_null(&isert_np->accepted,
+ struct isert_conn, node);
+ }
+ if (isert_conn)
+ list_del_init(&isert_conn->node);
+
+ return isert_conn;
+}
+
static void
isert_free_np(struct iscsi_np *np)
{
struct isert_np *isert_np = np->np_context;
- struct isert_conn *isert_conn, *n;
- LIST_HEAD(drop_conn_list);
+ struct isert_conn *isert_conn;
+ bool promoted;
if (isert_np->cm_id)
rdma_destroy_id(isert_np->cm_id);
@@ -2497,32 +2531,25 @@ isert_free_np(struct iscsi_np *np)
* up in accepted and pending lists.
*/
mutex_lock(&isert_np->mutex);
- if (!list_empty(&isert_np->pending)) {
+ if (!list_empty(&isert_np->pending))
isert_info("Still have isert pending connections\n");
- list_for_each_entry_safe(isert_conn, n,
- &isert_np->pending,
- node) {
- isert_info("cleaning isert_conn %p state (%d)\n",
- isert_conn, isert_conn->state);
- list_move_tail(&isert_conn->node, &drop_conn_list);
- }
- }
-
- if (!list_empty(&isert_np->accepted)) {
+ if (!list_empty(&isert_np->accepted))
isert_info("Still have isert accepted connections\n");
- list_for_each_entry_safe(isert_conn, n,
- &isert_np->accepted,
- node) {
- isert_info("cleaning isert_conn %p state (%d)\n",
- isert_conn, isert_conn->state);
- list_move_tail(&isert_conn->node, &drop_conn_list);
- }
- }
mutex_unlock(&isert_np->mutex);
- list_for_each_entry_safe(isert_conn, n, &drop_conn_list, node) {
- list_del_init(&isert_conn->node);
- isert_connect_release(isert_conn);
+ for (;;) {
+ mutex_lock(&isert_np->mutex);
+ isert_conn = isert_np_take_conn(isert_np, &promoted);
+ mutex_unlock(&isert_np->mutex);
+
+ if (!isert_conn)
+ break;
+
+ isert_info("cleaning isert_conn %p state (%d)\n",
+ isert_conn, isert_conn->state);
+ if (promoted)
+ isert_put_conn(isert_conn);
+ isert_put_conn(isert_conn);
}
np->np_context = NULL;
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 1/6] IB/isert: do not publish connections being released on a second list
2026-11-04 1:22 ` [PATCH v7 1/6] IB/isert: do not publish connections being released on a second list Yehyeong Lee
@ 2026-11-04 1:45 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-11-04 1:45 UTC (permalink / raw)
To: Yehyeong Lee; +Cc: linux-rdma
> isert_connect_error() and isert_disconnected_handler() take a connection
> off isert_np->accepted or isert_np->pending and release it, but they do not
> check whether the other one got there first, and the connection also stays
> reachable from the list that isert_free_np() walks. Two callers can
> therefore release the same connection, and isert_free_np() can release one
> a CM handler has already freed.
>
> Make the list the single owner token. A caller that empties
> isert_conn->node under isert_np->mutex owns the drain and the release; one
> that finds it already empty stands back and returns zero, so cma keeps the
> id and the owner destroys it. isert_connect_error() drains before it takes
> the connection, which is the order mainline already uses, so no drain can
> run on a QP whose id another handler is tearing down.
>
> A test-only delay in isert_free_np(), right after it has taken a
> [ ... ]
> ret_from_fork_asm+0x1a/0x30
>
> Fixes: 691b0480933f ("IB/isert: Fix dead lock in ib_isert")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104012219.1759523-1-yhlee@isslab.korea.ac.kr?part=1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 2/6] IB/isert: drain the QP without holding isert_conn->mutex
2026-11-04 1:22 [PATCH v7 0/6] IB/isert: fix the device removal use-after-free Yehyeong Lee
2026-11-04 1:22 ` [PATCH v7 1/6] IB/isert: do not publish connections being released on a second list Yehyeong Lee
@ 2026-11-04 1:22 ` Yehyeong Lee
2026-11-04 1:47 ` sashiko-bot
2026-11-04 1:22 ` [PATCH v7 3/6] IB/isert: tear connections down from an ib_client on device removal Yehyeong Lee
` (4 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Yehyeong Lee @ 2026-11-04 1:22 UTC (permalink / raw)
To: Sagi Grimberg, Max Gurtovoy, Jason Gunthorpe, Leon Romanovsky
Cc: Martin K . Petersen, Mike Christie, linux-rdma, target-devel,
linux-kernel, Yehyeong Lee, stable
isert_disconnected_handler() drains the QP with isert_conn->mutex held.
The drain waits for the flush completions of the send and receive queues,
and those are reaped by ib_cq_poll_work() on ib-comp-wq, where
isert_login_recv_done() takes isert_conn->mutex before it looks at the
connection state. If a login receive completes while the handler holds
the mutex, the completion handler blocks on the mutex and the drain blocks
on the completion that handler would have reaped.
Return from isert_conn_terminate() whether this caller moved the
connection to ISER_CONN_TERMINATING, and drain after dropping
isert_conn->mutex. Only the owner of isert_conn->node drains, so two
callers cannot drain the same QP; the send and receive queues reserve room
for one drain each, and a second concurrent drain hits the WARN_ONCE in
__ib_drain_sq() and returns without draining.
Two test-only delays open the window, one in isert_login_recv_done() and
one before the drain. Three runs of 7.4-rc1 plus patch 1 of this series
printed ten hung task warnings each, eight, five and five of them waiting
in ib_drain_qp(). Three runs with this patch printed none, and
PROVE_LOCKING reported no splat in two runs a side.
INFO: task kworker/0:1:10 blocked for more than 10 seconds.
Not tainted 7.4.0-rc1-NS1-00002-g7b7ecb49489e #121
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:kworker/0:1 state:D stack:26248 pid:10 tgid:10 ppid:2 task_flags:0x4208060 flags:0x00080000
Workqueue: ib_cm cm_work_handler
Call Trace:
<TASK>
__schedule+0x180f/0x4bb0
? _raw_spin_lock_irq+0x1/0xe0
? __pfx___schedule+0x10/0x10
schedule+0x70/0x1f0
schedule_timeout+0x1cd/0x200
? __pfx_schedule_timeout+0x10/0x10
? rxe_qp_from_attr+0x1497/0x18c0
? __pfx__raw_spin_lock_irq+0x10/0x10
? __pfx__raw_spin_lock_irqsave+0x10/0x10
? _raw_spin_lock_irqsave+0x86/0xe0
__wait_for_common+0x29e/0x420
? __pfx_schedule_timeout+0x10/0x10
? __pfx___wait_for_common+0x10/0x10
__ib_drain_sq+0x251/0x340
? __pfx___ib_drain_sq+0x10/0x10
? __pfx_ib_drain_qp_done+0x10/0x10
? irq_work_queue+0x1c/0xb0
? __pfx__printk+0x10/0x10
? __pfx_process_timeout+0x10/0x10
ib_drain_sq+0x146/0x1a0
ib_drain_qp+0xd/0x50
isert_disconnected_handler.isra.0+0xc4/0x3f0
isert_cma_handler+0xd74/0x27d0
? kasan_save_stack+0x42/0x60
? kasan_save_stack+0x33/0x60
? kasan_save_track+0x14/0x30
? kasan_save_free_info+0x3b/0x60
? __kasan_slab_free+0x43/0x70
? kfree+0x121/0x390
? cm_process_work+0x8b/0x330
? __pfx_isert_cma_handler+0x10/0x10
? kthread+0x2c8/0x3b0
? ret_from_fork+0x36e/0x5a0
? ret_from_fork_asm+0x1a/0x30
? _raw_spin_lock_irqsave+0x86/0xe0
cma_cm_event_handler+0x3e/0x240
cma_ib_handler+0x190/0x750
? __pfx_cma_ib_handler+0x10/0x10
cm_process_work+0x41/0x330
cm_work_handler+0x582b/0xc160
? _raw_spin_lock_irqsave+0x86/0xe0
? __pfx__raw_spin_lock_irqsave+0x10/0x10
? hrtimer_start_range_ns+0xa6/0x2c0
? __pfx_cm_work_handler+0x10/0x10
? finish_task_switch.isra.0+0x1d9/0xa70
process_one_work+0x697/0x1060
? __pfx___schedule+0x10/0x10
? __pfx_process_one_work+0x10/0x10
? _raw_spin_lock_irq+0x81/0xe0
? __pfx_cm_work_handler+0x10/0x10
? assign_work+0x11d/0x370
worker_thread+0x45b/0xd10
? __pfx_worker_thread+0x10/0x10
? __pfx_worker_thread+0x10/0x10
kthread+0x2c8/0x3b0
? recalc_sigpending+0x15c/0x1e0
? __pfx_kthread+0x10/0x10
ret_from_fork+0x36e/0x5a0
? __pfx_ret_from_fork+0x10/0x10
? __switch_to+0x572/0xdc0
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
</TASK>
Fixes: 6d1fba0c2cc7 ("iser-target: Rework connection termination")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
drivers/infiniband/ulp/isert/ib_isert.c | 75 ++++++++++++-------------
1 file changed, 36 insertions(+), 39 deletions(-)
diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index ef2edaa3218fee..61b214012a70b0 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -556,43 +556,21 @@ isert_put_conn(struct isert_conn *isert_conn)
kref_put(&isert_conn->kref, isert_release_kref);
}
-static void
-isert_handle_unbound_conn(struct isert_conn *isert_conn)
-{
- struct isert_np *isert_np = isert_conn->cm_id->context;
-
- mutex_lock(&isert_np->mutex);
- if (!list_empty(&isert_conn->node)) {
- /*
- * This means iscsi doesn't know this connection
- * so schedule a cleanup ourselves
- */
- list_del_init(&isert_conn->node);
- isert_put_conn(isert_conn);
- queue_work(isert_release_wq, &isert_conn->release_work);
- }
- mutex_unlock(&isert_np->mutex);
-}
-
/**
* isert_conn_terminate() - Initiate connection termination
* @isert_conn: isert connection struct
*
- * Notes:
- * In case the connection state is BOUND, move state
- * to TEMINATING and start teardown sequence (rdma_disconnect).
- * In case the connection state is UP, complete flush as well.
- *
- * This routine must be called with mutex held. Thus it is
- * safe to call multiple times.
+ * Return: true if this caller moved the connection to ISER_CONN_TERMINATING.
*/
-static void
+static bool
isert_conn_terminate(struct isert_conn *isert_conn)
{
int err;
+ lockdep_assert_held(&isert_conn->mutex);
+
if (isert_conn->state >= ISER_CONN_TERMINATING)
- return;
+ return false;
isert_info("Terminating conn %p state %d\n",
isert_conn, isert_conn->state);
@@ -601,6 +579,8 @@ isert_conn_terminate(struct isert_conn *isert_conn)
if (err)
isert_warn("Failed rdma_disconnect isert_conn %p\n",
isert_conn);
+
+ return true;
}
static int
@@ -635,26 +615,44 @@ isert_disconnected_handler(struct rdma_cm_id *cma_id,
enum rdma_cm_event_type event)
{
struct isert_conn *isert_conn = cma_id->qp->qp_context;
+ struct isert_np *isert_np = cma_id->context;
+ bool owned = false, bound = false, terminated = false;
mutex_lock(&isert_conn->mutex);
switch (isert_conn->state) {
- case ISER_CONN_TERMINATING:
- break;
case ISER_CONN_UP:
- isert_conn_terminate(isert_conn);
- ib_drain_qp(isert_conn->qp);
- isert_handle_unbound_conn(isert_conn);
+ mutex_lock(&isert_np->mutex);
+ if (list_empty(&isert_conn->node)) {
+ bound = isert_conn->state == ISER_CONN_BOUND ||
+ isert_conn->state == ISER_CONN_FULL_FEATURE;
+ } else {
+ owned = true;
+ list_del_init(&isert_conn->node);
+ }
+ mutex_unlock(&isert_np->mutex);
break;
case ISER_CONN_BOUND:
case ISER_CONN_FULL_FEATURE: /* FALLTHRU */
- iscsit_cause_connection_reinstatement(isert_conn->conn, 0);
+ bound = true;
break;
default:
- isert_warn("conn %p terminating in state %d\n",
- isert_conn, isert_conn->state);
+ break;
}
+
+ if (owned)
+ terminated = isert_conn_terminate(isert_conn);
+ else if (bound)
+ iscsit_cause_connection_reinstatement(isert_conn->conn, 0);
mutex_unlock(&isert_conn->mutex);
+ if (!owned)
+ return 0;
+
+ if (terminated)
+ ib_drain_qp(isert_conn->qp);
+ isert_put_conn(isert_conn);
+ queue_work(isert_release_wq, &isert_conn->release_work);
+
return 0;
}
@@ -665,8 +663,6 @@ isert_connect_error(struct rdma_cm_id *cma_id)
struct isert_np *isert_np = cma_id->context;
bool owned;
- ib_drain_qp(isert_conn->qp);
-
mutex_lock(&isert_np->mutex);
owned = !list_empty(&isert_conn->node);
list_del_init(&isert_conn->node);
@@ -677,6 +673,7 @@ isert_connect_error(struct rdma_cm_id *cma_id)
return 0;
}
+ ib_drain_qp(isert_conn->qp);
isert_conn->cm_id = NULL;
isert_put_conn(isert_conn);
@@ -2479,11 +2476,10 @@ isert_accept_np(struct iscsi_np *np, struct iscsit_conn *conn)
isert_conn = list_first_entry(&isert_np->pending,
struct isert_conn, node);
list_del_init(&isert_conn->node);
- mutex_unlock(&isert_np->mutex);
-
conn->context = isert_conn;
isert_conn->conn = conn;
isert_conn->state = ISER_CONN_BOUND;
+ mutex_unlock(&isert_np->mutex);
isert_set_conn_info(np, conn, isert_conn);
@@ -2547,6 +2543,7 @@ isert_free_np(struct iscsi_np *np)
isert_info("cleaning isert_conn %p state (%d)\n",
isert_conn, isert_conn->state);
+ ib_drain_qp(isert_conn->qp);
if (promoted)
isert_put_conn(isert_conn);
isert_put_conn(isert_conn);
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 2/6] IB/isert: drain the QP without holding isert_conn->mutex
2026-11-04 1:22 ` [PATCH v7 2/6] IB/isert: drain the QP without holding isert_conn->mutex Yehyeong Lee
@ 2026-11-04 1:47 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-11-04 1:47 UTC (permalink / raw)
To: Yehyeong Lee; +Cc: linux-rdma
> isert_disconnected_handler() drains the QP with isert_conn->mutex held.
> The drain waits for the flush completions of the send and receive queues,
> and those are reaped by ib_cq_poll_work() on ib-comp-wq, where
> isert_login_recv_done() takes isert_conn->mutex before it looks at the
> connection state. If a login receive completes while the handler holds
> the mutex, the completion handler blocks on the mutex and the drain blocks
> on the completion that handler would have reaped.
>
> Return from isert_conn_terminate() whether this caller moved the
> connection to ISER_CONN_TERMINATING, and drain after dropping
> isert_conn->mutex. Only the owner of isert_conn->node drains, so two
> callers cannot drain the same QP; the send and receive queues reserve room
> for one drain each, and a second concurrent drain hits the WARN_ONCE in
> __ib_drain_sq() and returns without draining.
>
> [ ... ]
> </TASK>
>
> Fixes: 6d1fba0c2cc7 ("iser-target: Rework connection termination")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104012219.1759523-1-yhlee@isslab.korea.ac.kr?part=2
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 3/6] IB/isert: tear connections down from an ib_client on device removal
2026-11-04 1:22 [PATCH v7 0/6] IB/isert: fix the device removal use-after-free Yehyeong Lee
2026-11-04 1:22 ` [PATCH v7 1/6] IB/isert: do not publish connections being released on a second list Yehyeong Lee
2026-11-04 1:22 ` [PATCH v7 2/6] IB/isert: drain the QP without holding isert_conn->mutex Yehyeong Lee
@ 2026-11-04 1:22 ` Yehyeong Lee
2026-11-04 1:58 ` sashiko-bot
2026-11-04 1:22 ` [PATCH v7 4/6] IB/isert: drop the promotion reference when a connection errors out Yehyeong Lee
` (3 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Yehyeong Lee @ 2026-11-04 1:22 UTC (permalink / raw)
To: Sagi Grimberg, Max Gurtovoy, Jason Gunthorpe, Leon Romanovsky
Cc: Martin K . Petersen, Mike Christie, linux-rdma, target-devel,
linux-kernel, Yehyeong Lee, stable
The RDMA_CM_EVENT_DEVICE_REMOVAL callback waits for the connection it was
handed to reach ISER_CONN_DOWN and then frees it. isert_release_work()
publishes that state before it drops its reference, so the handler can free
a connection that isert_connect_release() is still running on. The wait
itself deadlocks against the handler_mutex rdma_destroy_id() needs.
Register an ib_client instead, as nvmet-rdma did in commit f1d4ef7d8883
("nvmet-rdma: register ib_client to not deadlock in device removal"), and
tear the device's connections down from its remove callback. It walks
isert_conn_list under isert_conn_mutex, takes a reference with
kref_get_unless_zero(), tears each one down after dropping the lock, and
asks iscsi to reinstate the session. isert_device_put() completes
removal_comp once the CQs and the PD are gone. Publishing now happens in
the same isert_conn_mutex section that tests isert_device->removing, and
isert_np is reference counted because a connection can outlive its portal.
This cell drives 32 logins and then deletes the device, with no delay knob.
One of twelve runs of 7.4-rc1 finished the removal, and two reported the
use-after-free below, 17 and 11 times; all twelve runs of this series
finished and none reported it. The sides ran in separate rounds.
BUG: KASAN: slab-use-after-free in isert_connect_release+0x5cb/0x620
Read of size 4 at addr ffff88810af75158 by task kworker/u16:6/76
CPU: 1 UID: 0 PID: 76 Comm: kworker/u16:6 Not tainted 7.4.0-rc1-CTL #19 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
Workqueue: isert_release_wq isert_release_work
Call Trace:
<TASK>
dump_stack_lvl+0x53/0x70
print_report+0xd0/0x630
? __pfx__raw_spin_lock_irqsave+0x10/0x10
? kfree+0x121/0x390
? isert_connect_release+0x5cb/0x620
kasan_report+0xce/0x100
? isert_connect_release+0x5cb/0x620
isert_connect_release+0x5cb/0x620
process_one_work+0x697/0x1060
? __pfx___schedule+0x10/0x10
? __pfx_process_one_work+0x10/0x10
? _raw_spin_lock_irq+0x81/0xe0
? __pfx_isert_release_work+0x10/0x10
? assign_work+0x11d/0x370
worker_thread+0x45b/0xd10
? __pfx_worker_thread+0x10/0x10
kthread+0x2c8/0x3b0
? recalc_sigpending+0x15c/0x1e0
? __pfx_kthread+0x10/0x10
ret_from_fork+0x36e/0x5a0
? __pfx_ret_from_fork+0x10/0x10
? __switch_to+0x572/0xdc0
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
</TASK>
Allocated by task 51:
kasan_save_stack+0x33/0x60
kasan_save_track+0x14/0x30
__kasan_kmalloc+0x8f/0xa0
__kmalloc_cache_noprof+0x148/0x360
isert_cma_handler+0x1e3/0x2840
cma_cm_event_handler+0x3e/0x240
cma_ib_req_handler+0x17d9/0x4480
cm_process_work+0x41/0x330
cm_work_handler+0x5727/0xc160
process_one_work+0x697/0x1060
worker_thread+0x45b/0xd10
kthread+0x2c8/0x3b0
ret_from_fork+0x36e/0x5a0
ret_from_fork_asm+0x1a/0x30
Freed by task 217:
kasan_save_stack+0x33/0x60
kasan_save_track+0x14/0x30
kasan_save_free_info+0x3b/0x60
__kasan_slab_free+0x43/0x70
kfree+0x121/0x390
isert_cma_handler+0x10ff/0x2840
cma_cm_event_handler+0x3e/0x240
cma_process_remove+0xff/0x5a0
cma_remove_one+0xcc/0x250
remove_client_context+0xa4/0xf0
disable_device+0x128/0x240
__ib_unregister_device+0x226/0x3f0
ib_unregister_device_and_put+0x50/0x70
nldev_dellink+0x28e/0x3d0
rdma_nl_rcv_msg+0x29c/0x4e0
rdma_nl_rcv_skb.constprop.0.isra.0+0x21d/0x370
netlink_unicast+0x68f/0x940
netlink_sendmsg+0x810/0xf20
__sys_sendto+0x337/0x390
__x64_sys_sendto+0xdb/0x1b0
do_syscall_64+0xdd/0x4a0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Fixes: 63b268d232b8 ("IB/isert: Properly release resources on DEVICE_REMOVAL")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
drivers/infiniband/ulp/isert/ib_isert.c | 274 +++++++++++++++++-------
drivers/infiniband/ulp/isert/ib_isert.h | 9 +-
2 files changed, 208 insertions(+), 75 deletions(-)
diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 61b214012a70b0..4812dabd939453 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -43,6 +43,8 @@ MODULE_PARM_DESC(sg_tablesize,
static DEFINE_MUTEX(device_list_mutex);
static LIST_HEAD(device_list);
+static DEFINE_MUTEX(isert_conn_mutex);
+static LIST_HEAD(isert_conn_list);
static struct workqueue_struct *isert_login_wq;
static struct workqueue_struct *isert_comp_wq;
static struct workqueue_struct *isert_release_wq;
@@ -56,6 +58,10 @@ isert_rdma_accept(struct isert_conn *isert_conn);
struct rdma_cm_id *isert_setup_id(struct isert_np *isert_np);
static void isert_release_work(struct work_struct *work);
+static void
+isert_put_np(struct isert_np *isert_np);
+static void
+isert_teardown_conn(struct isert_conn *isert_conn);
static void isert_recv_done(struct ib_cq *cq, struct ib_wc *wc);
static void isert_send_done(struct ib_cq *cq, struct ib_wc *wc);
static void isert_login_recv_done(struct ib_cq *cq, struct ib_wc *wc);
@@ -255,6 +261,8 @@ isert_device_put(struct isert_device *device)
if (!device->refcount) {
isert_free_device_ib_res(device);
list_del(&device->dev_node);
+ if (device->removal_comp)
+ complete(device->removal_comp);
kfree(device);
}
mutex_unlock(&device_list_mutex);
@@ -309,7 +317,8 @@ isert_init_conn(struct isert_conn *isert_conn)
INIT_LIST_HEAD(&isert_conn->node);
init_completion(&isert_conn->login_comp);
init_completion(&isert_conn->login_req_comp);
- init_waitqueue_head(&isert_conn->rem_wait);
+ INIT_LIST_HEAD(&isert_conn->conn_list);
+ INIT_LIST_HEAD(&isert_conn->removal_node);
kref_init(&isert_conn->kref);
atomic_set(&isert_conn->ctrl_comp_cnt, 0);
mutex_init(&isert_conn->mutex);
@@ -456,17 +465,34 @@ isert_connect_request(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
goto out_rsp_dma_map;
}
+ mutex_lock(&isert_conn_mutex);
+ if (device->removing) {
+ mutex_unlock(&isert_conn_mutex);
+ isert_info("device %p is being removed, rejecting conn %p\n",
+ device, isert_conn);
+ ret = -ENODEV;
+ goto out_destroy_qp;
+ }
+
ret = isert_login_post_recv(isert_conn);
- if (ret)
+ if (ret) {
+ mutex_unlock(&isert_conn_mutex);
goto out_destroy_qp;
+ }
ret = isert_rdma_accept(isert_conn);
- if (ret)
+ if (ret) {
+ mutex_unlock(&isert_conn_mutex);
goto out_destroy_qp;
+ }
+ isert_conn->np = isert_np;
+ kref_get(&isert_np->kref);
+ list_add_tail(&isert_conn->conn_list, &isert_conn_list);
mutex_lock(&isert_np->mutex);
list_add_tail(&isert_conn->node, &isert_np->accepted);
mutex_unlock(&isert_np->mutex);
+ mutex_unlock(&isert_conn_mutex);
return 0;
@@ -486,14 +512,18 @@ static void
isert_connect_release(struct isert_conn *isert_conn)
{
struct isert_device *device = isert_conn->device;
+ struct isert_np *isert_np = isert_conn->np;
isert_dbg("conn %p\n", isert_conn);
BUG_ON(!device);
+ mutex_lock(&isert_conn_mutex);
+ list_del_init(&isert_conn->conn_list);
+ mutex_unlock(&isert_conn_mutex);
+
isert_free_rx_descriptors(isert_conn);
- if (isert_conn->cm_id &&
- !isert_conn->dev_removed)
+ if (isert_conn->cm_id)
rdma_destroy_id(isert_conn->cm_id);
if (isert_conn->qp)
@@ -504,12 +534,11 @@ isert_connect_release(struct isert_conn *isert_conn)
if (isert_conn->login_desc)
isert_free_login_buf(isert_conn);
- isert_device_put(device);
+ kfree(isert_conn);
- if (isert_conn->dev_removed)
- wake_up_interruptible(&isert_conn->rem_wait);
- else
- kfree(isert_conn);
+ if (isert_np)
+ isert_put_np(isert_np);
+ isert_device_put(device);
}
static void
@@ -517,22 +546,37 @@ isert_connected_handler(struct rdma_cm_id *cma_id)
{
struct isert_conn *isert_conn = cma_id->qp->qp_context;
struct isert_np *isert_np = cma_id->context;
+ bool owned, promote;
isert_info("conn %p\n", isert_conn);
+ mutex_lock(&isert_conn_mutex);
mutex_lock(&isert_conn->mutex);
mutex_lock(&isert_np->mutex);
- if (list_empty(&isert_conn->node)) {
- mutex_unlock(&isert_np->mutex);
- mutex_unlock(&isert_conn->mutex);
- isert_info("conn %p is being released already\n", isert_conn);
- return;
+ owned = !list_empty(&isert_conn->node);
+ promote = owned && isert_conn->state < ISER_CONN_TERMINATING &&
+ !isert_conn->device->removing;
+ if (promote) {
+ isert_conn->state = ISER_CONN_UP;
+ kref_get(&isert_conn->kref);
+ isert_conn->promoted = true;
+ list_move_tail(&isert_conn->node, &isert_np->pending);
}
- isert_conn->state = ISER_CONN_UP;
- kref_get(&isert_conn->kref);
- list_move_tail(&isert_conn->node, &isert_np->pending);
mutex_unlock(&isert_np->mutex);
mutex_unlock(&isert_conn->mutex);
+ mutex_unlock(&isert_conn_mutex);
+
+ if (!owned) {
+ isert_info("conn %p is being released already\n", isert_conn);
+ return;
+ }
+
+ if (!promote) {
+ isert_info("conn %p is not promoted, state %d\n",
+ isert_conn, isert_conn->state);
+ isert_teardown_conn(isert_conn);
+ return;
+ }
isert_info("np %p: Allow accept_np to continue\n", isert_np);
up(&isert_np->sem);
@@ -556,6 +600,20 @@ isert_put_conn(struct isert_conn *isert_conn)
kref_put(&isert_conn->kref, isert_release_kref);
}
+static void
+isert_release_np_kref(struct kref *kref)
+{
+ struct isert_np *isert_np = container_of(kref, struct isert_np, kref);
+
+ kfree(isert_np);
+}
+
+static void
+isert_put_np(struct isert_np *isert_np)
+{
+ kref_put(&isert_np->kref, isert_release_np_kref);
+}
+
/**
* isert_conn_terminate() - Initiate connection termination
* @isert_conn: isert connection struct
@@ -610,48 +668,50 @@ isert_np_cma_handler(struct isert_np *isert_np,
return -1;
}
-static int
-isert_disconnected_handler(struct rdma_cm_id *cma_id,
- enum rdma_cm_event_type event)
+static void
+isert_teardown_conn(struct isert_conn *isert_conn)
{
- struct isert_conn *isert_conn = cma_id->qp->qp_context;
- struct isert_np *isert_np = cma_id->context;
- bool owned = false, bound = false, terminated = false;
+ struct isert_np *isert_np = isert_conn->np;
+ bool owned = false, bound = false;
+ bool promoted = false, terminated = false;
mutex_lock(&isert_conn->mutex);
- switch (isert_conn->state) {
- case ISER_CONN_UP:
- mutex_lock(&isert_np->mutex);
- if (list_empty(&isert_conn->node)) {
- bound = isert_conn->state == ISER_CONN_BOUND ||
- isert_conn->state == ISER_CONN_FULL_FEATURE;
- } else {
- owned = true;
- list_del_init(&isert_conn->node);
- }
- mutex_unlock(&isert_np->mutex);
- break;
- case ISER_CONN_BOUND:
- case ISER_CONN_FULL_FEATURE: /* FALLTHRU */
- bound = true;
- break;
- default:
- break;
+ mutex_lock(&isert_np->mutex);
+ if (list_empty(&isert_conn->node)) {
+ bound = isert_conn->state == ISER_CONN_BOUND ||
+ isert_conn->state == ISER_CONN_FULL_FEATURE;
+ } else {
+ owned = true;
+ promoted = isert_conn->promoted;
+ list_del_init(&isert_conn->node);
}
+ mutex_unlock(&isert_np->mutex);
- if (owned)
+ if (owned) {
terminated = isert_conn_terminate(isert_conn);
- else if (bound)
+ } else if (bound) {
+ isert_conn_terminate(isert_conn);
iscsit_cause_connection_reinstatement(isert_conn->conn, 0);
+ }
mutex_unlock(&isert_conn->mutex);
if (!owned)
- return 0;
+ return;
if (terminated)
ib_drain_qp(isert_conn->qp);
- isert_put_conn(isert_conn);
+ if (promoted)
+ isert_put_conn(isert_conn);
queue_work(isert_release_wq, &isert_conn->release_work);
+}
+
+static int
+isert_disconnected_handler(struct rdma_cm_id *cma_id,
+ enum rdma_cm_event_type event)
+{
+ struct isert_conn *isert_conn = cma_id->qp->qp_context;
+
+ isert_teardown_conn(isert_conn);
return 0;
}
@@ -661,20 +721,33 @@ isert_connect_error(struct rdma_cm_id *cma_id)
{
struct isert_conn *isert_conn = cma_id->qp->qp_context;
struct isert_np *isert_np = cma_id->context;
- bool owned;
+ bool owned = false, bound = false;
+ mutex_lock(&isert_conn->mutex);
mutex_lock(&isert_np->mutex);
- owned = !list_empty(&isert_conn->node);
- list_del_init(&isert_conn->node);
+ if (list_empty(&isert_conn->node)) {
+ bound = isert_conn->state == ISER_CONN_BOUND ||
+ isert_conn->state == ISER_CONN_FULL_FEATURE;
+ } else {
+ owned = true;
+ list_del_init(&isert_conn->node);
+ }
mutex_unlock(&isert_np->mutex);
+ if (owned) {
+ isert_conn->cm_id = NULL;
+ } else if (bound) {
+ isert_conn_terminate(isert_conn);
+ iscsit_cause_connection_reinstatement(isert_conn->conn, 0);
+ }
+ mutex_unlock(&isert_conn->mutex);
+
if (!owned) {
isert_info("conn %p is being released already\n", isert_conn);
return 0;
}
ib_drain_qp(isert_conn->qp);
- isert_conn->cm_id = NULL;
isert_put_conn(isert_conn);
return -1;
@@ -684,7 +757,6 @@ static int
isert_cma_handler(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
{
struct isert_np *isert_np = cma_id->context;
- struct isert_conn *isert_conn;
int ret = 0;
isert_info("%s (%d): status %d id %p np %p\n",
@@ -705,21 +777,10 @@ isert_cma_handler(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
break;
case RDMA_CM_EVENT_ADDR_CHANGE:
case RDMA_CM_EVENT_DISCONNECTED:
- case RDMA_CM_EVENT_TIMEWAIT_EXIT: /* FALLTHRU */
+ case RDMA_CM_EVENT_TIMEWAIT_EXIT:
+ case RDMA_CM_EVENT_DEVICE_REMOVAL:
ret = isert_disconnected_handler(cma_id, event->event);
break;
- case RDMA_CM_EVENT_DEVICE_REMOVAL:
- isert_conn = cma_id->qp->qp_context;
- isert_conn->dev_removed = true;
- isert_disconnected_handler(cma_id, event->event);
- wait_event_interruptible(isert_conn->rem_wait,
- isert_conn->state == ISER_CONN_DOWN);
- kfree(isert_conn);
- /*
- * return non-zero from the callback to destroy
- * the rdma cm id
- */
- return 1;
case RDMA_CM_EVENT_REJECTED:
isert_info("Connection rejected: %s\n",
rdma_reject_msg(cma_id, event->status));
@@ -2331,6 +2392,7 @@ isert_setup_np(struct iscsi_np *np,
if (!isert_np)
return -ENOMEM;
+ kref_init(&isert_np->kref);
sema_init(&isert_np->sem, 0);
mutex_init(&isert_np->mutex);
INIT_LIST_HEAD(&isert_np->accepted);
@@ -2356,7 +2418,7 @@ isert_setup_np(struct iscsi_np *np,
return 0;
out:
- kfree(isert_np);
+ isert_put_np(isert_np);
return ret;
}
@@ -2476,6 +2538,7 @@ isert_accept_np(struct iscsi_np *np, struct iscsit_conn *conn)
isert_conn = list_first_entry(&isert_np->pending,
struct isert_conn, node);
list_del_init(&isert_conn->node);
+ isert_conn->promoted = false;
conn->context = isert_conn;
isert_conn->conn = conn;
isert_conn->state = ISER_CONN_BOUND;
@@ -2489,20 +2552,17 @@ isert_accept_np(struct iscsi_np *np, struct iscsit_conn *conn)
}
static struct isert_conn *
-isert_np_take_conn(struct isert_np *isert_np, bool *promoted)
+isert_np_take_conn(struct isert_np *isert_np)
{
struct isert_conn *isert_conn;
lockdep_assert_held(&isert_np->mutex);
- *promoted = true;
isert_conn = list_first_entry_or_null(&isert_np->pending,
struct isert_conn, node);
- if (!isert_conn) {
- *promoted = false;
+ if (!isert_conn)
isert_conn = list_first_entry_or_null(&isert_np->accepted,
struct isert_conn, node);
- }
if (isert_conn)
list_del_init(&isert_conn->node);
@@ -2535,7 +2595,8 @@ isert_free_np(struct iscsi_np *np)
for (;;) {
mutex_lock(&isert_np->mutex);
- isert_conn = isert_np_take_conn(isert_np, &promoted);
+ isert_conn = isert_np_take_conn(isert_np);
+ promoted = isert_conn && isert_conn->promoted;
mutex_unlock(&isert_np->mutex);
if (!isert_conn)
@@ -2550,7 +2611,8 @@ isert_free_np(struct iscsi_np *np)
}
np->np_context = NULL;
- kfree(isert_np);
+ isert_np->np = NULL;
+ isert_put_np(isert_np);
}
static void isert_release_work(struct work_struct *work)
@@ -2679,6 +2741,10 @@ static void isert_get_rx_pdu(struct iscsit_conn *conn)
/* Now we are in FULL_FEATURE phase */
mutex_lock(&isert_conn->mutex);
+ if (isert_conn->state >= ISER_CONN_TERMINATING) {
+ mutex_unlock(&isert_conn->mutex);
+ return;
+ }
isert_conn->state = ISER_CONN_FULL_FEATURE;
mutex_unlock(&isert_conn->mutex);
@@ -2713,8 +2779,61 @@ static struct iscsit_transport iser_target_transport = {
.iscsit_get_sup_prot_ops = isert_get_sup_prot_ops,
};
+static void
+isert_remove_one(struct ib_device *ib_device, void *client_data)
+{
+ DECLARE_COMPLETION_ONSTACK(removal_comp);
+ struct isert_device *device, *found = NULL;
+ struct isert_conn *isert_conn, *tmp;
+ LIST_HEAD(removal_list);
+
+ mutex_lock(&device_list_mutex);
+ list_for_each_entry(device, &device_list, dev_node) {
+ if (device->ib_device != ib_device)
+ continue;
+ device->refcount++;
+ device->removal_comp = &removal_comp;
+ found = device;
+ break;
+ }
+ mutex_unlock(&device_list_mutex);
+
+ if (!found)
+ return;
+
+ mutex_lock(&isert_conn_mutex);
+ found->removing = true;
+ list_for_each_entry(isert_conn, &isert_conn_list, conn_list) {
+ if (isert_conn->device != found)
+ continue;
+ /* A connection with no reference left is already releasing. */
+ if (!kref_get_unless_zero(&isert_conn->kref))
+ continue;
+ list_add_tail(&isert_conn->removal_node, &removal_list);
+ }
+ mutex_unlock(&isert_conn_mutex);
+
+ list_for_each_entry_safe(isert_conn, tmp, &removal_list, removal_node) {
+ list_del_init(&isert_conn->removal_node);
+ isert_info("removing conn %p state %d\n",
+ isert_conn, isert_conn->state);
+ isert_teardown_conn(isert_conn);
+ isert_put_conn(isert_conn);
+ }
+
+ isert_device_put(found);
+ wait_for_completion(&removal_comp);
+}
+
+static struct ib_client isert_ib_client = {
+ .name = "isert",
+ .remove = isert_remove_one,
+};
+
static int __init isert_init(void)
{
+ int ret = -ENOMEM;
+
isert_login_wq = alloc_workqueue("isert_login_wq", WQ_PERCPU, 0);
if (!isert_login_wq) {
isert_err("Unable to allocate isert_login_wq\n");
@@ -2735,21 +2854,30 @@ static int __init isert_init(void)
goto destroy_comp_wq;
}
+ ret = ib_register_client(&isert_ib_client);
+ if (ret) {
+ isert_err("Unable to register isert ib client\n");
+ goto destroy_release_wq;
+ }
+
iscsit_register_transport(&iser_target_transport);
isert_info("iSER_TARGET[0] - Loaded iser_target_transport\n");
return 0;
+destroy_release_wq:
+ destroy_workqueue(isert_release_wq);
destroy_comp_wq:
destroy_workqueue(isert_comp_wq);
destroy_login_wq:
destroy_workqueue(isert_login_wq);
- return -ENOMEM;
+ return ret;
}
static void __exit isert_exit(void)
{
+ ib_unregister_client(&isert_ib_client);
flush_workqueue(isert_login_wq);
destroy_workqueue(isert_release_wq);
destroy_workqueue(isert_comp_wq);
diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h
index 519b17e54bd342..fbb58f3865f215 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.h
+++ b/drivers/infiniband/ulp/isert/ib_isert.h
@@ -185,14 +185,16 @@ struct isert_conn {
struct ib_cq *cq;
u32 cq_size;
struct isert_device *device;
+ struct isert_np *np;
+ bool promoted;
struct mutex mutex;
struct kref kref;
struct work_struct release_work;
+ struct list_head removal_node;
atomic_t ctrl_comp_cnt;
bool logout_posted;
bool snd_w_inv;
- wait_queue_head_t rem_wait;
- bool dev_removed;
+ struct list_head conn_list;
};
struct isert_device {
@@ -203,6 +205,8 @@ struct isert_device {
struct isert_comp *comps;
int comps_used;
struct list_head dev_node;
+ struct completion *removal_comp;
+ bool removing;
};
struct isert_np {
@@ -212,4 +216,5 @@ struct isert_np {
struct mutex mutex;
struct list_head accepted;
struct list_head pending;
+ struct kref kref;
};
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 3/6] IB/isert: tear connections down from an ib_client on device removal
2026-11-04 1:22 ` [PATCH v7 3/6] IB/isert: tear connections down from an ib_client on device removal Yehyeong Lee
@ 2026-11-04 1:58 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-11-04 1:58 UTC (permalink / raw)
To: Yehyeong Lee; +Cc: linux-rdma, target-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] IB/isert: NULL cm_id dereference in isert_qp_event_callback()
--
commit 701b903737b3f3f8c4ac6d7d5a75f88a91eb010c
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
IB/isert: tear connections down from an ib_client on device removal
This patch registers an ib_client for isert to tear down device connections
from its remove callback instead of waiting and freeing connections inside the
RDMA_CM_EVENT_DEVICE_REMOVAL handler. It also adds reference counting for
isert_np and tracks connections on isert_conn_list under isert_conn_mutex.
> diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
> index 61b214012a70b..4812dabd93945 100644
> --- a/drivers/infiniband/ulp/isert/ib_isert.c
> +++ b/drivers/infiniband/ulp/isert/ib_isert.c
[ ... ]
> @@ -661,20 +721,33 @@ isert_connect_error(struct rdma_cm_id *cma_id)
> {
> struct isert_conn *isert_conn = cma_id->qp->qp_context;
> struct isert_np *isert_np = cma_id->context;
> - bool owned;
> + bool owned = false, bound = false;
>
> + mutex_lock(&isert_conn->mutex);
> mutex_lock(&isert_np->mutex);
> - owned = !list_empty(&isert_conn->node);
> - list_del_init(&isert_conn->node);
> + if (list_empty(&isert_conn->node)) {
> + bound = isert_conn->state == ISER_CONN_BOUND ||
> + isert_conn->state == ISER_CONN_FULL_FEATURE;
> + } else {
> + owned = true;
> + list_del_init(&isert_conn->node);
> + }
> mutex_unlock(&isert_np->mutex);
>
> + if (owned) {
> + isert_conn->cm_id = NULL;
> + } else if (bound) {
> + isert_conn_terminate(isert_conn);
> + iscsit_cause_connection_reinstatement(isert_conn->conn, 0);
> + }
> + mutex_unlock(&isert_conn->mutex);
> +
> if (!owned) {
> isert_info("conn %p is being released already\n", isert_conn);
> return 0;
> }
>
> ib_drain_qp(isert_conn->qp);
> - isert_conn->cm_id = NULL;
> isert_put_conn(isert_conn);
>
> return -1;
[Severity: High]
Can clearing isert_conn->cm_id before draining and destroying the QP cause a
NULL pointer dereference in isert_qp_event_callback()?
During passive connection setup, isert_connect_request() creates
isert_conn->qp with isert_qp_event_callback() as its event handler and calls
isert_rdma_accept() -> rdma_accept(), which transitions the QP to RTR/RTS.
When RDMA_CM_EVENT_UNREACHABLE, RDMA_CM_EVENT_REJECTED, or
RDMA_CM_EVENT_CONNECT_ERROR invokes isert_connect_error() while the connection
is still on isert_np->accepted or isert_np->pending (owned is true),
isert_conn->cm_id is now set to NULL before ib_drain_qp(isert_conn->qp) runs
and before the QP is destroyed. In addition, if isert_remove_one() holds a
reference on isert_conn via kref_get_unless_zero(), isert_put_conn() in
isert_connect_error() will not drop kref to zero, leaving isert_conn->qp alive
with isert_conn->cm_id == NULL until isert_remove_one() drops its reference.
On RDMA_CM_EVENT_UNREACHABLE, cma does not transition the QP to IB_QPS_ERR
before invoking the handler, and drivers such as mlx4 and mlx5 dispatch
IB_EVENT_COMM_EST asynchronously on a workqueue that is not synchronized by
ib_drain_qp(). If IB_EVENT_COMM_EST runs in isert_qp_event_callback() during
this window:
drivers/infiniband/ulp/isert/ib_isert.c:isert_qp_event_callback() {
...
switch (e->event) {
case IB_EVENT_COMM_EST:
rdma_notify(isert_conn->cm_id, IB_EVENT_COMM_EST);
break;
...
}
rdma_notify() will be called with a NULL cm_id and dereference
id_priv->cm_id.ib at a NULL base pointer.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104012219.1759523-1-yhlee@isslab.korea.ac.kr?part=3
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 4/6] IB/isert: drop the promotion reference when a connection errors out
2026-11-04 1:22 [PATCH v7 0/6] IB/isert: fix the device removal use-after-free Yehyeong Lee
` (2 preceding siblings ...)
2026-11-04 1:22 ` [PATCH v7 3/6] IB/isert: tear connections down from an ib_client on device removal Yehyeong Lee
@ 2026-11-04 1:22 ` Yehyeong Lee
2026-11-04 1:34 ` sashiko-bot
2026-11-04 1:22 ` [PATCH v7 5/6] IB/isert: do not build a new isert_device for a device being removed Yehyeong Lee
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Yehyeong Lee @ 2026-11-04 1:22 UTC (permalink / raw)
To: Sagi Grimberg, Max Gurtovoy, Jason Gunthorpe, Leon Romanovsky
Cc: Martin K . Petersen, Mike Christie, linux-rdma, target-devel,
linux-kernel, Yehyeong Lee, stable
isert_connected_handler() takes a reference when it moves a connection to
ISER_CONN_UP, and the paths that give a promoted connection up drop it
again. isert_connect_error() does not: it drops one reference for any
state. A promoted connection that then gets an error event keeps one
reference for ever, so isert_connect_release() never runs and the
isert_device reference it holds is never returned. isert_remove_one()
waits for that in wait_for_completion(&removal_comp).
A REJ after the RTU reaches this path. cm_rej_handler() accepts
IB_CM_ESTABLISHED while lap_state is IB_CM_LAP_UNINIT, and a passive id
stays in RDMA_CM_CONNECT after ESTABLISHED, so cma_ib_handler() passes the
event on as RDMA_CM_EVENT_REJECTED.
Drop the promotion reference too when the connection being given up was
promoted. A test-only knob delivered that event to eight promoted
connections. On patches 1 to 3 of this series the device delete did not
return within 240 seconds in any of three runs, each printed ten hung task
warnings, and isert_device_put() ran once, twice and twice, the first of
those being the one isert_remove_one() does itself. Three runs with this
patch ran it nine times and the delete returned in 15 seconds.
INFO: task rdma:206 blocked for more than 10 seconds.
Not tainted 7.4.0-rc1-NR3-00004-g11e5479fd20e #123
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:rdma state:D stack:26592 pid:206 tgid:206 ppid:1 task_flags:0x400100 flags:0x00080800
Call Trace:
<TASK>
__schedule+0x180f/0x4bb0
? filter_irq_stacks+0x71/0x90
? __pfx___schedule+0x10/0x10
? kasan_save_track+0x14/0x30
schedule+0x70/0x1f0
schedule_timeout+0x1cd/0x200
? __pfx_schedule_timeout+0x10/0x10
? vprintk_emit+0x200/0x2f0
? __pfx__raw_spin_lock_irq+0x10/0x10
__wait_for_common+0x29e/0x420
? __pfx_schedule_timeout+0x10/0x10
? __pfx___wait_for_common+0x10/0x10
? mutex_lock+0x79/0xd0
? mutex_unlock+0x7b/0xd0
? __pfx_mutex_unlock+0x10/0x10
isert_remove_one+0x529/0x710
? xas_load+0x1a/0x250
? __pfx_isert_remove_one+0x10/0x10
? xa_load+0xbf/0x120
? __pfx_xa_load+0x10/0x10
? __pfx_up_read+0x10/0x10
remove_client_context+0xa4/0xf0
disable_device+0x128/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:0x7fe0ecec20d7
RSP: 002b:00007fffe9d34978 EFLAGS: 00000202 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 00005621db1ab390 RCX: 00007fe0ecec20d7
RDX: 0000000000000018 RSI: 00005621db1aa2a0 RDI: 0000000000000004
RBP: 00007fffe9d349b0 R08: 00007fe0ecfb9200 R09: 000000000000000c
R10: 0000000000000000 R11: 0000000000000202 R12: 00007fffe9d34ba0
R13: 00007fffe9d34984 R14: 00007fffe9d34bc0 R15: 00007fffe9d34ba0
</TASK>
Fixes: 954f23722b57 ("iscsi,iser-target: Initiate termination only once")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
drivers/infiniband/ulp/isert/ib_isert.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 4812dabd939453..712d6165150e3c 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -721,7 +721,7 @@ isert_connect_error(struct rdma_cm_id *cma_id)
{
struct isert_conn *isert_conn = cma_id->qp->qp_context;
struct isert_np *isert_np = cma_id->context;
- bool owned = false, bound = false;
+ bool owned = false, bound = false, promoted = false;
mutex_lock(&isert_conn->mutex);
mutex_lock(&isert_np->mutex);
@@ -730,6 +730,7 @@ isert_connect_error(struct rdma_cm_id *cma_id)
isert_conn->state == ISER_CONN_FULL_FEATURE;
} else {
owned = true;
+ promoted = isert_conn->promoted;
list_del_init(&isert_conn->node);
}
mutex_unlock(&isert_np->mutex);
@@ -748,6 +749,8 @@ isert_connect_error(struct rdma_cm_id *cma_id)
}
ib_drain_qp(isert_conn->qp);
+ if (promoted)
+ isert_put_conn(isert_conn);
isert_put_conn(isert_conn);
return -1;
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 4/6] IB/isert: drop the promotion reference when a connection errors out
2026-11-04 1:22 ` [PATCH v7 4/6] IB/isert: drop the promotion reference when a connection errors out Yehyeong Lee
@ 2026-11-04 1:34 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-11-04 1:34 UTC (permalink / raw)
To: Yehyeong Lee; +Cc: linux-rdma
> isert_connected_handler() takes a reference when it moves a connection to
> ISER_CONN_UP, and the paths that give a promoted connection up drop it
> again. isert_connect_error() does not: it drops one reference for any
> state. A promoted connection that then gets an error event keeps one
> reference for ever, so isert_connect_release() never runs and the
> isert_device reference it holds is never returned. isert_remove_one()
> waits for that in wait_for_completion(&removal_comp).
>
> A REJ after the RTU reaches this path. cm_rej_handler() accepts
> IB_CM_ESTABLISHED while lap_state is IB_CM_LAP_UNINIT, and a passive id
> stays in RDMA_CM_CONNECT after ESTABLISHED, so cma_ib_handler() passes the
> event on as RDMA_CM_EVENT_REJECTED.
>
> Drop the promotion reference too when the connection being given up was
> promoted. A test-only knob delivered that event to eight promoted
> [ ... ]
> </TASK>
>
> Fixes: 954f23722b57 ("iscsi,iser-target: Initiate termination only once")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104012219.1759523-1-yhlee@isslab.korea.ac.kr?part=4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 5/6] IB/isert: do not build a new isert_device for a device being removed
2026-11-04 1:22 [PATCH v7 0/6] IB/isert: fix the device removal use-after-free Yehyeong Lee
` (3 preceding siblings ...)
2026-11-04 1:22 ` [PATCH v7 4/6] IB/isert: drop the promotion reference when a connection errors out Yehyeong Lee
@ 2026-11-04 1:22 ` Yehyeong Lee
2026-11-04 1:43 ` sashiko-bot
2026-11-04 1:22 ` [PATCH v7 6/6] IB/isert: only reinstate a connection that reached full feature phase Yehyeong Lee
2026-11-04 4:05 ` [PATCH v7 0/6] IB/isert: fix the device removal use-after-free Yehyeong Lee
6 siblings, 1 reply; 14+ messages in thread
From: Yehyeong Lee @ 2026-11-04 1:22 UTC (permalink / raw)
To: Sagi Grimberg, Max Gurtovoy, Jason Gunthorpe, Leon Romanovsky
Cc: Martin K . Petersen, Mike Christie, linux-rdma, target-devel,
linux-kernel, Yehyeong Lee, stable
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.
Let the ib_client data say whether a device may be used. isert_add_one()
stores a non-NULL value, isert_remove_one() clears it, and
isert_device_get() refuses a device whose data is NULL. The clear, the
lookup, the reference and the removing flag are one device_list_mutex
section, taken under isert_conn_mutex as in 3/6, so a device sharing a
node_guid cannot raise the reference count of one being removed.
A test-only delay before isert_device_get() opens the window. It opened
in three of fifteen runs of patches 1 to 4, which built a second
isert_device and took ten or eleven core warnings, and in six of eighteen
runs with this patch, which built one device and took none. Parking
isert_remove_one() after the lookup and connecting on a device sharing its
node_guid gave it two more references without this patch and none with it.
WARNING: drivers/infiniband/core/cq.c:370 at ib_cq_pool_cleanup+0x189/0x250, CPU#0: rdma/202
Modules linked in:
CPU: 0 UID: 0 PID: 202 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:ffff88810c737308 EFLAGS: 00010206
RAX: ffff88810a03fc40 RBX: ffff888104b5c000 RCX: 0000000000000007
RDX: 000000000000010c RSI: 0000000000000000 RDI: ffff888104b5c02c
RBP: dffffc0000000000 R08: 0000000000000001 R09: 00000000000000bf
R10: 0000000000000003 R11: 00000000d3012b20 R12: ffff8881047f0ad0
R13: dead000000000122 R14: dead000000000100 R15: ffff88810a03fc00
FS: 00007f8b74df1740(0000) GS:ffff888189e5e000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f9e81f41000 CR3: 0000000105f2b002 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:0x7f8b74f200d7
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:00007ffd34f4ef18 EFLAGS: 00000202 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 0000556b747c4390 RCX: 00007f8b74f200d7
RDX: 0000000000000018 RSI: 0000556b747c32a0 RDI: 0000000000000004
RBP: 00007ffd34f4ef50 R08: 00007f8b75017200 R09: 000000000000000c
R10: 0000000000000000 R11: 0000000000000202 R12: 00007ffd34f4f140
R13: 00007ffd34f4ef24 R14: 00007ffd34f4f160 R15: 00007ffd34f4f140
</TASK>
Fixes: 63b268d232b8 ("IB/isert: Properly release resources on DEVICE_REMOVAL")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
drivers/infiniband/ulp/isert/ib_isert.c | 41 +++++++++++++++++++------
1 file changed, 31 insertions(+), 10 deletions(-)
diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 712d6165150e3c..1b388075cc6332 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -41,6 +41,11 @@ module_param_cb(sg_tablesize, &sg_tablesize_ops, &isert_sg_tablesize, 0644);
MODULE_PARM_DESC(sg_tablesize,
"Number of gather/scatter entries in a single scsi command, should >= 128 (default: 128, max: 4096)");
+static struct ib_client isert_ib_client;
+
+/* Non-NULL client data on a device means it may be used. */
+static char isert_device_enabled;
+
static DEFINE_MUTEX(device_list_mutex);
static LIST_HEAD(device_list);
static DEFINE_MUTEX(isert_conn_mutex);
@@ -275,14 +280,19 @@ isert_device_get(struct rdma_cm_id *cma_id)
int ret;
mutex_lock(&device_list_mutex);
+ if (!ib_get_client_data(cma_id->device, &isert_ib_client)) {
+ mutex_unlock(&device_list_mutex);
+ return ERR_PTR(-ENODEV);
+ }
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);
- mutex_unlock(&device_list_mutex);
- return device;
- }
+ 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);
@@ -2790,22 +2800,25 @@ isert_remove_one(struct ib_device *ib_device, void *client_data)
struct isert_conn *isert_conn, *tmp;
LIST_HEAD(removal_list);
+ mutex_lock(&isert_conn_mutex);
mutex_lock(&device_list_mutex);
+ ib_set_client_data(ib_device, &isert_ib_client, NULL);
list_for_each_entry(device, &device_list, dev_node) {
if (device->ib_device != ib_device)
continue;
device->refcount++;
device->removal_comp = &removal_comp;
+ device->removing = true;
found = device;
break;
}
mutex_unlock(&device_list_mutex);
- if (!found)
+ if (!found) {
+ mutex_unlock(&isert_conn_mutex);
return;
+ }
- mutex_lock(&isert_conn_mutex);
- found->removing = true;
list_for_each_entry(isert_conn, &isert_conn_list, conn_list) {
if (isert_conn->device != found)
continue;
@@ -2828,8 +2841,16 @@ 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)
+{
+ ib_set_client_data(ib_device, &isert_ib_client, &isert_device_enabled);
+
+ return 0;
+}
+
static struct ib_client isert_ib_client = {
.name = "isert",
+ .add = isert_add_one,
.remove = isert_remove_one,
};
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 5/6] IB/isert: do not build a new isert_device for a device being removed
2026-11-04 1:22 ` [PATCH v7 5/6] IB/isert: do not build a new isert_device for a device being removed Yehyeong Lee
@ 2026-11-04 1:43 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-11-04 1:43 UTC (permalink / raw)
To: Yehyeong Lee; +Cc: linux-rdma, target-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] IB/isert: Cross-device aliasing via node_guid lookup in isert_device_get()
--
commit 01d8f2361924f11ef1e502a2b19ce2e53509658d
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
IB/isert: do not build a new isert_device for a device being removed
This patch uses ib_client data to track whether an ib_device is active and
skips devices marked as removing in isert_device_get() so a concurrent
CONNECT_REQUEST cannot create or grab an isert_device during device removal.
> diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
> index 712d6165150e3..1b388075cc633 100644
> --- a/drivers/infiniband/ulp/isert/ib_isert.c
> +++ b/drivers/infiniband/ulp/isert/ib_isert.c
[ ... ]
> @@ -275,14 +280,19 @@ isert_device_get(struct rdma_cm_id *cma_id)
> int ret;
>
> mutex_lock(&device_list_mutex);
> + if (!ib_get_client_data(cma_id->device, &isert_ib_client)) {
> + mutex_unlock(&device_list_mutex);
> + return ERR_PTR(-ENODEV);
> + }
> 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);
> - mutex_unlock(&device_list_mutex);
> - return device;
> - }
> + 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;
> }
[Severity: High]
As reported by Sashiko on v6 [1], when two ib_device instances share the same
node_guid (for example, software RDMA devices over netdevs sharing a MAC
address, which the commit message notes), can matching by node_guid in
isert_device_get() bind a connection on one ib_device to another ib_device's
isert_device?
Each isert_device is bound to a specific ib_device pointer
(device->ib_device = cma_id->device), allocates its protection domain on that
device in isert_create_device_ib_res(), and is looked up by pointer equality
in isert_remove_one():
drivers/infiniband/ulp/isert/ib_isert.c:isert_remove_one() {
...
list_for_each_entry(device, &device_list, dev_node) {
if (device->ib_device != ib_device)
continue;
device->refcount++;
device->removal_comp = &removal_comp;
device->removing = true;
found = device;
break;
}
}
In addition, while the commit message mentions that taking device_list_mutex
under isert_conn_mutex prevents a sibling device from raising the reference
count of one being removed, isert_device_get() only acquires
device_list_mutex and does not acquire isert_conn_mutex.
If ib_dev1 has an active isert_device in device_list and a CONNECT_REQUEST
arrives on ib_dev2 (which shares node_guid with ib_dev1), isert_device_get()
passes ib_get_client_data(cma_id->device, &isert_ib_client) for ib_dev2,
matches ib_dev1's isert_device by node_guid, increments its refcount, and
returns it.
In isert_connect_request(), isert_alloc_login_buf() then maps DMA buffers on
cma_id->device (ib_dev2), while isert_create_qp() allocates a CQ on
device->ib_device (ib_dev1) and calls rdma_create_qp(cma_id, device->pd,
&attr), which fails with -EINVAL because id->device != pd->device:
drivers/infiniband/ulp/isert/ib_isert.c:isert_connect_request() {
...
ret = isert_alloc_login_buf(isert_conn, cma_id->device);
if (ret)
goto out_conn_dev;
isert_set_nego_params(isert_conn, &event->param.conn);
isert_conn->qp = isert_create_qp(isert_conn, cma_id);
if (IS_ERR(isert_conn->qp)) {
ret = PTR_ERR(isert_conn->qp);
goto out_rsp_dma_map;
}
}
On the out_rsp_dma_map error path, isert_free_login_buf() unmaps
login_rsp_dma and login_desc->dma_addr using isert_conn->device->ib_device
(ib_dev1) instead of cma_id->device (ib_dev2), unmapping on the wrong device
and leaking the DMA mappings on ib_dev2:
drivers/infiniband/ulp/isert/ib_isert.c:isert_free_login_buf() {
struct ib_device *ib_dev = isert_conn->device->ib_device;
ib_dma_unmap_single(ib_dev, isert_conn->login_rsp_dma,
ISER_RX_PAYLOAD_SIZE, DMA_TO_DEVICE);
...
}
Conversely, if ib_dev2 already has an active isert_device in device_list and
ib_dev1 has none when a CONNECT_REQUEST arrives on ib_dev1 just before
isert_remove_one(ib_dev1) runs, isert_device_get() matches ib_dev2's
isert_device. When isert_remove_one(ib_dev1) then searches device_list for
device->ib_device == ib_dev1, found is NULL and it returns immediately
without waiting while isert_connect_request() is still mapping DMA buffers on
ib_dev1.
Should isert_device_get() match by device->ib_device != cma_id->device
instead of node_guid so that it is consistent with isert_remove_one()?
[1] https://lore.kernel.org/r/20261103223802.1697607-6-yhlee@isslab.korea.ac.kr
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104012219.1759523-1-yhlee@isslab.korea.ac.kr?part=5
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 6/6] IB/isert: only reinstate a connection that reached full feature phase
2026-11-04 1:22 [PATCH v7 0/6] IB/isert: fix the device removal use-after-free Yehyeong Lee
` (4 preceding siblings ...)
2026-11-04 1:22 ` [PATCH v7 5/6] IB/isert: do not build a new isert_device for a device being removed Yehyeong Lee
@ 2026-11-04 1:22 ` Yehyeong Lee
2026-11-04 1:37 ` sashiko-bot
2026-11-04 4:05 ` [PATCH v7 0/6] IB/isert: fix the device removal use-after-free Yehyeong Lee
6 siblings, 1 reply; 14+ messages in thread
From: Yehyeong Lee @ 2026-11-04 1:22 UTC (permalink / raw)
To: Sagi Grimberg, Max Gurtovoy, Jason Gunthorpe, Leon Romanovsky
Cc: Martin K . Petersen, Mike Christie, linux-rdma, target-devel,
linux-kernel, Yehyeong Lee, stable
isert_teardown_conn() and isert_connect_error() ask iscsi to reinstate the
connection whenever it is bound, ISER_CONN_BOUND included. A connection
stays BOUND from isert_accept_np() until isert_get_rx_pdu(); during the
final login response the rx thread is already running but still waits for
its login completion. iscsit_cause_connection_reinstatement() sends it
SIGINT, that wait fails, and iscsi_target_rx_thread() returns without the
connection exit path, so isert_remove_one() waits for ever.
Ask for reinstatement only from ISER_CONN_FULL_FEATURE, read under
isert_conn->mutex. There the rx thread is inside isert_get_rx_pdu() and
returns to the exit path, so the signal still helps. In BOUND terminating
is enough: if the login completes, isert_get_rx_pdu() sees
ISER_CONN_TERMINATING and returns; if it fails,
iscsi_target_login_sess_out() calls isert_wait_conn() and
isert_free_conn().
A test-only delay in that gap, with the device deleted inside it,
reproduced this on all three runs of patches 1 to 5: the rx thread was
signalled and the delete did not return within 240 seconds. Three runs
with this patch signalled nothing and the delete returned in 8 seconds.
INFO: task rdma:194 blocked for more than 10 seconds.
Not tainted 7.4.0-rc1-V7L5-00006-g4f59be58f97a #141
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:rdma state:D stack:26328 pid:194 tgid:194 ppid:1 task_flags:0x400100 flags:0x00080800
Call Trace:
<TASK>
__schedule+0x180f/0x4bb0
? __pfx___schedule+0x10/0x10
? __pfx_complete_signal+0x10/0x10
? irq_work_queue+0x1c/0xb0
schedule+0x70/0x1f0
schedule_timeout+0x1cd/0x200
? __pfx_schedule_timeout+0x10/0x10
? vprintk_emit+0x200/0x2f0
? __pfx__raw_spin_lock_irq+0x10/0x10
__wait_for_common+0x29e/0x420
? __pfx_schedule_timeout+0x10/0x10
? __pfx___wait_for_common+0x10/0x10
? mutex_lock+0x79/0xd0
? mutex_unlock+0x7b/0xd0
? __pfx_mutex_unlock+0x10/0x10
isert_remove_one+0x554/0x740
? xas_load+0x1a/0x250
? __pfx_isert_remove_one+0x10/0x10
? xa_load+0xbf/0x120
? __pfx_xa_load+0x10/0x10
? __pfx_up_read+0x10/0x10
remove_client_context+0xa4/0xf0
disable_device+0x128/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:0x7f1de13200d7
RSP: 002b:00007fff7f1dd9d8 EFLAGS: 00000202 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 0000563429332390 RCX: 00007f1de13200d7
RDX: 0000000000000018 RSI: 00005634293312a0 RDI: 0000000000000004
RBP: 00007fff7f1dda10 R08: 00007f1de1417200 R09: 000000000000000c
R10: 0000000000000000 R11: 0000000000000202 R12: 00007fff7f1ddc00
R13: 00007fff7f1dd9e4 R14: 00007fff7f1ddc20 R15: 00007fff7f1ddc00
</TASK>
Fixes: 6d1fba0c2cc7 ("iser-target: Rework connection termination")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
drivers/infiniband/ulp/isert/ib_isert.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 1b388075cc6332..1ed8dbe067ab88 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -682,14 +682,14 @@ static void
isert_teardown_conn(struct isert_conn *isert_conn)
{
struct isert_np *isert_np = isert_conn->np;
- bool owned = false, bound = false;
+ bool owned = false, bound = false, full_feature = false;
bool promoted = false, terminated = false;
mutex_lock(&isert_conn->mutex);
mutex_lock(&isert_np->mutex);
if (list_empty(&isert_conn->node)) {
- bound = isert_conn->state == ISER_CONN_BOUND ||
- isert_conn->state == ISER_CONN_FULL_FEATURE;
+ full_feature = isert_conn->state == ISER_CONN_FULL_FEATURE;
+ bound = full_feature || isert_conn->state == ISER_CONN_BOUND;
} else {
owned = true;
promoted = isert_conn->promoted;
@@ -701,7 +701,9 @@ isert_teardown_conn(struct isert_conn *isert_conn)
terminated = isert_conn_terminate(isert_conn);
} else if (bound) {
isert_conn_terminate(isert_conn);
- iscsit_cause_connection_reinstatement(isert_conn->conn, 0);
+ if (full_feature)
+ iscsit_cause_connection_reinstatement(isert_conn->conn,
+ 0);
}
mutex_unlock(&isert_conn->mutex);
@@ -732,12 +734,13 @@ isert_connect_error(struct rdma_cm_id *cma_id)
struct isert_conn *isert_conn = cma_id->qp->qp_context;
struct isert_np *isert_np = cma_id->context;
bool owned = false, bound = false, promoted = false;
+ bool full_feature = false;
mutex_lock(&isert_conn->mutex);
mutex_lock(&isert_np->mutex);
if (list_empty(&isert_conn->node)) {
- bound = isert_conn->state == ISER_CONN_BOUND ||
- isert_conn->state == ISER_CONN_FULL_FEATURE;
+ full_feature = isert_conn->state == ISER_CONN_FULL_FEATURE;
+ bound = full_feature || isert_conn->state == ISER_CONN_BOUND;
} else {
owned = true;
promoted = isert_conn->promoted;
@@ -749,7 +752,9 @@ isert_connect_error(struct rdma_cm_id *cma_id)
isert_conn->cm_id = NULL;
} else if (bound) {
isert_conn_terminate(isert_conn);
- iscsit_cause_connection_reinstatement(isert_conn->conn, 0);
+ if (full_feature)
+ iscsit_cause_connection_reinstatement(isert_conn->conn,
+ 0);
}
mutex_unlock(&isert_conn->mutex);
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 6/6] IB/isert: only reinstate a connection that reached full feature phase
2026-11-04 1:22 ` [PATCH v7 6/6] IB/isert: only reinstate a connection that reached full feature phase Yehyeong Lee
@ 2026-11-04 1:37 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-11-04 1:37 UTC (permalink / raw)
To: Yehyeong Lee; +Cc: linux-rdma
> isert_teardown_conn() and isert_connect_error() ask iscsi to reinstate the
> connection whenever it is bound, ISER_CONN_BOUND included. A connection
> stays BOUND from isert_accept_np() until isert_get_rx_pdu(); during the
> final login response the rx thread is already running but still waits for
> its login completion. iscsit_cause_connection_reinstatement() sends it
> SIGINT, that wait fails, and iscsi_target_rx_thread() returns without the
> connection exit path, so isert_remove_one() waits for ever.
>
> Ask for reinstatement only from ISER_CONN_FULL_FEATURE, read under
> isert_conn->mutex. There the rx thread is inside isert_get_rx_pdu() and
> returns to the exit path, so the signal still helps. In BOUND terminating
> is enough: if the login completes, isert_get_rx_pdu() sees
> ISER_CONN_TERMINATING and returns; if it fails,
> iscsi_target_login_sess_out() calls isert_wait_conn() and
> isert_free_conn().
> [ ... ]
> </TASK>
>
> Fixes: 6d1fba0c2cc7 ("iser-target: Rework connection termination")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104012219.1759523-1-yhlee@isslab.korea.ac.kr?part=6
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 0/6] IB/isert: fix the device removal use-after-free
2026-11-04 1:22 [PATCH v7 0/6] IB/isert: fix the device removal use-after-free Yehyeong Lee
` (5 preceding siblings ...)
2026-11-04 1:22 ` [PATCH v7 6/6] IB/isert: only reinstate a connection that reached full feature phase Yehyeong Lee
@ 2026-11-04 4:05 ` Yehyeong Lee
6 siblings, 0 replies; 14+ messages in thread
From: Yehyeong Lee @ 2026-11-04 4:05 UTC (permalink / raw)
To: Sagi Grimberg, Max Gurtovoy, Jason Gunthorpe, Leon Romanovsky
Cc: Martin K . Petersen, Mike Christie, linux-rdma, target-devel,
linux-kernel, Yehyeong Lee
This series is superseded by v8, which addresses the two Sashiko
reviews of v7: 3/6 clears cm_id after ib_drain_qp() again, and 5/6
looks the isert_device up by ib_device pointer.
https://lore.kernel.org/r/20261104040318.1844865-1-yhlee@isslab.korea.ac.kr
^ permalink raw reply [flat|nested] 14+ messages in thread