Messages in this thread | | | From | Daniel Gutson <> | Date | Tue, 18 Aug 2020 12:55:59 -0300 | Subject | Re: [PATCH] mtd: spi-nor: intel-spi: Do not try to make the SPI flash chip writable |
| |
On Sun, Aug 16, 2020 at 5:42 AM Arnd Bergmann <arnd@arndb.de> wrote: > > On Thu, Aug 13, 2020 at 11:40 PM Daniel Gutson <daniel@eclypsium.com> wrote: > > > > On Thu, Aug 13, 2020 at 12:41 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > On Tue, Aug 4, 2020 at 11:26 PM Daniel Gutson <daniel@eclypsium.com> wrote: > > > > On Tue, Aug 4, 2020 at 5:46 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > 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 > > > > > > Neither of these matches what I see in the source code. Please > > > check again. > > > > > > Once more: intel-spi.c has a module parameter that controls writing > > > to the device regardless of the back-end (platform or pci), purely > > > in software. > > > > If I understand you correctly, this is not what I see: > > If the deviceID is listed in intel-spi-pci.c > > (https://elixir.bootlin.com/linux/latest/source/drivers/mtd/spi-nor/controllers/intel-spi-pci.c#L66) > > then intel_spi_pci_probe will be called, where it unconditionally will > > try to make the chip writable > > https://elixir.bootlin.com/linux/latest/source/drivers/mtd/spi-nor/controllers/intel-spi-pci.c#L44 > > These devices correspond to the BXT and CNL devices (lines 19 and 23 resp.). > > > > Lines later (53), it will call intel-spi.c 's intel_spi_probe > > function, which ends up calling intel_spi_init, > > which checks for the type > > (https://elixir.bootlin.com/linux/latest/source/drivers/mtd/spi-nor/controllers/intel-spi.c#L313) > > It is in this switch where the module parameter is checked, but only > > in the BYT case; however, > > flow coming from intel-spi-pci is BXT and CNL as mentioned before, > > landing in their case labels (lines 343 and 351 respectively) > > where the module parameter is not checked. > > > > Therefore, for BXT and CNL probed in intel-spi-pci, the chip is turned > > writable and later the module parameter is not honored. > > The module parameter is definitely honored on all hardware, but the driver > only cares about the functionality it provides through the mtd interface: > > https://elixir.bootlin.com/linux/latest/source/drivers/mtd/spi-nor/controllers/intel-spi.c#L941
That is a logical constraint which doesn't impact in the hardware, which already was changed before in https://elixir.bootlin.com/linux/latest/source/drivers/mtd/spi-nor/controllers/intel-spi.c#L924
> > 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."
> > 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
| |