[PATCH] scsi: elx: efct: hold a node reference across asynchronous session setup

[PATCH] scsi: elx: efct: hold a node reference across asynchronous session setup

[Yehyeong Lee]

efct_scsi_new_initiator() hands a bare struct efc_node pointer to lio_wq
and returns EFC_SCSI_CALL_ASYNC, so the node state machine keeps running
while efct_lio_setup_session() is still queued. The work item then
dereferences that pointer on both sides of a sleeping call.
efct_get_vport_tpg() reads node->nport before target_setup_session(),
efct_session_cb() reads it from inside that call, and node->tgt_node is
read after it returns.

Nothing keeps the node alive across that window. efc_node_alloc() takes
a single reference and efc_node_free() drops it as the last step of a
node shutdown. A shutdown posted from efc_nport_shutdown(), which an FC
link down, a domain or fabric change, a fabric that rejects FLOGI or
FDISC, or target teardown can all reach, unwinds through
efc_node_post_event() and returns the node to efc->node_pool while the
work item is in flight. The pool is created with curr_nr == min_nr, so
mempool_free() calls kfree() instead of refilling the reserve and the
work item reads freed slab memory:

  BUG: KASAN: slab-use-after-free in efct_lio_setup_session+0xad2/0xda0
  Read of size 8 at addr ffff8880029dd060 by task kworker/u4:1/27

  CPU: 0 UID: 0 PID: 27 Comm: kworker/u4:1 Tainted: G                 N  7.3.0-00342-g5b8610135c8d-dirty #2 PREEMPT(lazy)
  Tainted: [N]=TEST
  Workqueue: efct-kunit-wq efct_lio_setup_session
  Call Trace:
   <TASK>
   dump_stack_lvl+0x3f/0x50
   print_report+0xcd/0x620
   ? kfree+0xdd/0x300
   ? __pfx_mempool_kfree+0x10/0x10
   ? efct_lio_setup_session+0xad2/0xda0
   kasan_report+0xd9/0x110
   ? efct_lio_setup_session+0xad2/0xda0
   efct_lio_setup_session+0xad2/0xda0
   ? __pfx_try_to_wake_up+0x10/0x10
   ? __pfx_rpm_suspend+0x10/0x10
   ? __pfx_efct_lio_setup_session+0x10/0x10
   ? __dequeue_entity+0xe9a/0x20a0
   process_one_work+0x6b9/0x1160
   ? __pfx_process_one_work+0x10/0x10
   ? mutex_unlock+0x8f/0xf0
   ? __pfx_efct_lio_setup_session+0x10/0x10
   ? assign_work+0x11d/0x370
   worker_thread+0x575/0xf50
   ? __kthread_parkme+0x56/0x1a0
   ? __pfx_worker_thread+0x10/0x10
   kthread+0x33b/0x4a0
   ? __pfx_kthread+0x10/0x10
   ret_from_fork+0x347/0x620
   ? __pfx_ret_from_fork+0x10/0x10
   ? __switch_to+0x22/0xbd0
   ? __pfx_kthread+0x10/0x10
   ret_from_fork_asm+0x19/0x30
   </TASK>

  Allocated by task 32:
   kasan_save_stack+0x33/0x60
   kasan_save_track+0x14/0x30
   __kasan_kmalloc+0x8f/0xa0
   __kmalloc_noprof+0x19b/0x330
   mempool_alloc_noprof+0x131/0x210
   efct_kt_node_alloc.isra.0+0x38/0x460
   efct_kt_uaf_after_cb+0x7a/0x370
   kunit_try_run_case+0x1ca/0x4b0
   kunit_generic_run_threadfn_adapter+0x7b/0xe0
   kthread+0x33b/0x4a0
   ret_from_fork+0x347/0x620
   ret_from_fork_asm+0x19/0x30

  Freed by task 27:
   kasan_save_stack+0x33/0x60
   kasan_save_track+0x14/0x30
   kasan_save_free_info+0x3b/0x60
   __kasan_slab_free+0x43/0x70
   kfree+0xdd/0x300
   mempool_free+0xbc/0x100
   efc_node_free+0x308/0x4a0
   efct_lio_setup_session+0x93f/0xda0
   process_one_work+0x6b9/0x1160
   worker_thread+0x575/0xf50
   kthread+0x33b/0x4a0
   ret_from_fork+0x347/0x620
   ret_from_fork_asm+0x19/0x30

  The buggy address belongs to the object at ffff8880029dd000
   which belongs to the cache kmalloc-1k of size 1024
  The buggy address is located 96 bytes inside of
   freed 1024-byte region [ffff8880029dd000, ffff8880029dd400)

