RE: [PATCH v7 10/15] platform/x86: dell-smbios: add filtering capability for requests

[Mario.Limonciello]

> -----Original Message-----
> From: Greg KH [mailto:greg@xxxxxxxxx]
> Sent: Friday, October 13, 2017 4:43 AM
> To: Darren Hart <dvhart@xxxxxxxxxxxxx>
> Cc: Alan Cox <gnomes@xxxxxxxxxxxxxxxxxxx>; Limonciello, Mario
> <Mario_Limonciello@xxxxxxxx>; Andy Shevchenko
> <andy.shevchenko@xxxxxxxxx>; LKML <linux-kernel@xxxxxxxxxxxxxxx>;
> platform-driver-x86@xxxxxxxxxxxxxxx; Andy Lutomirski <luto@xxxxxxxxxx>;
> quasisec@xxxxxxxxxx; pali.rohar@xxxxxxxxx; rjw@xxxxxxxxxxxxx;
> mjg59@xxxxxxxxxx; hch@xxxxxx
> Subject: Re: [PATCH v7 10/15] platform/x86: dell-smbios: add filtering capability
> for requests
> 
> On Thu, Oct 12, 2017 at 05:46:45PM -0700, Darren Hart wrote:
> > On Thu, Oct 12, 2017 at 11:09:03AM +0100, One Thousand Gnomes wrote:
> > > On Wed, 11 Oct 2017 11:27:36 -0500
> > > Mario Limonciello <mario.limonciello@xxxxxxxx> wrote:
> > >
> > > > There are some categories of tokens and SMBIOS calls that it makes
> > > > sense to protect userspace from accessing.  These are calls that
> > > > may write to one time use fields or activate hardware debugging
> > > > capabilities.  They are not intended for general purpose use.
> > > >
> > > > This same functionality may be be later extended to also intercept
> > > > calls that may cause kernel functionality to get out of sync if
> > > > the same functions are used by other drivers.
> > >
> > > This doesn't work. You are creating an API. If you then have to remove
> > > parts of the API because it messes stuff up you break your API guarantee.
> > >
> > > As I said before, this needs to be a whitelist of stuff that is safe, and
> > > it needs to have a security model. When you make a feature available you
> > > can't nicely take it away again as you'll break people's user space.
> > >
> > > Start with a whitelist of the ones you know people want to use, or that
> > > your existing tooling you want to enable uses. Add others as needed in
> > > future releases of the kernel.
> >
> > Hi Alan,
> >
> > I've attempted to get a focused discussion on this topic a few times,
> > but have failed to get it the focus in really needs. Thanks for bringing
> > it up. I look forward to your thoughts on the following:
> >
> > The core of the issue is we are trying to achieve feature parity with
> > Windows using the Windows Management Instrumentation (WMI) mechanism.
> > I'm trying to support vendors like Dell in their attempts to do so, and
> > acknowledging that this involves supporting a platform which was
> > designed according to the norms and standards of the windows ecosystem.
> 
> Note, we really don't care what Windows does, as I really doubt they
> care what we do, so this isn't a valid decision.  Do we also care about
> OS-X and how it handles WMI?  :)

If you want to be able to offer tools that do the same thing on all OSes that
hardware ships with it's a good idea to do it through the same interface in
both places and to use the same code base on both if possible.  Otherwise
that's just asking for bugs.

Maybe Dell is alone in this area in trying to improve this, I don't know.

> 
> > This WMI mechanism was designed to enable vendors to create management
> > tools which could talk to interfaces their firmware exposed for this
> > purpose without any platform specific OS driver or vendor specific
> > knowledge on the part of the OS driver [1].
> 
> That was how Microsoft designed it, due to totally different
> requirements of their ecosystem.  You really can't compare the two for
> obvious reasons, and I think that's the major disconnect here.
> 
> > The result of this design decision is as you would expect: there is no
> > consistency in design, no guarantee of interfaces with functional
> > boundaries, across the various WMI interface implementations across
> > vendors (or even within the interface of a given vendor on a given
> > platform).
> 
> See, different ecosystem, I like ours better :)
> 
> > The result is a mechanism which is fundamentally incompatible with
> > standard Linux kernel approach to isolating userspace from the firmware.
> > WMI enumerates available data, methods, and events, and shepherds a
> > buffer of bits back and forth between firmware and userspace through
> > these methods.
> >
> > Over the last few months, I've worked with Mario, Pali, Andy L, and
> > others to try and find an acceptable way to support Dell in their
> > efforts, while respecting Linux's design principles.
> >
> > The first concession was to agree not to automatically publish a WMI
> > interface to userspace. This series includes the mechanism which
> > requires a GUID-specific driver to be written and to provide the file
> > ops structure, explicitly requesting the character device. This creates
> > a WMI GUID granular whitelist. I'm willing to deny any such driver which
> > is sent for review without the explicit collaboration of the vendor, to
> > ensure they are doing their due diligence with respect to their firmware
> > implementation. There has been some discussion in this thread regarding
> > Dell's firmware testing, and the access these interfaces have to
> > critical hardware resources.
> 
> How are you going to prevent out-of-tree drivers from using this same
> api to directly get access to the WMI interface without using any type
> of whitelist or review?

Out of tree drivers is really a distraction in this context.  An out of tree driver
could also just as easily create its own character device without using the
WMI bus.

