Message-ID: <CAHp75VdRwvU5WjFP5E4gg8U+_e34A0Lwze+nz_wVHoB49jLeLg@mail.gmail.com>
Date:   Wed, 10 Nov 2021 18:31:45 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Mauro Lima <mauro.lima@...ypsium.com>
Cc:     Hans-Gert Dahmen <hans-gert.dahmen@...u.ne>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Philipp Deppenwiese <philipp.deppenwiese@...u.ne>,
        Richard Hughes <hughsient@...il.com>,
        "platform-driver-x86@...r.kernel.org" 
        <platform-driver-x86@...r.kernel.org>
Subject: Re: [PATCH] firmware: export x86_64 platform flash bios region via sysfs

On Wed, Nov 10, 2021 at 3:13 PM Mauro Lima <mauro.lima@...ypsium.com> wrote:
>
> On Wed, Nov 10, 2021 at 7:00 AM Hans-Gert Dahmen
> <hans-gert.dahmen@...u.ne> wrote:
> >
> > Am Mi., 10. Nov. 2021 um 10:25 Uhr schrieb Andy Shevchenko
> > <andy.shevchenko@...il.com>:
> > >
> > > On Wed, Nov 10, 2021 at 11:17 AM Hans-Gert Dahmen
> > > <hans-gert.dahmen@...u.ne> wrote:
> > > > Am Mi., 10. Nov. 2021 um 10:05 Uhr schrieb Andy Shevchenko
> > > > <andy.shevchenko@...il.com>:
> > > > > On Wed, Nov 10, 2021 at 10:37 AM Hans-Gert Dahmen
> > > > > <hans-gert.dahmen@...u.ne> wrote:
> > > > > > Am Mi., 10. Nov. 2021 um 07:35 Uhr schrieb Andy Shevchenko
> > > > > > <andy.shevchenko@...il.com>:
> > > > > > > On Tuesday, November 9, 2021, Hans-Gert Dahmen <hans-gert.dahmen@...u.ne> wrote:
> > > > > > >> Am Di., 9. Nov. 2021 um 13:42 Uhr schrieb Greg KH <gregkh@...uxfoundation.org>:
> > > > >
> > > > > > >> > Do you have a pointer to these complex and buggy drivers anywhere?
> > > > > > >>
> > > > > > >> I am talking about the linux-mtd intel-spi driver for example, but I
> > > > > > >> feel that this gets the discussion in the wrong direction.
> > > > > > >
> > > > > > > This is the driver that covers all BIOSes on modern x86\64. What’s wrong with it? Why do you need this?!
> > > > > > >
> > > > > > > If it’s buggy, where is the bug reports from you or somebody else?
> > > > > >
> > > > > > Please see Mauro's mail in this thread from yesterday below:
> > > > >
> > > > > I didn't get this. What's wrong with the response from the author of
> > > > > intel-spi (since we are speaking about it, let me add him to the
> > > > > thread)?
> > > > > What you are trying to do is to sneak around with ugly hack behind the
> > > > > proper subsystem's back.
> > > > >
> > > > > NAK as I already said.
> > > >
> > > > There is nothing wrong with the response. Also we are not trying to
> > > > sneak anything into the kernel. This just is no mtd or spi driver, it
> > > > is not even a driver. What this does is it opens back up a portion of
> > > > memory that can not be read anymore through /dev/mem on locked down
> > > > systems with SecureBoot enabled. This portion of memory is actively
> > > > being used by userspace programs. We, 9elements, Eclypsium, fwudp and
> > > > immune are among those who rely upon this to improve the security of
> > > > x86_64 linux devices.
> > >
> > > I appreciate this.
> > >
> > > x86_64 starting from long time ago has more or less standard hardware
> > > interface for BIOS chip, i.e. SPI NOR. PCH usually has a separate host
> > > controller and we have the driver in the kernel for that (coverage of
> > > the systems may not be 100%, but close enough). Now you are trying to
> > > repeat something that is _already_ provided by the kernel. Correct?
> >
> > Yes and no. We are not repeating the functionality of the SPI driver
> > because we don't read nor write the flash specifically. What we do, is
> > we make the boot vector readable by userspace when it is not
> > accessible by /dev/mem. It happens to be a portion of the SPI flash
> > but that doesn't have to be the case because the required view upon io
> > memory here is CPU-centric and the rest of the system could be built
> > in a way that backs that memory window differently. However, this is
> > more or less hypothetical if you ignore some probably never-used bits
> > that can be set in PCH registers. As I said, effectively we are trying
> > to partially revert the lockdown on /dev/mem to be able to do a
> > harmless read on an important memory range. From that point of view we
> > are trying to keep functionality intact. The interface being used here
> > has not changed a bit since 15 years and it probably will stay that
> > way. In contrast to the intel-spi driver this will likely cover more
> > future and past systems in different use-cases with fewer lines of
> > code and next to no maintenance.
> >
> > >
> > > > Now what happens is that more distros start to
> > > > lock down their kernels and require signed modules. With the mtd
> > > > driver not working on those machine to read the bios binary, you are
> > > > effectively forcing users into less secure system configurations (i.e.
> > > > opening up the whole /dev/mem and/or disabling SecureBoot) just to be
> > > > able to run fwupd or other tools to assess firmware information. The
> > > > issue of being able to assess and compare the bios binary to the one
> > > > publicly available from the vendors is increasingly getting important
> > > > in the wake of recent CVEs about supply-chain attacks where UEFI
> > > > malware was pre-installed. So we are not even doing anything new here,
> > > > you are just making life harder for everybody.
> > >
> > > Why me? As far as I got it from above the bottleneck is the distros
> > > which do not enable the driver. So why not go and work with them?
> > >
> >
> > Oh come on,  the distros have no choice here. Either not provide a
> > more secure locked down system or allow a dangerous driver where
> > patches to make it less dangerous are not welcome. And even if the
> > latter could be solved, the history clearly is that it is in no way
> > not even remotely as reliable as the memory region my patch is
> > relaying to userspace.
>
> As Hans already said, it is a no go to put drivers tagged as
> _(DANGEROUS)_ within the kernel distro. In this thread [1] from the
> latest changes to the intel-spi driver I asked if there is any reason
> to keep the dangerous tag or if there is any work we can do to achieve
> this. As long as this tag is in the driver, we can't use the driver.
>
> [1] https://lore.kernel.org/all/CAArk9MPWh4f1E=ecKBHy8PFzvU_y_ROgDyUU_O3ZQ0FuMhkj5Q@mail.gmail.com/

>From [1]:
  "> IMHO we can make certain parts of the driver, that are known not to
  > cause any issues available without the DANGEROUS. I mean the controller
  > exposes some information that I think you are intersted in and that does
  > not cause anything to be sent to the flash chip itself.
  This will work for me."

And?.. Where is your proposal to modify the driver the way maintainer
and you will be happy? Why do we see this instead? Try again to
collaborate with Intel SPI driver author(s) / maintainer(s) by sending
the proposal that may work.

-- 
With Best Regards,
Andy Shevchenko

Powered by blists - more mailing lists