[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

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

[Yehyeong Lee]

Thanks for the review.

On Wed, Jul 29, 2026 at 04:01:41PM +0300, Leon Romanovsky wrote:
> 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.

I will fix all three in v2.

> 4. Please address Sashiko's concerns about this patch.

Posting the receive buffers from the RX thread does leave the receive
queue empty between the final Login Response going out and
isert_get_rx_pdu() running. On IB and RoCE that is absorbed by the
rnr_retry_count = 7 that isert_rdma_accept() puts in the REP, but iWARP
has no RNR flow control and cma_accept_iw() ignores the field, so the
patch as posted is wrong there.

v2 will post the buffers in isert_put_login_tx() as before and delay the
final Login Response instead. An initiator cannot enter Full Feature
before it sees that response, so the queue is never empty and no command
can arrive before __transport_register_session().

Best regards,
Yehyeong