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

[yangxingui <yangxingui@huawei.com>]

Hi Jason,

Thanks for the review.

On 2026/6/23 16:52, Jason Yan wrote:
> 在 2026/6/23 10:43, Xingui Yang 写道:
>> Introduce sas_dev_is_flutter() and sas_rediscover_ex_phy() to improve
>> flutter and device replace detection during rediscovery.
>>
>> sas_dev_is_flutter() adds validation for linkrate and sas_addr changes.
>> When the SAS address changes, it restores phy->attached_sas_addr back to
>> the original address before returning false, ensuring
>> sas_unregister_devs_sas_addr() can properly match and unregister the old
>> device via sas_phy_match_dev_addr().
>>
>> The sas_addr check is ordered before the linkrate check to ensure the
>> address restoration is not skipped when both change simultaneously.
>>
>> Hold a kref on child_dev across the sas_ex_phy_discover() call to
>> prevent use-after-free, since sas_ex_phy_discover() sends an SMP
>> request which can sleep, during which the device could be freed by
>> a concurrent removal path.
>>
>> sas_rediscover_ex_phy() uses the async discovery pattern
>> (sas_discover_event) instead of the synchronous sas_discover_new() to
>> ensure proper ordering between device unregistration and rediscovery,
>> avoiding sysfs_warn_dup() errors.
>>
>> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
>> ---
>>   drivers/scsi/libsas/sas_expander.c | 89 +++++++++++++++++++++++++-----
>>   1 file changed, 75 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/scsi/libsas/sas_expander.c 
>> b/drivers/scsi/libsas/sas_expander.c
>> index cb9d3b748222..63d033e78985 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)
>>       return false;
>>   }
>> +static void sas_rediscover_ex_phy(struct domain_device *dev, int phy_id,
>> +                  bool last)
>> +{
>> +    struct expander_device *ex = &dev->ex_dev;
>> +    struct ex_phy *phy = &ex->ex_phy[phy_id];
>> +
>> +    phy->phy_change_count = -1;
>> +    ex->ex_change_count = -1;
>> +    sas_unregister_devs_sas_addr(dev, phy_id, last);
>> +    sas_discover_event(dev->port, DISCE_REVALIDATE_DOMAIN);
>> +}
>> +
>> +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;
>> +
>> +    kref_get(&child_dev->kref);
> 
> This is not necessary so I think you can remove it as domain device will 
> never release here sine we are in the discover workqueue process.
> It's ture that sas_find_dev_by_rphy() is not perfect. It shall get a 
> reference after lock ->dev_list_lock. But this will affect many existing 
> users. We can do that in another patchset.

Agreed. In v9 I have removed the kref_get/sas_put_device pattern
entirely. Instead, sas_ex_phy_discover() is now called before
sas_ex_to_dev(), so the child device pointer is obtained after the
sleeping SMP request completes. This eliminates the UAF concern
without needing a kref, since we are serialized by disco_mutex in
the discover workqueue.

The sas_find_dev_by_rphy() reference counting improvement is noted
as a separate patchset as you suggested.

Thanks,
Xingui