Re: [PATCH v8 2/2] scsi: libsas: Add linkrate and sas_addr change detection in rediscover

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] `sas_dev_is_flutter()` improperly bypasses PHY discovery when `child_dev` is NULL, failing to update the libsas routing table and PHY state.

Pre-existing issues:
- [High] Attempting to prevent UAF by acquiring a reference via `kref_get(&child_dev->kref)` results in a TOCTOU race because `sas_find_dev_by_rphy()` returns a raw pointer without holding a reference.
--

commit 4a9be29a10948d7f2b0efa1425b3ea27e6c08b1f
Author: Xingui Yang <yangxingui@huawei.com>

scsi: libsas: Add linkrate and sas_addr change detection in rediscover

This commit introduces sas_dev_is_flutter() and sas_rediscover_ex_phy()
to improve flutter and device replace detection during rediscovery.
It holds a kref on the child device during SMP discovery and uses async
discovery to avoid ordering issues.

> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index cb9d3b7482225..63d033e789855 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
[ ... ]
> @@ -1966,6 +1966,78 @@ static bool dev_type_flutter(enum sas_device_type new, enum sas_device_type old)
[ ... ]
> +static bool sas_dev_is_flutter(struct domain_device *dev, int phy_id,
> +			       u8 *sas_addr, enum sas_device_type type)
> +{
> +	struct expander_device *ex = &dev->ex_dev;
> +	struct ex_phy *phy = &ex->ex_phy[phy_id];
> +	struct domain_device *child_dev = NULL;
> +	char *action = "";
> +	int res;
> +
> +	if (SAS_ADDR(sas_addr) != SAS_ADDR(phy->attached_sas_addr) ||
> +	    !dev_type_flutter(type, phy->attached_dev_type))
> +		return false;
> +
> +	child_dev = sas_ex_to_dev(dev, phy_id);
> +	if (!child_dev)
> +		goto out;

[Severity: Medium]
In the original code, sas_ex_phy_discover() was called unconditionally.
Will this new early return bypass the SMP request that updates the PHY state
(via sas_set_ex_phy()) when the child device is not instantiated or has been
removed?

> +
> +	kref_get(&child_dev->kref);

[Severity: High]
This is a pre-existing issue, but could attempting to prevent a use-after-free
by acquiring a reference here result in a time-of-check to time-of-use
(TOCTOU) race?

Looking at sas_find_dev_by_rphy() (called by sas_ex_to_dev()), it acquires
port->dev_list_lock, finds the device, and drops the lock before returning
the raw pointer:

drivers/scsi/libsas/sas_scsi_host.c:sas_find_dev_by_rphy() {
    ...
    found_dev = dev;
    spin_unlock(&port->dev_list_lock);
    goto found;
    ...
}

If a concurrent link-loss or device removal event runs, can it acquire the
lock, remove the device, and drop its last reference before we call
kref_get(&child_dev->kref) here, leading to a use-after-free?

> +	res = sas_ex_phy_discover(dev, phy_id);
> +	if (res)
> +		goto out_put;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623024304.714582-1-yangxingui@huawei.com?part=2