Take a reference on the node before queuing the work and drop it when the
work finishes, so the slab stays valid. Re-check under efc->lock, which
efc_node_free() holds while it clears node->nport and sets node->req_free,
that the node is still live before using it, and bail otherwise.
efct_session_cb() makes the same check, so a session is registered only for
a live node and efct_scsi_del_initiator() removes exactly those.

The lookup store moves under efc->lock and therefore to GFP_ATOMIC, and
the two early returns that leaked wq_data now share the common exit.

The KUnit harness used to order the free against the work item is not
part of this patch.

Fixes: 692e5d73a811 ("scsi: elx: efct: LIO backend interface routines")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
 drivers/scsi/elx/efct/efct_lio.c | 63 ++++++++++++++++++++++++++++++--
 1 file changed, 59 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/elx/efct/efct_lio.c b/drivers/scsi/elx/efct/efct_lio.c
index 67d686dd6fb33..add939366bc6a 100644
--- a/drivers/scsi/elx/efct/efct_lio.c
+++ b/drivers/scsi/elx/efct/efct_lio.c
@@ -1062,17 +1062,42 @@ _efct_tgt_node_free(struct kref *arg)
 	kfree(tgt_node);
 }
 
+/*
+ * Report whether the node state machine may still have a session registered
+ * for @node. The caller must hold efc->lock, which is also held by the paths
+ * that shut a node down.
+ */
+static bool efct_node_registrable(struct efc_node *node)
+{
+	return node->nport && !node->req_free;
+}
+
 static int efct_session_cb(struct se_portal_group *se_tpg,
 			   struct se_session *se_sess, void *private)
 {
 	struct efc_node *node = private;
 	struct efct_node *tgt_node;
 	struct efct *efct = node->efc->base;
+	unsigned long flags;
 
 	tgt_node = kzalloc_obj(*tgt_node);
 	if (!tgt_node)
 		return -ENOMEM;
 
+	/*
+	 * target_setup_session() sleeps, so the node may have entered
+	 * shutdown since it was called. Refuse the session in that case:
+	 * efct_scsi_del_initiator() only queues the removal work once
+	 * node->tgt_node is set, so either it sees the session below or
+	 * there is no session to remove at all.
+	 */
+	spin_lock_irqsave(&node->efc->lock, flags);
+	if (!efct_node_registrable(node)) {
+		spin_unlock_irqrestore(&node->efc->lock, flags);
+		kfree(tgt_node);
+		return -ENODEV;
+	}
+
 	kref_init(&tgt_node->ref);
 	tgt_node->release = _efct_tgt_node_free;
 
@@ -1086,6 +1111,7 @@ static int efct_session_cb(struct se_portal_group *se_tpg,
 	tgt_node->port_fc_id = node->nport->fc_id;
 	tgt_node->vpi = node->nport->indicator;
 	tgt_node->rpi = node->rnode.indicator;
+	spin_unlock_irqrestore(&node->efc->lock, flags);
 
 	spin_lock_init(&tgt_node->active_ios_lock);
 	INIT_LIST_HEAD(&tgt_node->active_ios);
@@ -1157,15 +1183,25 @@ static void efct_lio_setup_session(struct work_struct *work)
 		container_of(work, struct efct_lio_wq_data, work);
 	struct efct *efct = wq_data->efct;
 	struct efc_node *node = wq_data->ptr;
+	struct efc *efc = node->efc;
 	char wwpn[WWN_NAME_LEN];
 	struct efct_lio_tpg *tpg;
 	struct efct_node *tgt_node;
 	struct se_portal_group *se_tpg;
 	struct se_session *se_sess;
+	unsigned long flags;
 	int watermark;
 	int ini_count;
 	u64 id;
 
+	spin_lock_irqsave(&efc->lock, flags);
+	if (!efct_node_registrable(node)) {
+		spin_unlock_irqrestore(&efc->lock, flags);
+		efc_log_debug(efct, "node %p shut down before session setup\n",
+			      node);
+		goto done;
+	}
+
 	/* Check to see if it's belongs to vport,
 	 * if not get physical port
 	 */
@@ -1176,8 +1212,9 @@ static void efct_lio_setup_session(struct work_struct *work)
 		tpg = efct->tgt_efct.tpg;
 		se_tpg = &tpg->tpg;
 	} else {
+		spin_unlock_irqrestore(&efc->lock, flags);
 		efc_log_err(efct, "failed to init session\n");
-		return;
+		goto done;
 	}
 
 	/*
@@ -1186,14 +1223,28 @@ static void efct_lio_setup_session(struct work_struct *work)
 	 * ConfigFS NodeACLs.
 	 */
 	efct_format_wwn(wwpn, sizeof(wwpn), "",	efc_node_get_wwpn(node));
+	spin_unlock_irqrestore(&efc->lock, flags);
 
 	se_sess = target_setup_session(se_tpg, 0, 0, TARGET_PROT_NORMAL, wwpn,
 				       node, efct_session_cb);
 	if (IS_ERR(se_sess)) {
 		efc_log_err(efct, "failed to setup session\n");
-		kfree(wq_data);
+		spin_lock_irqsave(&efc->lock, flags);
+		if (!efct_node_registrable(node)) {
+			spin_unlock_irqrestore(&efc->lock, flags);
+			goto done;
+		}
+		spin_unlock_irqrestore(&efc->lock, flags);
 		efc_scsi_sess_reg_complete(node, -EIO);
-		return;
+		goto done;
+	}
+
+	spin_lock_irqsave(&efc->lock, flags);
+	if (!efct_node_registrable(node)) {
+		spin_unlock_irqrestore(&efc->lock, flags);
+		efc_log_debug(efct, "node %p shut down during session setup\n",
+			      node);
+		goto done;
 	}
 
 	tgt_node = node->tgt_node;
@@ -1202,8 +1253,9 @@ static void efct_lio_setup_session(struct work_struct *work)
 	efc_log_debug(efct, "new initiator sess=%p node=%p id: %llx\n",
 		      se_sess, node, id);
 
-	if (xa_err(xa_store(&efct->lookup, id, tgt_node, GFP_KERNEL)))
+	if (xa_err(xa_store(&efct->lookup, id, tgt_node, GFP_ATOMIC)))
 		efc_log_err(efct, "Node lookup store failed\n");
+	spin_unlock_irqrestore(&efc->lock, flags);
 
 	efc_scsi_sess_reg_complete(node, 0);
 
@@ -1215,7 +1267,9 @@ static void efct_lio_setup_session(struct work_struct *work)
 			efct->tgt_efct.watermark_min : watermark;
 	atomic_set(&efct->tgt_efct.io_high_watermark, watermark);
 
+done:
 	kfree(wq_data);
+	kref_put(&node->ref, node->release);
 }
 
 int efct_scsi_new_initiator(struct efc *efc, struct efc_node *node)
