[PATCH] scsi: target: use kref_get_unless_zero() in core_get_se_deve_from_rtpi()

[PATCH] scsi: target: use kref_get_unless_zero() in core_get_se_deve_from_rtpi()

[Yifei Gao]

core_get_se_deve_from_rtpi() iterates nacl->lun_entry_hlist under
rcu_read_lock() and takes a plain kref_get() on deve->pr_kref for the
matching entry. The disable path, core_disable_device_list_for_node(),
unhashes the entry, drops the final reference, waits for pr_comp and
frees it via call_rcu() while holding lun_entry_mutex, which the reader
does not hold. A reader racing that path can revive the kref after it has
reached zero, defeating the pr_comp completion barrier and leading to a
use-after-free.

Use kref_get_unless_zero() and skip entries whose refcount has already
dropped to zero.

Fixes: 29a05deebf6c ("target: Convert se_node_acl->device_list[] to RCU hlist")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Yifei Gao <gyf161023@gmail.com>
---
 drivers/target/target_core_device.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 9db2201aa553..bff97c6efffb 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -220,7 +220,8 @@ struct se_dev_entry *core_get_se_deve_from_rtpi(
 		if (lun->lun_tpg->tpg_rtpi != rtpi)
 			continue;
 
-		kref_get(&deve->pr_kref);
+		if (!kref_get_unless_zero(&deve->pr_kref))
+			continue;
 		rcu_read_unlock();
 
 		return deve;
-- 
2.43.0

Re: [PATCH] scsi: target: use kref_get_unless_zero() in core_get_se_deve_from_rtpi()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch addresses a kref revival race in `core_get_se_deve_from_rtpi()`, but identically flawed `kref_get()` patterns remain in `__core_scsi3_do_alloc_registration()` and `core_scsi3_check_aptpl_registration()`.
- [High] Fetching `tfc_sess` under RCU using a plain `kref_get()` in `ft_sess_get()` can revive a 0-refcount object, leading to a double free.
--

--- Patch [1]: [PATCH] scsi: target: use kref_get_unless_zero() in core_get_se_deve_from_rtpi() ---
commit 1ccb09531fbaa5b4a4d4c2e984295bc9d5b45e8e
Author: Yifei Gao <gyf161023@gmail.com>

scsi: target: use kref_get_unless_zero() in core_get_se_deve_from_rtpi()

This commit fixes a use-after-free vulnerability in core_get_se_deve_from_rtpi().
It replaces a plain kref_get() with kref_get_unless_zero() when acquiring a
reference to se_dev_entry under RCU. This prevents the revival of a 0-refcount
object during a concurrent teardown.

> diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
> index 9db2201aa553f..bff97c6efffbe 100644
> --- a/drivers/target/target_core_device.c
> +++ b/drivers/target/target_core_device.c
> @@ -220,7 +220,8 @@ struct se_dev_entry *core_get_se_deve_from_rtpi(
>  		if (lun->lun_tpg->tpg_rtpi != rtpi)
>  			continue;
>  
> -		kref_get(&deve->pr_kref);
> +		if (!kref_get_unless_zero(&deve->pr_kref))
> +			continue;

[Severity: High]
This isn't a bug introduced by this patch, but since this addresses a kref 
revival race, could the same flaw exist in other places doing lockless RCU
lookups of se_dev_entry?

For example, drivers/target/target_core_pr.c:__core_scsi3_do_alloc_registration()
does:

		kref_get(&pr_reg->pr_reg_deve->pr_kref);
		rcu_read_unlock();

And similarly in drivers/target/target_core_pr.c:core_scsi3_check_aptpl_registration():

			kref_get(&pr_reg->pr_reg_deve->pr_kref);
			rcu_read_unlock();

If a concurrent disable path unhashes the entry and drops its final reference,
could a delayed reader executing the plain kref_get() resurrect the dying
object and cause a use-after-free when the disable path proceeds to free it
via call_rcu()?

[Severity: High]
This is another pre-existing issue, but while looking at plain kref_get()
usage under RCU, could a similar double-free issue occur in
drivers/target/tcm_fc/tfc_sess.c:ft_sess_get()?

			kref_get(&sess->kref);
			rcu_read_unlock();

If a teardown path (like handling a PRLO) calls ft_sess_delete() to unhash
the session and drops the final reference concurrently, could a reader
executing this plain kref_get() resurrect the session?

When the reader later drops its reference, would this lead to 
target_remove_session() and kfree_rcu() executing a second time?

>  		rcu_read_unlock();
>  
>  		return deve;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804213726.3411698-1-gyf161023@gmail.com?part=1