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

On 7/26/26 5:44 AM, Mike Christie wrote:
> I think we need to figure out how we got into this situation because we
> can't free the session while it's still in use.

You were right to ask.  The session is not being freed - it has not
finished being set up, and this patch is aimed at the wrong place.  I am
withdrawing it.

I instrumented transport_deregister_session() and
__transport_register_session() with a silent ring keyed on the
se_session pointer, and printed only from the cold NULL branch.  In a
login-churn workload that build recorded nine hits, and every one says
the same thing: the se_session had never passed
transport_deregister_session(); it had a matching
__transport_register_session() record; ktime_get_ns() taken at the read,
before any printk, is earlier than the timestamp taken at the top of
__transport_register_session() in every case; and conn->conn_state was
TARG_CONN_STATE_LOGGED_IN every time.

The window is in login, not teardown.  isert_put_login_tx() posts the
full-feature receive buffers and then sends the final Login Response;
iscsi_post_login_handler(), which calls __transport_register_session(),
only runs after that.  An initiator that sends a command as soon as it
sees the Login Response gets it received and completed against a session
whose se_tpg is still NULL.  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 is not exposed to this: it waits on
conn->rx_login_comp, which is completed at the end of
iscsi_post_login_handler(), after registration.  isert takes its PDUs
from the completion queue instead, and isert_recv_done() has no
connection-state check.

So the commit message on this patch is wrong.  It describes a
deregistration window that none of the nine observations involved.  I
described a mechanism I had not measured, and I will not do that again.

Guarding target_complete() is also the wrong shape.  In the same window
transport_lookup_cmd_lun() dereferences se_sess->se_tpg on the
virtual-LUN-0 fallback, so a command arriving before registration for an
unmapped LUN would fault there instead.

I have posted the isert fix to linux-rdma:

  Message-ID: <[email protected]>
  https://lore.kernel.org/linux-rdma/[email protected]/

It moves the isert_post_recvm() call into isert_get_rx_pdu(), which
iscsi_target_rx_thread() invokes after the same rx_login_comp wait, so
the buffers only become available once the session is registered.
Unpatched 9 of 10 runs oops, patched 0 of 10, run alternately in a
single session under KASAN with soft-RoCE.

Please drop this patch.

Best regards,