From: Daniel Gutson <daniel@eclypsium.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Tudor Ambarus <tudor.ambarus@microchip.com>,
Miquel Raynal <miquel.raynal@bootlin.com>,
Richard Weinberger <richard@nod.at>,
Vignesh Raghavendra <vigneshr@ti.com>,
Mika Westerberg <mika.westerberg@linux.intel.com>,
Boris Brezillon <bbrezillon@kernel.org>,
linux-mtd <linux-mtd@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Alex Bazhaniuk <alex@eclypsium.com>,
Richard Hughes <hughsient@gmail.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] mtd: spi-nor: intel-spi: Do not try to make the SPI flash chip writable
Date: Tue, 18 Aug 2020 12:55:59 -0300 [thread overview]
Message-ID: <CAFmMkTHqQO1Gj7VeXr4Y_Umb1KzZc2Pf=1pDQvPPpb_XeAFPqQ@mail.gmail.com> (raw)
In-Reply-To: <CAK8P3a2_RV33kiJ0c34aopZ4iG7LYBR-u=-+BbC+Upyjh1T0Eg@mail.gmail.com>
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
next prev parent reply other threads:[~2020-08-18 15:56 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-04 13:58 [PATCH] mtd: spi-nor: intel-spi: Do not try to make the SPI flash chip writable Daniel Gutson
2020-08-04 14:08 ` Mika Westerberg
2020-08-04 15:21 ` Arnd Bergmann
[not found] ` <CAFmMkTHEm8k+5GZkVJbDZMEhMwpsqVKRb-hGskSpBstdLRuFyA@mail.gmail.com>
2020-08-04 19:06 ` Arnd Bergmann
2020-08-04 19:57 ` Daniel Gutson
2020-08-04 20:46 ` Arnd Bergmann
2020-08-04 21:26 ` Daniel Gutson
2020-08-12 15:41 ` Daniel Gutson
2020-08-13 9:01 ` Greg Kroah-Hartman
[not found] ` <CAFmMkTFgjW+9gNfx=2SU7B0foww=SLiiyVi+P-hZpEFDbMTf2Q@mail.gmail.com>
2020-08-13 11:47 ` Greg Kroah-Hartman
2020-08-13 15:41 ` Arnd Bergmann
2020-08-13 21:40 ` Daniel Gutson
2020-08-16 8:41 ` Arnd Bergmann
2020-08-18 15:55 ` Daniel Gutson [this message]
2020-08-19 6:57 ` Mika Westerberg
2020-08-19 8:38 ` Arnd Bergmann
2020-08-19 9:11 ` Mika Westerberg
2020-08-22 16:06 ` Arnd Bergmann
2020-08-24 8:22 ` Mika Westerberg
2020-08-24 9:08 ` Arnd Bergmann
2020-08-24 9:15 ` Mika Westerberg
2020-08-24 9:31 ` Arnd Bergmann
2020-08-24 9:44 ` Mika Westerberg
2020-08-24 16:00 ` Daniel Gutson
2020-08-19 9:19 ` David Laight
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFmMkTHqQO1Gj7VeXr4Y_Umb1KzZc2Pf=1pDQvPPpb_XeAFPqQ@mail.gmail.com' \
--to=daniel@eclypsium.com \
--cc=alex@eclypsium.com \
--cc=arnd@arndb.de \
--cc=bbrezillon@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=hughsient@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=mika.westerberg@linux.intel.com \
--cc=miquel.raynal@bootlin.com \
--cc=richard@nod.at \
--cc=tudor.ambarus@microchip.com \
--cc=vigneshr@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).