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

[yangxingui]

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@xxxxxxxxxx>
> > ---
> >   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