linux-kernel - Re: [bug report] scsi: SATA devices missing after FLR is triggered during HBA suspended

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240701203903.GA16142@bhelgaas>
Date: Mon, 1 Jul 2024 15:39:03 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Damien Le Moal <dlemoal@...nel.org>
Cc: Yihang Li <liyihang9@...wei.com>, cassel@...nel.org,
	James.Bottomley@...senpartnership.com, martin.petersen@...cle.com,
	john.g.garry@...cle.com, yanaijie@...wei.com,
	linux-kernel@...r.kernel.org, linux-scsi@...r.kernel.org,
	linuxarm@...wei.com, chenxiang66@...ilicon.com,
	prime.zeng@...wei.com,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Alex Williamson <alex.williamson@...hat.com>
Subject: Re: [bug report] scsi: SATA devices missing after FLR is triggered
 during HBA suspended

[+cc Alex]

On Thu, Jun 27, 2024 at 09:56:02AM +0900, Damien Le Moal wrote:
> On 6/27/24 00:15, Bjorn Helgaas wrote:
> >>> Yes, I am talking about the PCI "Function Level Reset"
> >>>
> >>>> FLR and disk/controller suspend execution timing are unrelated.
> >>>> FLR can be triggered at any time through sysfs. So please give
> >>>> details here. Why is FLR done when the system is being
> >>>> suspended ?
> >>>
> >>> Yes, it is because FLR can be triggered at any time that we are
> >>> testing the reliability of executing FLR commands after
> >>> disk/controller suspended.
> >>
> >> "can be triggered" ? FLR is not a random asynchronous event. It
> >> is an action that is *issued* by a user with sys admin rights.
> >> And such users can do a lot of things that can break a machine...
> >>
> >> I fail to see the point of doing a function reset while the
> >> device is suspended. But granted, I guess the device should
> >> comeback up in such case, though I would like to hear what the
> >> PCI guys have to say about this.
> >>
> >> Bjorn,
> >>
> >> Is reseting a suspended PCI device something that should be/is
> >> supported ?
> > 
> > I doubt it.  The PCI core should be preserving all the generic PCI
> > state across suspend/resume.  The driver should only need to
> > save/restore device-specific things the PCI core doesn't know about.
> > 
> > A reset will clear out most state, and the driver doesn't know the
> > reset happened, so it will expect most device state to have been
> > preserved.
> 
> That is what I suspected. However, checking the code, reset_store() in
> pci-sysfs.c does:
> 
> 	pm_runtime_get_sync(dev);
> 	result = pci_reset_function(pdev);
> 	pm_runtime_put(dev);
> 
> and pm_runtime_get_sync() calls __pm_runtime_resume() which will
> resume a suspended device.
> 
> So while I still think it is not a good idea to reset a suspended
> device, things should still work as execpected and not cause any
> problem with the device state, right ?

The reset will clear almost all state, including both the generic PCI
part that pci_reset_function() saves/restores *and* any
device-specific state the PCI core doesn't know about.

That device-specific state isn't saved and restored anywhere in the
sysfs reset path, and the driver doesn't know this reset happened, so
I think all bets are off and we shouldn't expect the driver to work
afterwards.

A user-space reset might make sense if there's no driver bound to the
device, but I don't think it does if there is a driver (except maybe a
trivial stub driver that doesn't actually operate the device).

Bjorn