[PATCH v3 1/2] IB/isert: delay the final Login Response until the session is registered

[PATCH v3 1/2] IB/isert: delay the final Login Response until the session is registered

[Yehyeong Lee]

isert_put_login_tx() puts the final Login Response on the wire before
__transport_register_session(), which iscsi_post_login_handler() reaches
only after iscsi_target_do_login() returns.  An initiator that issues a
SCSI command as soon as it sees that response can have it executed against
an se_session whose se_tpg is still NULL, and the ib-comp-wq worker oopses
on the NULL dereference.

  [    6.469002] Oops: general protection fault, probably for non-canonical address 0xdffffc000000000f: 0000 [#1] SMP KASAN NOPTI
  [    6.469652] KASAN: null-ptr-deref in range [0x0000000000000078-0x000000000000007f]
  [    6.470053] CPU: 0 UID: 0 PID: 178 Comm: kworker/0:1H Not tainted 7.2.0-rc5-V2CTL-gf5098b6bae76 #10 PREEMPT(lazy)
  [    6.470560] 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
  [    6.471120] Workqueue: ib-comp-wq ib_cq_poll_work
  [    6.471372] RIP: 0010:target_submit+0xbe/0x390
  [    6.471599] Code: fa 48 c1 ea 03 80 3c 02 00 0f 85 89 02 00 00 48 b8 00 00 00 00 00 fc ff df 4d 8b 64 24 18 49 8d 7c 24 78 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 5a 02 00 00 48 8d 7b 78 4d 8b 6c 24 78 48 b8 00
  [    6.472497] RSP: 0018:ffff8881058cfa78 EFLAGS: 00010206
  [    6.472761] RAX: dffffc0000000000 RBX: ffff88810c78c6f0 RCX: ffffffff964bb363
  [    6.473112] RDX: 000000000000000f RSI: 00000000fffffe00 RDI: 0000000000000078
  [    6.473439] RBP: 1ffff11020b19f52 R08: 0000000000000001 R09: ffffed1020b19f52
  [    6.473790] R10: 0000000000000003 R11: ffff88810596c000 R12: 0000000000000000
  [    6.474153] R13: ffff88810c61b000 R14: ffff88810c6a3400 R15: ffff88810c61b044
  [    6.474479] FS:  0000000000000000(0000) GS:ffff8881822b2000(0000) knlGS:0000000000000000
  [    6.474884] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  [    6.475156] CR2: 00007f1f1b83c000 CR3: 000000006fe72001 CR4: 0000000000770ef0
  [    6.475483] PKRU: 55555554
  [    6.475628] Call Trace:
  [    6.475763]  <TASK>
  [    6.475886]  ? __pfx__raw_spin_lock_bh+0x10/0x10
  [    6.476146]  ? __pfx_target_submit+0x10/0x10
  [    6.476348]  ? mutex_lock+0x81/0xe0
  [    6.476517]  ? __pfx_mutex_lock+0x10/0x10
  [    6.476722]  ? iscsit_execute_cmd+0x650/0x850
  [    6.476958]  iscsit_sequence_cmd+0x186/0x3d0
  [    6.477164]  iscsit_process_scsi_cmd+0x87/0x300
  [    6.477362]  isert_recv_done+0x1002/0x2390
  [    6.477546]  ? __pfx_isert_recv_done+0x10/0x10
  [    6.477761]  ? rxe_poll_cq+0x253/0x3d0
  [    6.477945]  ? finish_task_switch.isra.0+0x1dc/0xa70
  [    6.478192]  __ib_process_cq+0xe1/0x390
  [    6.478360]  ib_cq_poll_work+0x46/0x150
  [    6.478531]  process_one_work+0x633/0x1030
  [    6.478730]  ? assign_work+0x11d/0x370
  [    6.478918]  worker_thread+0x45b/0xd10
  [    6.479099]  ? __pfx_worker_thread+0x10/0x10
  [    6.479295]  ? __pfx_worker_thread+0x10/0x10
  [    6.479480]  kthread+0x2c6/0x3b0
  [    6.479632]  ? recalc_sigpending+0x15c/0x1e0
  [    6.479848]  ? __pfx_kthread+0x10/0x10
  [    6.480029]  ret_from_fork+0x36e/0x5a0
  [    6.480214]  ? __pfx_ret_from_fork+0x10/0x10
  [    6.480399]  ? __switch_to+0x572/0xdd0
  [    6.480566]  ? __pfx_kthread+0x10/0x10
  [    6.480748]  ret_from_fork_asm+0x1a/0x30
  [    6.480952]  </TASK>
  [    6.481062] Modules linked in:
  [    6.481235] ---[ end trace 0000000000000000 ]---

Delay the final Login Response instead.  isert_get_rx_pdu() runs from
iscsi_target_rx_thread() after conn->rx_login_comp, completed by
iscsi_post_login_handler() after __transport_register_session(); iscsi-TCP
and cxgbit already take PDUs from that thread, isert alone does not.  The
buffers are still posted first, so the initiator's first command does not
meet an empty receive queue and nothing depends on RNR flow control, and
the header and payload live in isert_conn, not in the struct iscsi_login
that iscsi_target_nego_release() frees first.

Over rxe, 400 login cycles per run, the oops appeared in 10 of 20
unpatched runs and in none of 20 runs with this patch.  An
initiator that never waits is handled by the next patch.

Not tested: iWARP, discovery sessions over iSER, and real HCAs.

Fixes: b8d26b3be8b3 ("iser-target: Add iSCSI Extensions for RDMA (iSER) target driver")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
v3: unmap login_tx_desc in isert_connect_release() (1/2); make the
    FULL_FEATURE store a release and the state check an acquire load, and
    log the value actually tested (2/2).  Both from Sashiko's review of v2.
    v2: https://lore.kernel.org/all/20260730063124.1554460-1-yhlee@isslab.korea.ac.kr/

 drivers/infiniband/ulp/isert/ib_isert.c | 20 ++++++++++++++++++--
 drivers/infiniband/ulp/isert/ib_isert.h |  1 +
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 1015a51f750a..e117cd2e62c9 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -59,6 +59,8 @@ 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);
 static void isert_login_send_done(struct ib_cq *cq, struct ib_wc *wc);
