* [PATCH v2] scsi: target: fix demo-mode dynamic ACL dev-entry UAF vs LUN teardown
[not found] <20261013120528.37749-1-yuz08559@gmail.com>
@ 2026-10-14 1:37 ` Yu Zhang
2026-10-29 1:42 ` Martin K. Petersen
2026-10-31 1:40 ` Martin K. Petersen (Oracle)
0 siblings, 2 replies; 3+ messages in thread
From: Yu Zhang @ 2026-10-14 1:37 UTC (permalink / raw)
To: Martin K . Petersen
Cc: linux-scsi, target-devel, linux-kernel, Mike Christie,
Dmitry Bogdanov, stable, Yu Zhang
A demo-mode (generate_node_acls=1) login that allocates a dynamic
NodeACL populates that ACL's per-LUN dev entries in
core_tpg_add_node_to_devs() *before* the ACL is linked onto
tpg->acl_node_list by target_add_node_acl():
core_tpg_check_initiator_node_acl():
...
core_tpg_add_node_to_devs(acl, tpg, NULL); /* builds deves */
target_add_node_acl(acl); /* only now listed */
The LUN teardown sweep, core_clear_lun_from_tpg(), enumerates only
tpg->acl_node_list, so a dev entry whose ACL is still in the window
above is invisible to the sweep and is never disabled. It is left on
the se_lun->lun_deve_list after core_tpg_remove_lun() frees the se_lun;
when the same unpacked_lun is later re-added and the se_lun is
reallocated at the same address, the stale dev entry is rediscovered by
target_nacl_find_deve() and core_enable_device_list_for_node() corrupts
lun_deve_list:
BUG: KASAN: slab-use-after-free ...
core_enable_device_list_for_node+0x5ce/0x880
core_tpg_add_node_to_devs+0x169/0x190
core_dev_add_lun+0x146/0x190
target_fabric_port_link+0xe9/0x1b0
configfs_symlink+0x381/0xae0
__x64_sys_symlink+0x55/0x80
The demo-mode login path that drives the ACL/dev-entry creation is
reachable by an unauthenticated initiator; the colliding LUN add/remove
is a local configfs operation.
Commit 49cb77e297dc ("target: Avoid mappedlun symlink creation during
lun shutdown") introduced se_lun->lun_shutdown and guarded the
*explicit* ACL MappedLUN path (target_fabric_mappedlun_link()), but the
demo-mode automatic path, core_tpg_add_node_to_devs() ->
core_enable_device_list_for_node(), was never given the equivalent
guard, and the publish-after-populate ordering in
core_tpg_check_initiator_node_acl() was left unchanged.
Fix it in three cooperating parts:
1) Publish the dynamic NodeACL onto acl_node_list *before* creating its
dev entries, so the teardown sweep can observe them.
2) Refuse to attach a dev entry to an se_lun that is shutting down,
checked under nacl->lun_entry_mutex -- the same lock the sweep takes
-- so once the sweep has passed an ACL any later enable for that LUN
observes lun_shutdown and skips it.
3) In core_enable_device_list_for_node(), insert the new dev entry onto
lun->lun_deve_list while still holding nacl->lun_entry_mutex, in both
the replace and the new-entry branch. Previously the entry was added
to nacl->lun_entry_hlist under the mutex but to lun->lun_deve_list
only after dropping it; with the ACL now published early, the sweep
could take the mutex in that gap, find the entry on the hlist,
core_disable_device_list_for_node() it (list_del() on the not-yet-
linked lun_link succeeds and the entry is freed), and then the enable
would list_add_tail() the freed entry back onto lun_deve_list.
Linking both lists under the mutex makes enable and the sweep
mutually exclusive, closing that window.
core_tpg_add_node_to_devs() already ignores the return value of
core_enable_device_list_for_node(), so a shutting-down LUN is simply
skipped.
Cc: stable@vger.kernel.org
Fixes: 49cb77e297dc ("target: Avoid mappedlun symlink creation during lun shutdown")
Signed-off-by: Yu Zhang <yuz08559@gmail.com>
Tested-by: Yu Zhang <yuz08559@gmail.com>
---
v2: core_enable_device_list_for_node() now links the new dev entry onto
lun->lun_deve_list while still holding nacl->lun_entry_mutex (part 3
above), closing a TOCTOU window the v1 reorder opened where the
teardown sweep could free a dev entry between its hlist insert and
its lun_deve_list insert. Caught by automated review. No functional
change to the v1 tpg.c reorder or the lun_shutdown guard.
drivers/target/target_core_device.c | 18 +++++++++++-------
drivers/target/target_core_tpg.c | 10 +++++++---
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 1111111..2222222 100644
@@ -349,6 +349,19 @@
new->attach_count++;
mutex_lock(&nacl->lun_entry_mutex);
+ /*
+ * Do not attach a new dev entry to an se_lun that is being torn down.
+ * lun_shutdown is set by core_tpg_remove_lun() before its
+ * core_clear_lun_from_tpg() sweep; checking it here under the same
+ * nacl->lun_entry_mutex the sweep takes guarantees that once the sweep
+ * has passed this ACL no dev entry can be left dangling on the
+ * se_lun->lun_deve_list that is about to be freed.
+ */
+ if (lun->lun_shutdown) {
+ mutex_unlock(&nacl->lun_entry_mutex);
+ ret = -ENODEV;
+ goto free_stats;
+ }
orig = target_nacl_find_deve(nacl, mapped_lun);
if (orig && orig->se_lun) {
struct se_lun *orig_lun = orig->se_lun;
@@ -375,12 +388,12 @@
new->se_lun_acl = lun_acl;
hlist_del_rcu(&orig->link);
hlist_add_head_rcu(&new->link, &nacl->lun_entry_hlist);
- mutex_unlock(&nacl->lun_entry_mutex);
spin_lock(&lun->lun_deve_lock);
list_del(&orig->lun_link);
list_add_tail(&new->lun_link, &lun->lun_deve_list);
spin_unlock(&lun->lun_deve_lock);
+ mutex_unlock(&nacl->lun_entry_mutex);
kref_put(&orig->pr_kref, target_pr_kref_release);
wait_for_completion(&orig->pr_comp);
@@ -393,11 +406,10 @@
new->se_lun = lun;
new->se_lun_acl = lun_acl;
hlist_add_head_rcu(&new->link, &nacl->lun_entry_hlist);
- mutex_unlock(&nacl->lun_entry_mutex);
-
spin_lock(&lun->lun_deve_lock);
list_add_tail(&new->lun_link, &lun->lun_deve_list);
spin_unlock(&lun->lun_deve_lock);
+ mutex_unlock(&nacl->lun_entry_mutex);
target_luns_data_has_changed(nacl, new, true);
return 0;
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 3333333..4444444 100644
@@ -267,6 +267,15 @@
acl->dynamic_node_acl = 1;
/*
+ * Publish the dynamic NodeACL on tpg->acl_node_list before creating
+ * its demo-mode MappedLUN dev entries below, so that a concurrent LUN
+ * teardown sweep (core_clear_lun_from_tpg(), which walks acl_node_list)
+ * can observe and disable those dev entries. Otherwise a dev entry can
+ * be left on an se_lun->lun_deve_list that is about to be freed.
+ */
+ target_add_node_acl(acl);
+
+ /*
* Here we only create demo-mode MappedLUNs from the active
* TPG LUNs if the fabric is not explicitly asking for
* tpg_check_demo_mode_login_only() == 1.
@@ -275,7 +284,6 @@
(tpg->se_tpg_tfo->tpg_check_demo_mode_login_only(tpg) != 1))
core_tpg_add_node_to_devs(acl, tpg, NULL);
- target_add_node_acl(acl);
return acl;
}
EXPORT_SYMBOL(core_tpg_check_initiator_node_acl);
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread