linux-kernel - Re: [PATCH v2] PCI: Disable Samsung SM951/PM951 NVMe before FLR

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 1 Jul 2021 15:15:45 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     Robert Straw <drbawb@...alsyntax.com>, bhelgaas@...gle.com,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        alex.williamson@...hat.com
Subject: Re: [PATCH v2] PCI: Disable Samsung SM951/PM951 NVMe before FLR

On Thu, Jul 01, 2021 at 08:59:49PM +0100, Christoph Hellwig wrote:
> On Thu, Jul 01, 2021 at 02:38:56PM -0500, Bjorn Helgaas wrote:
> > On Fri, Apr 30, 2021 at 06:01:19PM -0500, Robert Straw wrote:
> > > The SM951/PM951, when used in conjunction with the vfio-pci driver and
> > > passed to a KVM guest, can exhibit the fatal state addressed by the
> > > existing `nvme_disable_and_flr` quirk. If the guest cleanly shuts down
> > > the SSD, and vfio-pci attempts an FLR to the device while it is in this
> > > state, the nvme driver will fail when it attempts to bind to the device
> > > after the FLR due to the frozen config area, e.g:
> > > 
> > >   nvme nvme2: frozen state error detected, reset controller
> > >   nvme nvme2: Removing after probe failure status: -12
> > > 
> > > By including this older model (Samsung 950 PRO) of the controller in the
> > > existing quirk: the device is able to be cleanly reset after being used
> > > by a KVM guest.
> > > 
> > > Signed-off-by: Robert Straw <drbawb@...alsyntax.com>
> > 
> > Applied to pci/virtualization for v5.14, thanks!
> 
> FYI, I really do not like the idea of the PCIe core messing with NVMe
> registers like this.

I hadn't looked at the nvme_disable_and_flr() implementation, but yes,
I see what you mean, that *is* ugly.  I dropped this patch for now.

I see that you suggested earlier that we not allow these devices to be
assigned via VFIO [1].  Is that practical?  Sounds like it could be
fairly punitive.

I assume this reset is normally used when vfio-pci is the driver in
the host kernel and there probably is no guest.  In that particular
case, I'd guess there's no conflict, but as you say, the sysfs reset
attribute could trigger this reset when there *is* a guest driver, so
there *would* be a conflict.

Could we coordinate this reset with vfio somehow so we only use
nvme_disable_and_flr() when there is no guest?

Bjorn

[1] https://lore.kernel.org/r/YKTP2GQkLz5jma/q@infradead.org