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

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_scs=
i3_do_alloc_registration()` and `core_scsi3_check_aptpl_registration()`.
- [High] Fetching `tfc_sess` under RCU using a plain `kref_get()` in `ft_se=
ss_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 <[email protected]>

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_r=
tpi().
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-refco=
unt
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 !=3D rtpi)
>  			continue;
> =20
> -		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=
=20
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_registra=
tion()
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_reg=
istration():

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

If a concurrent disable path unhashes the entry and drops its final referen=
ce,
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=20
target_remove_session() and kfree_rcu() executing a second time?

>  		rcu_read_unlock();
> =20
>  		return deve;

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804213726.3411=
[email protected]?part=3D1