@@ -1231,6 +1285,7 @@ int efct_scsi_new_initiator(struct efc *efc, struct efc_node *node)
 	if (!wq_data)
 		return -ENOMEM;
 
+	kref_get(&node->ref);
 	wq_data->ptr = node;
 	wq_data->efct = efct;
 	INIT_WORK(&wq_data->work, efct_lio_setup_session);

base-commit: f47d223627f80cb7a1eb14c585fe6cb35de5e984
-- 
2.43.0

Re: [PATCH] scsi: elx: efct: hold a node reference across asynchronous session setup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] scsi: elx: efct: incomplete shutdown check in efct_node_registrable()
- [Critical] scsi: elx: efct: xa_erase() race with xa_store() in efct_lio_setup_session()
- [High] scsi: elx: efct: session leak on post-setup bailout in efct_lio_setup_session()
- [High] scsi: elx: efct: TOCTOU race before efc_scsi_sess_reg_complete()
--

commit 9faa68f52085fb309e7aece4f85020881ca6843a
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

scsi: elx: efct: hold a node reference across asynchronous session setup

This commit takes a kref on struct efc_node while efct_lio_setup_session() is
queued on lio_wq and adds efct_node_registrable() checks under efc->lock to
bail out if the node has shut down during asynchronous session setup.

