Re: [PATCH] mtd: spi-nor: intel-spi: Do not try to make the SPI flash chip writable

[Arnd Bergmann]

On Wed, Aug 19, 2020 at 8:57 AM Mika Westerberg
<mika.westerberg@xxxxxxxxxxxxxxx> wrote:
>
> On Tue, Aug 18, 2020 at 12:55:59PM -0300, Daniel Gutson wrote:
> > > If you care about other (malicious) code writing to the driver, please explain
> > > what the specific attack scenario is that you are worried about, and
> > > why you think
> > > this is not sufficient. What code would be able to write to the device
> > > if not the
> > > device driver itself?
> >
> > Maybe Mika can answer this better, but what I'm trying to do is to
> > limit the possibility of
> > damage, as explained in the Kconfig:
> > "Intel PCH/PCU SPI flash PCI driver (DANGEROUS)"
> > "Say N here unless you know what you are doing. Overwriting the
> >   SPI flash may render the system unbootable."
>
> Right, the PCI part of the driver unconditionally (and wrongly) tried to
> set the chip writeable.
>
> What this whole thing tries to protect is that the user does not
> accidentally write to the flash chip. It contains BIOS and other
> important firmware so touching it (if it is not locked in the BIOS side)
> may potentially brick the system. That's why we also require that
> command line parameter so the user who knows what he or she is doing can
> enable it for writing.

The same thing can happen with the platform driver if you load it
once with 'writeable=1' and then unload, leaving the chip in writeable
state. If you load it a second time without the module parameter, it
will be in the same state as the PCI driver: the hardware bit allows
writing, but the MTD layer prevents writes from being issued to the
device.

> Actually thinking about this bit more, to make PCI and the platform
> parts consistent we can make the "writeable" control this for the PCI
> part as well. So what if we add a callback to struct intel_spi_boardinfo
> that the PCI driver populates and then the "core" driver uses to enable
> writing when "writeable" is set to 1.

If you are really worried about the write protection being bypassed by
a different driver or code injection, the best way would seem to be to
only enable writing in the mtd write callback and disable it immediately
after the write is complete. I still don't see why this hardware would
be more susceptible to this kind of attack than other drivers though,
as it already has the safeguard against writing through the MTD layer
without the module parameter.

       Arnd