+static void isert_unmap_tx_desc(struct iser_tx_desc *tx_desc,
+				struct ib_device *ib_dev);
 
 static int isert_sg_tablesize_set(const char *val, const struct kernel_param *kp)
 {
@@ -496,6 +498,8 @@ isert_connect_release(struct isert_conn *isert_conn)
 	if (isert_conn->qp)
 		isert_destroy_qp(isert_conn);
 
+	isert_unmap_tx_desc(&isert_conn->login_tx_desc, device->ib_device);
+
 	if (isert_conn->login_desc)
 		isert_free_login_buf(isert_conn);
 
@@ -956,14 +960,17 @@ isert_put_login_tx(struct iscsit_conn *conn, struct iscsi_login *login,
 			mutex_lock(&isert_conn->mutex);
 			isert_conn->state = ISER_CONN_FULL_FEATURE;
 			mutex_unlock(&isert_conn->mutex);
-			goto post_send;
+
+			/* Sent from isert_get_rx_pdu() after registration. */
+			isert_conn->login_rsp_pending = true;
+			return 0;
 		}
 
 		ret = isert_login_post_recv(isert_conn);
 		if (ret)
 			return ret;
 	}
-post_send:
+
 	ret = isert_login_post_send(isert_conn, tx_desc);
 	if (ret)
 		return ret;