> 
> > The second concession was to acknowledge that some features implemented
> > in WMI are rightly the domain of the Linux kernel. LEDs, RFKill,
> > Backlight control, hotkey support, etc. When an exported GUID is used
> > for these purposes in addition to whatever purpose userspace requires
> > (see my comment above about a lack of functional boundaries in the
> > exported interfaces), we will provide a way to filter these usages. As
> > Mario said, we can either return an error, or we can attempt to handle
> > the request via the appropriate Linux kernel subsystem connections.
> 
> That's great, but I don't see that in the patches posted, did I miss it?

You are correct, as of v7 this specific filtering of stuff already done in kernel 
is not present.  Unless there is more opposition to this approach I'll modify 
to return errors for those.

> 
> > From the Linux kernel side, the ask is that we acknowledge that it is
> > not practical to audit every WMI interface to present a set of
> > functional knobs to userspace - because they were specifically designed
> > without that requirement.
> 
> Not true, we can audit whatever we want.  whitelists are good for this
> very reason.  Just because a vendor is used to replacing any part of the
> OS with their own custom api direct to the hardware in other operating
> systems, doesn't mean that we have to also emulate that crazy api as
> well, especially given that it can be easily abused, as we discussed
> before.
> 
> > The platform was designed in this way specifically to optimize the
> > software support effort on the part of the vendor. They don't write
> > Windows OS drivers for these mechanisms, it is unrealistic to expect
> > them to write them for Linux.
> 
> Note, we write drivers for hardware for free if asked, so is it
> unrealistic to require a kernel driver if we offer to do it for them? :)
> 
> > I believe we need to arrive at a compromise which allows a vendor to
> > work upstream like Mario is doing on behalf of Dell to fully enable
> > their platforms as they were designed on Linux, while ensuring we don't
> > adversely affect the functionality or security of other platforms. I
> > believe the whitelist and blacklist above provides for this compromise.
> >
> > Perhaps an additional concession we should consider is to make exporting
> > WMI interfaces a configuration option. If not set, the drivers will
> > continue to work, but the WMI bus driver will not create the character
> > device, even if requested. This would allow vendors to create a fully
> > supported platform using upstream code, while allowing individuals the
> > control to disable WMI userspace functionality if they don't trust the
> > mechanism.
> 
> config options do not work, distros have to turn them all on, you know
> that...
> 
> > The alternative seems to be that we accept these features will not be
> > available on Linux for these platforms, or that vendors will develop a
> > single wmi mapping driver out of tree, likely without the noted
> > concessions.
> 
> So you are saying they are blackmailing us that if we do not provide
> these wide-open api, then they just will take their toys and go home?
> In other words, back where we are today?  :)

I think within all of this discussion the spirit of intent is getting lost.

There is an interface that exists today in Windows as a dell signed kernel 
driver and in Linux as dcdbas that lets various tools access interesting
pieces of data from the BIOS.

Dell is deprecating this interface and moving to a driverless model on 
Windows and this patch series represents the parity of the "driverless"
solution for Linux.

Certainly I can advocate internally that all this effort is a waste of time
and something can be carried out of tree and shipped with tools, but
I don't want people using our tools to have to use tainted kernels.

> 
> I understand the goal here of getting this to all work properly, but
> note that this is a different type of an operating system, and for some
> things, maybe we just do not allow direct userspace access for the
> obvious reasons (security, maintance, auditability, long-term-support,
> etc.)  A whitelist of known-good commands sounds like a good place to
> start, why not start with that and go from there?
> 

Take off your "kernel" hat and put on a "customer" hat for a few moments
while I try to put this in practical terms why the whitelist approach doesn't
scale for what I'm trying to do.

Let's say hypothetically a future version of this series that has only whitelisted
commands and tokens lands in a kernel that's in the next Ubuntu LTS, RHEL
release etc.
Hardware coming out about that time works fine, you can control the various
knobs.  Then later that year some new headless hardware is released that has
zigbee controllers that work with inbox kernel drivers but you can't turn them 
on and off any way BUT through the manageability interface (it's headless!).
In the manageability interface we also offer a new class/select or token that 
can control the GPIO that turns on/off these radios.

A patch could be submitted upstream to whitelist that new class/select or token, 
but then you run into the problem that anyone on these stable LTS type distros
won't pick it up.  You can submit it to linux-stable and that might help Ubuntu, 
but a distro like RHEL doesn't track linux-stable.  So that means now you're stuck 
in a place that tools don't work with the interface solely because of a whitelist
that the kernel community hasn't yet blessed that it's ok to turn on/off zigbee
controllers through the OEM's manageability interface.

You now have introduced a ton of red tape that will artificially slow down supporting
HW with Linux for something as simple as controlling a GPIO in a manageability interface.

Tools that could otherwise work in many kernel versions will be gimped to only
offer full functionality in the latest kernel versions.

As a naïve customer why Does Dell's tool for managing my zigbee radios not work
in RHEL 7.x but only in RHEL 7.x+1?  No one will understand that.

So considering the above isn't offering stuff like this a decision better made by the OEM?
If the OEM  doen't want customers to be able to modify something we don't offer it in the
manageability interface.  If the kernel community doesn't want people to be 
modifying something the OEM does offer, it can just as well be blacklisted in the 
kernel driver like the current filtering approach offers.