[PATCH] IB/isert: post full-feature receive buffers after session registration

[PATCH] IB/isert: post full-feature receive buffers after session registration

[Yehyeong Lee]

isert_put_login_tx() posts the full-feature receive buffers and then
sends the final Login Response on the same call:

	isert_alloc_rx_descriptors()
	isert_post_recvm(isert_conn, ISERT_QP_MAX_RECV_DTOS)
	isert_conn->state = ISER_CONN_FULL_FEATURE
	isert_login_post_send()

iscsi_post_login_handler() only runs after that, and it is what calls
__transport_register_session(), which sets se_sess->se_tpg.  An
initiator that sends a SCSI command as soon as it sees the Login
Response can therefore have that command received, executed and
completed before the session is registered, and target_complete()
dereferences a NULL se_sess->se_tpg:

  Oops: general protection fault, probably for non-canonical address ...
  KASAN: null-ptr-deref in range [0x0000000000000080-0x0000000000000087]
  Workqueue: ib-comp-wq ib_cq_poll_work
  RIP: 0010:target_complete_cmd_with_sense.part.0+0x17d/0xc90
  Call Trace:
   fd_execute_rw
   __target_execute_cmd
   iscsit_execute_cmd
   iscsit_sequence_cmd
   isert_recv_done
   __ib_process_cq
   ib_cq_poll_work

The command reaches the backend because transport_lookup_cmd_lun() does
not need se_tpg on the mapped-LUN path; it uses se_sess->se_node_acl,
which iscsit sets much earlier in iscsi_target_locate_portal().

iscsit's own RX thread does not have this problem: it is started at the
same point but waits on conn->rx_login_comp, which is completed at the
end of iscsi_post_login_handler(), after __transport_register_session().
isert takes its PDUs from the completion queue instead, and
isert_recv_done() has no connection-state check.

Post the receive buffers from isert_get_rx_pdu() instead.  That callback
is invoked by iscsi_target_rx_thread() only after the same rx_login_comp
wait, so the buffers become available to the HCA only once the session
is registered.  isert_recv_done() runs on ib-comp-wq and cannot wait
there itself.  The allocation stays in isert_put_login_tx() so that a
memory allocation failure still cannot happen after the final Login
Response has been sent.

rx_descs can legitimately be NULL in isert_get_rx_pdu(): the login
timeout timer sets login->login_failed from timer context, so
isert_put_login_tx() can skip the allocation branch while
iscsi_target_do_login() still returns 1 and the connection reaches
TARG_CONN_STATE_LOGGED_IN.  Return early in that case, which matches the
existing behaviour of not posting at all on that path.

Reproduced with soft-RoCE (rdma_rxe) under KASAN using an initiator that
issues a WRITE immediately after login, 400 login cycles per run.
Unpatched: 9 of 10 runs oopsed.  Patched: 0 of 10.  A build carrying a
diagnostic that reports the NULL without faulting recorded 16 hits
across 9 of 15 runs unpatched and 0 across 15 runs patched; the two sets
were run alternately in a single session.

