Re: [PATCH] scsi: pm8001: Fix data race in sysfs SAS address read

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Thu, 2026-01-15 at 09:24 -0800, Bart Van Assche wrote:
> On 1/15/26 9:11 AM, Chengfeng Ye wrote:
> > diff --git a/drivers/scsi/pm8001/pm8001_ctl.c
> > b/drivers/scsi/pm8001/pm8001_ctl.c
> > index cbfda8c04e95..e49f11969b3b 100644
> > --- a/drivers/scsi/pm8001/pm8001_ctl.c
> > +++ b/drivers/scsi/pm8001/pm8001_ctl.c
> > @@ -311,8 +311,15 @@ static ssize_t
> > pm8001_ctl_host_sas_address_show(struct device *cdev,
> >   	struct Scsi_Host *shost = class_to_shost(cdev);
> >   	struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost);
> >   	struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
> > -	return sysfs_emit(buf, "0x%016llx\n",
> > -			be64_to_cpu(*(__be64 *)pm8001_ha-
> > >sas_addr));
> > +	unsigned long flags;
> > +	ssize_t ret;
> > +
> > +	spin_lock_irqsave(&pm8001_ha->lock, flags);
> > +	ret = sysfs_emit(buf, "0x%016llx\n",
> > +			 be64_to_cpu(*(__be64 *)pm8001_ha-
> > >sas_addr));
> > +	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> > +
> > +	return ret;
> >   }
> >   static DEVICE_ATTR(host_sas_address, S_IRUGO,
> >   		   pm8001_ctl_host_sas_address_show, NULL);
> 
> Why isn't READ_ONCE() sufficient? And why explicit
> spin_lock_irqsave() and spin_unlock_irqrestore() calls instead of
> using scoped_guard()?

Do we really care?  The host sas address isn't something that changes
often, so even if it is being overwritten in an interrupt routine, it's
likely with the same value.  Whereas taking the internal host lock in a
sysfs read routine could potentially be a DoS vector.

Regards,

James