> diff --git a/drivers/scsi/elx/efct/efct_lio.c b/drivers/scsi/elx/efct/efct_lio.c
> index 67d686dd6fb33..add939366bc6a 100644
> --- a/drivers/scsi/elx/efct/efct_lio.c
> +++ b/drivers/scsi/elx/efct/efct_lio.c
> @@ -1062,17 +1062,42 @@ _efct_tgt_node_free(struct kref *arg)
>  	kfree(tgt_node);
>  }
>  
> +/*
> + * Report whether the node state machine may still have a session registered
> + * for @node. The caller must hold efc->lock, which is also held by the paths
> + * that shut a node down.
> + */
> +static bool efct_node_registrable(struct efc_node *node)
> +{
> +	return node->nport && !node->req_free;
> +}
> +
>  static int efct_session_cb(struct se_portal_group *se_tpg,
>  			   struct se_session *se_sess, void *private)
>  {
>  	struct efc_node *node = private;
>  	struct efct_node *tgt_node;
>  	struct efct *efct = node->efc->base;
> +	unsigned long flags;
>  
>  	tgt_node = kzalloc_obj(*tgt_node);
>  	if (!tgt_node)
>  		return -ENOMEM;
>  
> +	/*
> +	 * target_setup_session() sleeps, so the node may have entered
> +	 * shutdown since it was called. Refuse the session in that case:
> +	 * efct_scsi_del_initiator() only queues the removal work once
> +	 * node->tgt_node is set, so either it sees the session below or
> +	 * there is no session to remove at all.
> +	 */
> +	spin_lock_irqsave(&node->efc->lock, flags);
> +	if (!efct_node_registrable(node)) {
> +		spin_unlock_irqrestore(&node->efc->lock, flags);
> +		kfree(tgt_node);
> +		return -ENODEV;
> +	}

[Severity: Critical]
Is checking node->nport and !node->req_free in efct_node_registrable()
sufficient to detect a node that has already entered shutdown?

When a remote initiator logs in and efc_d_send_prli_rsp() queues
efct_lio_setup_session(), a shutdown event arriving before efct_session_cb()
sets node->tgt_node runs __efc_d_initiate_shutdown() in
drivers/scsi/elx/libefc/efc_device.c:

    if (node->nport->enable_tgt)
        rc = efc->tt.scsi_del_node(efc, node,
            EFC_SCSI_INITIATOR_DELETED);

    if (rc == EFC_SCSI_CALL_COMPLETE || rc < 0)
        efc_node_post_event(node,
            EFC_EVT_NODE_DEL_INI_COMPLETE, NULL);

Because node->tgt_node is still NULL, efct_scsi_del_initiator() returns
-EIO without queuing efct_lio_remove_session().

In drivers/scsi/elx/libefc/efc_node.c, efc_node_initiate_cleanup() ->
efc_node_check_els_quiesced() then checks whether the hardware node is still
attached or ELS IOs are active:

    } else {
        /*
         * hw node detach hasn't completed,
         * transition and wait
         */
        node_printf(node, "HW node still attached\n");
        efc_node_transition(node, __efc_node_wait_node_free,
                            NULL);
    }

While the node waits in __efc_node_wait_node_free (or
__efc_node_wait_els_shutdown, or __efc_d_wait_del_node for an
initiator+target node), node->nport is still non-NULL (cleared only in
efc_node_free()) and node->req_free is still false (set only on entry to
__efc_node_shutdown()). Similarly, efc_node_handle_implicit_logo() and
efc_node_handle_explicit_logo() reset node->req_free = false while keeping
node->nport valid:

    /* Re-attach node with the same HW node resources */
    node->req_free = false;
    rc = efc_node_attach(node);
    efc_node_transition(node, __efc_d_wait_node_attach, NULL);

In all of these states, efct_node_registrable(node) returns true even though
efct_scsi_del_initiator() has already run for that login. When the detach
later completes and the node reaches efc_node_free() without calling
efct_scsi_del_initiator() again, won't this leak se_sess and tgt_node and
leave dangling pointers to the freed struct efc_node in efct->lookup and
se_sess->fabric_sess_ptr?