Not covered by that testing: discovery sessions over iSER, MC/S
(non-zero TSIH) logins, ERL=2 connection recovery, and real HCAs, where
RNR behaviour differs from soft-RoCE.

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>
---
This replaces "scsi: target: Fix NULL se_tpg dereference in
target_complete()" (target-devel, Message-ID
20260725165818.769963-1-yhlee@isslab.korea.ac.kr), which guarded the
symptom in target core.  That patch is being withdrawn.

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

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 1015a51f750a..b055513a7bd8 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -943,15 +943,18 @@ isert_put_login_tx(struct iscsit_conn *conn, struct iscsi_login *login,
 	}
 	if (!login->login_failed) {
 		if (login->login_complete) {
+			/*
+			 * Allocate here, but do not post yet: the receive
+			 * buffers are posted from isert_get_rx_pdu() once the
+			 * session has been registered.  Allocating early keeps
+			 * the original property that a memory allocation
+			 * failure cannot happen after the final Login Response
+			 * has been sent.
+			 */
 			ret = isert_alloc_rx_descriptors(isert_conn);
 			if (ret)
 				return ret;
 
-			ret = isert_post_recvm(isert_conn,
-					       ISERT_QP_MAX_RECV_DTOS);
-			if (ret)
-				return ret;
-
 			/* Now we are in FULL_FEATURE phase */
 			mutex_lock(&isert_conn->mutex);
 			isert_conn->state = ISER_CONN_FULL_FEATURE;
@@ -2585,8 +2588,31 @@ 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;
 
+	/*
+	 * Post the full-feature receive buffers here rather than from
+	 * isert_put_login_tx().  iscsi_target_rx_thread() only calls us after
+	 * waiting on conn->rx_login_comp, which iscsi_post_login_handler()
+	 * completes after __transport_register_session().  Posting them in
+	 * isert_put_login_tx() - before the final Login Response goes on the
+	 * wire - lets the initiator's first command be received and executed
+	 * against an se_session whose se_tpg is still NULL.
+	 *
+	 * On failure just return: iscsi_target_rx_thread() sets
+	 * transport_failed and tears the connection down.
+	 *
+	 * rx_descs can be NULL here: the login error path completes
+	 * rx_login_comp without ever reaching the login_complete branch that
+	 * allocates them.
+	 */
+	if (!isert_conn->rx_descs)
+		return;
+
+	if (isert_post_recvm(isert_conn, ISERT_QP_MAX_RECV_DTOS))
+		return;
+
 	init_completion(&comp);
 
 	wait_for_completion_interruptible(&comp);
-- 
2.43.0

Re: [PATCH] IB/isert: post full-feature receive buffers after session registration

[Leon Romanovsky]

On Sun, Jul 26, 2026 at 11:42:13PM +0900, Yehyeong Lee wrote:
> isert_put_login_tx() posts the full-feature receive buffers and then
> sends the final Login Response on the same call:
> 
> 	isert_alloc_rx_descriptors()
> 	isert_post_recvm(isert_conn, ISERT_QP_MAX_RECV_DTOS)
> 	isert_conn->state = ISER_CONN_FULL_FEATURE
> 	isert_login_post_send()
> 
> iscsi_post_login_handler() only runs after that, and it is what calls
> __transport_register_session(), which sets se_sess->se_tpg.  An
> initiator that sends a SCSI command as soon as it sees the Login
> Response can therefore have that command received, executed and
> completed before the session is registered, and target_complete()
> dereferences a NULL se_sess->se_tpg:
> 
>   Oops: general protection fault, probably for non-canonical address ...
>   KASAN: null-ptr-deref in range [0x0000000000000080-0x0000000000000087]
>   Workqueue: ib-comp-wq ib_cq_poll_work
>   RIP: 0010:target_complete_cmd_with_sense.part.0+0x17d/0xc90
>   Call Trace:
>    fd_execute_rw
>    __target_execute_cmd
>    iscsit_execute_cmd
>    iscsit_sequence_cmd
>    isert_recv_done
>    __ib_process_cq
>    ib_cq_poll_work
> 
> The command reaches the backend because transport_lookup_cmd_lun() does
> not need se_tpg on the mapped-LUN path; it uses se_sess->se_node_acl,
> which iscsit sets much earlier in iscsi_target_locate_portal().
> 
> iscsit's own RX thread does not have this problem: it is started at the
> same point but waits on conn->rx_login_comp, which is completed at the
> end of iscsi_post_login_handler(), after __transport_register_session().
> isert takes its PDUs from the completion queue instead, and
> isert_recv_done() has no connection-state check.
> 
> Post the receive buffers from isert_get_rx_pdu() instead.  That callback
> is invoked by iscsi_target_rx_thread() only after the same rx_login_comp
> wait, so the buffers become available to the HCA only once the session
> is registered.  isert_recv_done() runs on ib-comp-wq and cannot wait
> there itself.  The allocation stays in isert_put_login_tx() so that a
> memory allocation failure still cannot happen after the final Login
> Response has been sent.
> 
> rx_descs can legitimately be NULL in isert_get_rx_pdu(): the login
> timeout timer sets login->login_failed from timer context, so
> isert_put_login_tx() can skip the allocation branch while
> iscsi_target_do_login() still returns 1 and the connection reaches
> TARG_CONN_STATE_LOGGED_IN.  Return early in that case, which matches the
> existing behaviour of not posting at all on that path.
> 
> Reproduced with soft-RoCE (rdma_rxe) under KASAN using an initiator that
> issues a WRITE immediately after login, 400 login cycles per run.
> Unpatched: 9 of 10 runs oopsed.  Patched: 0 of 10.  A build carrying a
> diagnostic that reports the NULL without faulting recorded 16 hits
> across 9 of 15 runs unpatched and 0 across 15 runs patched; the two sets
> were run alternately in a single session.
> 
> Not covered by that testing: discovery sessions over iSER, MC/S
> (non-zero TSIH) logins, ERL=2 connection recovery, and real HCAs, where
> RNR behaviour differs from soft-RoCE.
> 
> 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>
> ---
> This replaces "scsi: target: Fix NULL se_tpg dereference in
> target_complete()" (target-devel, Message-ID
> 20260725165818.769963-1-yhlee@isslab.korea.ac.kr), which guarded the
> symptom in target core.  That patch is being withdrawn.
> 
>  drivers/infiniband/ulp/isert/ib_isert.c | 36 +++++++++++++++++++++----
>  1 file changed, 31 insertions(+), 5 deletions(-)

1. Please keep commit messages concise and easy to read.
2. Include the full kernel Oops in the commit message.
3. Keep code comments to a minimum.
4. Please address Sashiko's concerns about this patch.

Thanks