lkml.org 
[lkml]   [2020]   [Aug]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] mtd: spi-nor: intel-spi: Do not try to make the SPI flash chip writable
    ping

    On Tue, Aug 4, 2020 at 6:26 PM Daniel Gutson <daniel@eclypsium.com> wrote:
    >
    > On Tue, Aug 4, 2020 at 5:46 PM Arnd Bergmann <arnd@arndb.de> wrote:
    > >
    > > On Tue, Aug 4, 2020 at 9:57 PM Daniel Gutson <daniel@eclypsium.com> wrote:
    > > > On Tue, Aug 4, 2020 at 4:06 PM Arnd Bergmann <arnd@arndb.de> wrote:
    > > > > On Tue, Aug 4, 2020 at 5:49 PM Daniel Gutson <daniel@eclypsium.com> wrote:
    > > > > > On Tue, Aug 4, 2020 at 12:21 PM Arnd Bergmann <arnd@arndb.de> wrote:
    > > > > >> On Tue, Aug 4, 2020 at 3:58 PM Daniel Gutson
    > > > > >> <daniel.gutson@eclypsium.com> wrote:
    > > > > >
    > > > > > What about just saying
    > > > > >
    > > > > > "This patch removes the attempt by the intel-spi-pci driver to
    > > > > > make the chip always writable."
    > > > >
    > > > > Yes, that is much better, though it still sounds like it would at the
    > > > > moment allow writing to the device from software without also
    > > > > setting the module parameter. I would say something like
    > > > >
    > > > > "Disallow overriding the write protection in the PCI driver
    > > > > with a module parameter and instead honor the current
    > > > > state of the write protection as set by the firmware."
    > > >
    > > > But wait, Mika, the author of the file, asked earlier not to remove
    > > > the module parameter of intel-spi, and just remove the unconditional
    > > > attempt to turn the chip writable in intle-spi-pci.
    > >
    > > Yes, and I think that is fine (aside from the inconsistency with bay trail
    > > that you have not commented on),
    >
    > There are two inconsistencies before any of my patches:
    > 1) in intel-spi.c: uses the module parameter only for bay trail.
    > 2) intel-spi.c uses a module parameter whereas intel-spi-pci doesn't
    >
    > My initial patch addressed #2 by also adding a module parameter to
    > intel-spi-pci,
    > but then some of you discouraged me to use module parameters.
    > Mika showed up and suggested to leave intel-spi.c as is (with its
    > module parameter),
    > and remove the code in intel-spi-pci that tried to turn the SPI chip
    > writable if the BIOS
    > was unlocked.
    >
    > > but that only touches the hardware
    > > write-protection, which doesn't really have any effect unless user
    > > space also configures the driver module to allow writing to the
    > > mtd device.
    > >
    > > > So I'm not touching intel-pci, just removing that code from
    > > > intel-spi-pci without adding a new module parameter.
    > > >
    > > > Are you aligned on this?
    > >
    > > One of us is still very confused about what the driver does.
    > > You seem to have gone back to saying that without the
    > > change a user could just write to the device even without
    > > passing the module parameter to intel-spi.ko?
    >
    > What I'm trying to say is that, if the BIOS is unlocked
    > (no driver involvement here), the intel-spi-pci turns the
    > chip writable even without changing the module parameter of intel-spi.
    > This is because the attempt to turn the chip writable occurs in
    > the probing of intel-spi-pci, that is, earlier than the intel-spi
    > initialization.
    >
    > >
    > > Maybe you should start by explaining what scenario you
    > > actually want to prevent here. Is it
    >
    > Was it clear from above?
    >
    > Before commenting below, it's important to note again that
    > the driver will succeed in turning the chip writable only if the
    > BIOS is unlocked by its build time specification.
    > The WPD field (Write Protect Disable) bit only has effect if
    > the BIOS is not locked. This WPD bit is the one that the intel-spi-pci
    > driver tries to set unconditionally. If the BIOS is locked, it will cause
    > no effect. But if the BIOS is not locked, the chip will
    > end up in Write Protect Disabled state.
    > My latest patch simply leaves alone the WPD bit in intel-spi-pci,
    > not trying to set it to 1.
    >
    > I'm not sure the options below are now fully compatible
    > with my explanation above.
    >
    > >
    > > a) the hardware write-protect bit getting changed, which
    > > introduces the possibility of corrupting the flash even
    > > if nothing tries to write to it,
    > >
    > > b) root users setting the device writable with the intention
    > > of writing to it even though firmware has politely asked
    > > for this not to be done (by setting the write-protect bit
    > > but not preventing it from being disabled again), or
    > >
    > > c) a writeable mtd device showing up even without
    > > the module parameter being set at all?
    > >
    > > I thought the initial patch was about c) which turned out
    > > to be a non-issue, and then the later patch being about b).
    > >
    > > Arnd
    >
    >
    >
    > --
    > Daniel Gutson
    > Argentina Site Director
    > Enginieering Director
    > Eclypsium
    >
    > Below The Surface: Get the latest threat research and insights on
    > firmware and supply chain threats from the research team at Eclypsium.
    > https://eclypsium.com/research/#threatreport



    --
    Daniel Gutson
    Argentina Site Director
    Enginieering Director
    Eclypsium

    Below The Surface: Get the latest threat research and insights on
    firmware and supply chain threats from the research team at Eclypsium.
    https://eclypsium.com/research/#threatreport

    \
     
     \ /
      Last update: 2020-08-12 17:43    [W:4.951 / U:0.064 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site