@@ -2585,8 +2592,17 @@ static void isert_free_conn(struct iscsit_conn *conn)
 
 static void isert_get_rx_pdu(struct iscsit_conn *conn)
 {
+	struct isert_conn *isert_conn = conn->context;
 	struct completion comp;
 
+	/* The session is registered by now; see isert_put_login_tx(). */
+	if (isert_conn->login_rsp_pending) {
+		isert_conn->login_rsp_pending = false;
+		if (isert_login_post_send(isert_conn,
+					  &isert_conn->login_tx_desc))
+			return;
+	}
+
 	init_completion(&comp);
 
 	wait_for_completion_interruptible(&comp);
diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h
index 0b2dfd6e7e27..0bac5aa66c80 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.h
+++ b/drivers/infiniband/ulp/isert/ib_isert.h
@@ -178,6 +178,7 @@ struct isert_conn {
 	struct completion	login_comp;
 	struct completion	login_req_comp;
 	struct iser_tx_desc	login_tx_desc;
+	bool			login_rsp_pending;
 	struct rdma_cm_id	*cm_id;
 	struct ib_qp		*qp;
 	struct ib_cq		*cq;
-- 
2.43.0

[PATCH v3 2/2] IB/isert: reject full-feature PDUs that arrive before registration

[Yehyeong Lee]

The full-feature receive buffers are posted from isert_put_login_tx(),
which runs before __transport_register_session().  An initiator that does
not wait for the final Login Response can therefore still have a SCSI
command executed against an se_session whose se_tpg is NULL, with the same
oops as the previous patch.

Move the ISER_CONN_FULL_FEATURE transition into isert_get_rx_pdu(), where
it happens after the session has been registered, and reinstate the
connection on any PDU that arrives before it, the same call this function
already makes when a work completion fails.  The state is read without
isert_conn->mutex because isert_recv_done() runs on ib-comp-wq; the acquire
load pairs with the release store so that the session registration
preceding it is visible.

Measured over rxe with an initiator that fires SCSI commands into that
window without waiting for the Login Response, 400 login cycles per run:
the oops appeared in 7 of 10 runs with only the previous patch applied and
in 0 of 10 runs with this one on top.  The check fired 22 times across
those 10 runs, 16 in ISER_CONN_BOUND and 6 in ISER_CONN_TERMINATING, where
the reinstatement is a no-op because teardown has already latched.
Well-formed traffic is unaffected: 20 runs of a spec-conforming initiator,
400 login cycles each, pass with the check never firing.

Fixes: b8d26b3be8b3 ("iser-target: Add iSCSI Extensions for RDMA (iSER) target driver")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
v3: unmap login_tx_desc in isert_connect_release() (1/2); make the
    FULL_FEATURE store a release and the state check an acquire load, and
    log the value actually tested (2/2).  Both from Sashiko's review of v2.
    v2: https://lore.kernel.org/all/20260730063124.1554460-1-yhlee@isslab.korea.ac.kr/

Depends on 1/2 and must not be backported without it.

 drivers/infiniband/ulp/isert/ib_isert.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index e117cd2e62c9..40290ab4bdb5 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -956,11 +956,6 @@ isert_put_login_tx(struct iscsit_conn *conn, struct iscsi_login *login,
 			if (ret)
 				return ret;
 
-			/* Now we are in FULL_FEATURE phase */
-			mutex_lock(&isert_conn->mutex);
-			isert_conn->state = ISER_CONN_FULL_FEATURE;
-			mutex_unlock(&isert_conn->mutex);
-
 			/* Sent from isert_get_rx_pdu() after registration. */
 			isert_conn->login_rsp_pending = true;
 			return 0;
@@ -1327,6 +1322,7 @@ isert_recv_done(struct ib_cq *cq, struct ib_wc *wc)
 	struct iser_ctrl *iser_ctrl = isert_get_iser_hdr(rx_desc);
 	uint64_t read_va = 0, write_va = 0;
 	uint32_t read_stag = 0, write_stag = 0;
+	enum iser_conn_state state;
 
 	if (unlikely(wc->status != IB_WC_SUCCESS)) {
 		isert_print_wc(wc, "recv");
@@ -1335,6 +1331,15 @@ isert_recv_done(struct ib_cq *cq, struct ib_wc *wc)
 		return;
 	}
 
+	/* A full-feature PDU before registration is a protocol violation. */
+	state = smp_load_acquire(&isert_conn->state);
+	if (unlikely(state != ISER_CONN_FULL_FEATURE)) {
+		isert_err("PDU received in state %d, resetting connection\n",
+			  state);
+		iscsit_cause_connection_reinstatement(isert_conn->conn, 0);
+		return;
+	}
+
 	rx_desc->in_use = true;
 
 	ib_dma_sync_single_for_cpu(ib_dev, rx_desc->dma_addr,
@@ -2598,6 +2603,12 @@ static void isert_get_rx_pdu(struct iscsit_conn *conn)
 	/* The session is registered by now; see isert_put_login_tx(). */
 	if (isert_conn->login_rsp_pending) {
 		isert_conn->login_rsp_pending = false;
+
+		mutex_lock(&isert_conn->mutex);
+		/* Pairs with the acquire in isert_recv_done(). */
+		smp_store_release(&isert_conn->state, ISER_CONN_FULL_FEATURE);
+		mutex_unlock(&isert_conn->mutex);
+
 		if (isert_login_post_send(isert_conn,
 					  &isert_conn->login_tx_desc))
 			return;
-- 
2.43.0

Re: [PATCH v3 2/2] IB/isert: reject full-feature PDUs that arrive before registration

[Leon Romanovsky]

On Thu, Jul 30, 2026 at 10:51:10PM +0900, Yehyeong Lee wrote:
> The full-feature receive buffers are posted from isert_put_login_tx(),
> which runs before __transport_register_session().  An initiator that does
> not wait for the final Login Response can therefore still have a SCSI
> command executed against an se_session whose se_tpg is NULL, with the same
> oops as the previous patch.
> 
> Move the ISER_CONN_FULL_FEATURE transition into isert_get_rx_pdu(), where
> it happens after the session has been registered, and reinstate the
> connection on any PDU that arrives before it, the same call this function
> already makes when a work completion fails.  The state is read without
> isert_conn->mutex because isert_recv_done() runs on ib-comp-wq; the acquire
> load pairs with the release store so that the session registration
> preceding it is visible.
> 
> Measured over rxe with an initiator that fires SCSI commands into that
> window without waiting for the Login Response, 400 login cycles per run:
> the oops appeared in 7 of 10 runs with only the previous patch applied and
> in 0 of 10 runs with this one on top.  The check fired 22 times across
> those 10 runs, 16 in ISER_CONN_BOUND and 6 in ISER_CONN_TERMINATING, where
> the reinstatement is a no-op because teardown has already latched.
> Well-formed traffic is unaffected: 20 runs of a spec-conforming initiator,
> 400 login cycles each, pass with the check never firing.
> 
> Fixes: b8d26b3be8b3 ("iser-target: Add iSCSI Extensions for RDMA (iSER) target driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
> ---
> v3: unmap login_tx_desc in isert_connect_release() (1/2); make the
>     FULL_FEATURE store a release and the state check an acquire load, and
>     log the value actually tested (2/2).  Both from Sashiko's review of v2.
>     v2: https://lore.kernel.org/all/20260730063124.1554460-1-yhlee@isslab.korea.ac.kr/

Sashiko is still not satisfied with this patch.

Thanks