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

[Jason Yan <yanaijie@huawei.com>]

在 2026/6/23 10:57, 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?

Actually all libsas events are processed sequentially in sas_ha->disco_q 
workqueue. So this will not happen.

Thanks,
Jason

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