> +
>  	kref_init(&tgt_node->ref);
>  	tgt_node->release = _efct_tgt_node_free;
>  
> @@ -1086,6 +1111,7 @@ static int efct_session_cb(struct se_portal_group *se_tpg,
>  	tgt_node->port_fc_id = node->nport->fc_id;
>  	tgt_node->vpi = node->nport->indicator;
>  	tgt_node->rpi = node->rnode.indicator;
> +	spin_unlock_irqrestore(&node->efc->lock, flags);
>  
>  	spin_lock_init(&tgt_node->active_ios_lock);
>  	INIT_LIST_HEAD(&tgt_node->active_ios);
> @@ -1157,15 +1183,25 @@ static void efct_lio_setup_session(struct work_struct *work)
>  		container_of(work, struct efct_lio_wq_data, work);
>  	struct efct *efct = wq_data->efct;
>  	struct efc_node *node = wq_data->ptr;
> +	struct efc *efc = node->efc;
>  	char wwpn[WWN_NAME_LEN];
>  	struct efct_lio_tpg *tpg;
>  	struct efct_node *tgt_node;
>  	struct se_portal_group *se_tpg;
>  	struct se_session *se_sess;
> +	unsigned long flags;
>  	int watermark;
>  	int ini_count;
>  	u64 id;
>  
> +	spin_lock_irqsave(&efc->lock, flags);
> +	if (!efct_node_registrable(node)) {
> +		spin_unlock_irqrestore(&efc->lock, flags);
> +		efc_log_debug(efct, "node %p shut down before session setup\n",
> +			      node);
> +		goto done;
> +	}
> +
>  	/* Check to see if it's belongs to vport,
>  	 * if not get physical port
>  	 */
> @@ -1176,8 +1212,9 @@ static void efct_lio_setup_session(struct work_struct *work)
>  		tpg = efct->tgt_efct.tpg;
>  		se_tpg = &tpg->tpg;
>  	} else {
> +		spin_unlock_irqrestore(&efc->lock, flags);
>  		efc_log_err(efct, "failed to init session\n");
> -		return;
> +		goto done;
>  	}
>  
>  	/*
> @@ -1186,14 +1223,28 @@ static void efct_lio_setup_session(struct work_struct *work)
>  	 * ConfigFS NodeACLs.
>  	 */
>  	efct_format_wwn(wwpn, sizeof(wwpn), "",	efc_node_get_wwpn(node));
> +	spin_unlock_irqrestore(&efc->lock, flags);
>  
>  	se_sess = target_setup_session(se_tpg, 0, 0, TARGET_PROT_NORMAL, wwpn,
>  				       node, efct_session_cb);
>  	if (IS_ERR(se_sess)) {
>  		efc_log_err(efct, "failed to setup session\n");
> -		kfree(wq_data);
> +		spin_lock_irqsave(&efc->lock, flags);
> +		if (!efct_node_registrable(node)) {
> +			spin_unlock_irqrestore(&efc->lock, flags);
> +			goto done;
> +		}
> +		spin_unlock_irqrestore(&efc->lock, flags);
>  		efc_scsi_sess_reg_complete(node, -EIO);
> -		return;
> +		goto done;
>  	}

[Severity: High]
Can dropping efc->lock before calling efc_scsi_sess_reg_complete() here (and
before efc_scsi_sess_reg_complete(node, 0) below) race with node teardown?

In drivers/scsi/elx/libefc/efc_node.c, efc_scsi_sess_reg_complete()
re-acquires efc->lock and posts an event to the node:

    spin_lock_irqsave(&efc->lock, flags);
    /* Notify the node to resume */
    efc_node_post_event(node, evt, NULL);
    spin_unlock_irqrestore(&efc->lock, flags);

If the node finishes shutting down on another CPU in the window after
efct_lio_setup_session() releases efc->lock and before
efc_scsi_sess_reg_complete() re-acquires it, __efc_node_shutdown() sets
node->req_free = true and efc_node_post_event() calls efc_node_free(node),
which clears node->nport = NULL while leaving node->req_free set to true.

When efc_scsi_sess_reg_complete() then calls efc_node_post_event(),
node->evtdepth returns to 0 with node->req_free still true:

    if (node->evtdepth == 0 && node->req_free)
        free_node = true;

    if (free_node)
        efc_node_free(node);

In efc_node_free(), if node->refound is set, efc_node_find(node->nport,
FC_FID_DIR_SERV) runs before the !node->nport check:

    if (node->refound) {
        ns = efc_node_find(node->nport, FC_FID_DIR_SERV);
    }

    if (!node->nport) {
        efc_log_err(efc, "Node already Freed\n");
        return;
    }

Wouldn't calling efc_node_free() a second time dereference a NULL
node->nport inside efc_node_find()?

> +
> +	spin_lock_irqsave(&efc->lock, flags);
> +	if (!efct_node_registrable(node)) {
> +		spin_unlock_irqrestore(&efc->lock, flags);
> +		efc_log_debug(efct, "node %p shut down during session setup\n",
> +			      node);
> +		goto done;
> +	}

