[PATCH v2] scsi: target: Fix NULL se_tpg dereference in target_complete()

[Yehyeong Lee]

Commit a47fa41381a0 ("scsi: target: Fix NULL dereference on XCOPY
completion") guarded target_complete() against a NULL se_tpg_wwn, which the
EXTENDED COPY worker hits because its global xcopy_pt_tpg carries no
se_tpg_wwn.  target_complete() dereferences
cmd->se_sess->se_tpg->se_tpg_wwn; that fix handled the innermost pointer.

The same statement can fault one level earlier: se_sess->se_tpg itself may
be NULL.  After a session is deregistered, transport_deregister_session()
clears se_sess->se_tpg, yet the isert receive path can still submit and
complete a command that references that session, so the backend completion
runs target_complete() with se_tpg == NULL and dereferences it:

  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 exact window in which an isert completion outlives the session
deregistration has not been pinned down; the guard defends the dereference
regardless of how the command reaches this point.

Load se_tpg first and fall back to a NULL wwn when it is absent.  The
read is lockless - transport_deregister_session() clears the field under
se_tpg->session_lock, which target_complete() does not hold - so use
READ_ONCE() to keep the compiler from re-fetching se_sess->se_tpg after
the NULL test.  The existing "if (!wwn)" path already queues the
completion on cmd->cpuid, so no new branch is needed; this extends that
guard by one pointer.

target_submit() reads the same chain (se_sess->se_tpg->se_tpg_tfo) and is
exposed to the identical NULL.  That path has not been reproduced here and
is left for a separate change.

Reproduced with soft-RoCE (rdma_rxe) and KASAN under iSCSI connection
churn; no kernel-side test hooks were needed.

Fixes: 39ae3edda325 ("scsi: target: core: Make completion affinity configurable")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Yehyeong Lee <yhlee@xxxxxxxxxxxxxxxxxx>
---

v2:
- Use READ_ONCE() for the lockless read of se_sess->se_tpg.  The field is
  cleared concurrently by transport_deregister_session(), so nothing
  prevents the compiler from re-fetching it after the NULL test.  The
  store side is a plain aligned pointer write under se_tpg->session_lock
  and is left unannotated; happy to add WRITE_ONCE() there as well if
  preferred.
- Retested from scratch; every number below is from the v2 build.

Tested on 7.2.0-rc4 with soft-RoCE (rdma_rxe) + KASAN under iSCSI/iSER
connection churn.  Unpatched, 5 of 10 runs oops with the same RIP
(target_complete_cmd_with_sense.part.0+0x17d).  With this patch, 20
consecutive runs show no oops.

To confirm that the guard - and not an incidental timing change - is what
prevents the crash, a build carrying an additional print inside the new
NULL branch was run 10 more times: a NULL se_tpg was observed 9 times
across 7 of those runs and was handled every time, with no oops.

Some runs end early with an initiator-side login timeout from the
soft-RoCE rig instead of completing the full 160-cycle workload: 4 of the
20 patched runs, and 2 of the 5 unpatched runs that did not oops (the
other 5 unpatched runs time out after the oops).  The rig behaves the
same way with and without the patch.

The BUG_ON(!se_tpg) in target_init_cmd() and target_submit_tmr() can be
reached by the same teardown race on fabrics that use those helpers.
iscsit does not - it calls __target_init_cmd() directly and has its own
TMF path - so those paths could not be reproduced or tested here, and
converting a deliberate assertion into graceful failure looked like a
decision for the maintainers rather than something to fold into this fix.

 drivers/target/target_core_transport.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index fad03a15c969..0b292e386691 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -904,7 +904,8 @@ static bool target_cmd_interrupted(struct se_cmd *cmd)
 
 static void target_complete(struct se_cmd *cmd, int success)
 {
-	struct se_wwn *wwn = cmd->se_sess->se_tpg->se_tpg_wwn;
+	struct se_portal_group *se_tpg = READ_ONCE(cmd->se_sess->se_tpg);
+	struct se_wwn *wwn = se_tpg ? se_tpg->se_tpg_wwn : NULL;
 	struct se_dev_attrib *da;
 	u8 compl_type;
 	int cpu;
-- 
2.43.0