[Severity: High]
What cleans up se_sess and tgt_node when efct_lio_setup_session() takes this
goto done path after target_setup_session() has already succeeded?

By the time target_setup_session() returns a valid se_sess,
efct_session_cb() has allocated tgt_node and set node->tgt_node = tgt_node,
and target_setup_session() has registered se_sess with
se_sess->fabric_sess_ptr = node.

If efct_node_registrable(node) is false here (for instance, if the node
completed __efc_node_shutdown() and efc_node_free() while
transport_register_session() was running, or if efct_scsi_del_initiator()
ran after efct_session_cb() unlocked efc->lock and failed its GFP_ATOMIC
allocation of wq_data), efct_lio_remove_session() will not be queued for
this session.

Jumping directly to done frees wq_data and drops node->ref without calling
target_remove_session(se_sess) or freeing tgt_node. Doesn't this leak
se_sess and tgt_node, leave se_sess->fabric_sess_ptr pointing to a freed
struct efc_node, and leave efct->tgt_efct.initiator_count unbalanced if
efct_scsi_del_initiator() already decremented it?

>  
>  	tgt_node = node->tgt_node;
>  	id = (u64) tgt_node->port_fc_id << 32 | tgt_node->node_fc_id;
>  
>  	efc_log_debug(efct, "new initiator sess=%p node=%p id: %llx\n",
>  		      se_sess, node, id);
>  
> -	if (xa_err(xa_store(&efct->lookup, id, tgt_node, GFP_KERNEL)))
> +	if (xa_err(xa_store(&efct->lookup, id, tgt_node, GFP_ATOMIC)))
>  		efc_log_err(efct, "Node lookup store failed\n");
> +	spin_unlock_irqrestore(&efc->lock, flags);

[Severity: Critical]
Could xa_store() here insert tgt_node into efct->lookup after
efct_scsi_del_initiator() has already called xa_erase()?

Inside target_setup_session(), efct_session_cb() sets
node->tgt_node = tgt_node and releases node->efc->lock before
target_setup_session() returns to efct_lio_setup_session().

If the node enters shutdown during that unlocked window,
__efc_d_initiate_shutdown() -> efct_scsi_del_initiator() sees
node->tgt_node != NULL and erases id from efct->lookup before queuing
efct_lio_remove_session():

    id = (u64) tgt_node->port_fc_id << 32 | tgt_node->node_fc_id;
    xa_erase(&efct->lookup, id);

    wq_data->ptr = node;
    wq_data->efct = efct;
    INIT_WORK(&wq_data->work, efct_lio_remove_session);
    queue_work(lio_wq, &wq_data->work);

Because efct_scsi_del_initiator() returns EFC_SCSI_CALL_ASYNC, the node
waits in __efc_d_wait_del_node for efct_lio_remove_session() to run on
lio_wq behind efct_lio_setup_session(). In __efc_d_wait_del_node,
node->nport is non-NULL and node->req_free is false, so
efct_node_registrable(node) still evaluates to true.

efct_lio_setup_session() then runs xa_store(&efct->lookup, id, tgt_node,
GFP_ATOMIC) after the xa_erase() in efct_scsi_del_initiator() has already
happened. When efct_lio_remove_session() subsequently frees tgt_node
without erasing id again, won't efct->lookup retain a dangling tgt_node
pointer that efct_node_find() in drivers/scsi/elx/efct/efct_unsol.c can
dereference via kref_get(&node->ref)?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021022209.2580332-1-yhlee@isslab.korea.ac.kr?part=1

Re: [PATCH] scsi: elx: efct: hold a node reference across asynchronous session setup

[Yehyeong Lee]

Thanks for the review. The concern is right, so I have sent a v2 rather
than argue it.

v1 gated the setup work on node->nport and node->req_free, but that is
not enough. req_free is only set in __efc_node_shutdown(), which the node
reaches after efct_scsi_del_initiator() has already run and returned, so
a node sitting in __efc_d_wait_del_node still passes the test. On that
path the setup work can register a session that nothing ever removes, and
in the narrow window where efct_session_cb() has published node->tgt_node
it can store a lookup entry after efct_scsi_del_initiator() erased it,
leaving a freed pointer in efct->lookup.

v2 drops that predicate and gives the node an explicit target-session
state, read and written under efc->lock, so efct_scsi_del_initiator() and
the setup work agree on who owns the session. I have posted it as a v2 at
https://lore.kernel.org/r/20261021130802.2691833-1-yhlee@isslab.korea.ac.kr