Date:   Tue, 9 Jun 2020 16:23:54 -0700
From:   Rajat Jain <rajatja@...gle.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rajat Jain <rajatxjain@...il.com>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        "Krishnakumar, Lalithambika" <lalithambika.krishnakumar@...el.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        linux-pci <linux-pci@...r.kernel.org>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Jean-Philippe Brucker <jean-philippe@...aro.org>,
        Prashant Malani <pmalani@...gle.com>,
        Benson Leung <bleung@...gle.com>,
        Todd Broch <tbroch@...gle.com>,
        Alex Levin <levinale@...gle.com>,
        Mattias Nissler <mnissler@...gle.com>,
        Zubin Mithra <zsm@...gle.com>,
        Bernie Keany <bernie.keany@...el.com>,
        Aaron Durbin <adurbin@...gle.com>,
        Diego Rivas <diegorivas@...gle.com>,
        Duncan Laurie <dlaurie@...gle.com>,
        Furquan Shaikh <furquan@...gle.com>,
        Jesse Barnes <jsbarnes@...gle.com>,
        Christian Kellner <christian@...lner.me>,
        Alex Williamson <alex.williamson@...hat.com>,
        Joerg Roedel <joro@...tes.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC] Restrict the untrusted devices, to bind to only a set of
 "whitelisted" drivers

Hi Bjorn,

Thanks for sending out the summary, I was about to send it out but got lazy.

On Tue, Jun 9, 2020 at 2:04 PM Bjorn Helgaas <helgaas@...nel.org> wrote:
>
> On Sun, Jun 07, 2020 at 01:36:32PM +0200, Greg Kroah-Hartman wrote:
>
> > Your "problem" I think can be summed up a bit more concise:
> >       - you don't trust kernel drivers to be "secure" for untrusted
> >         devices
> >       - you only want to bind kernel drivers to "internal" devices
> >         automatically as you "trust" drivers in that situation.
> >       - you want to only bind specific kernel drivers that you somehow
> >         feel are "secure" to untrusted devices "outside" of a system
> >         when those devices are added to the system.
> >
> > Is that correct?
> >
> > If so, fine, you can do that today with the bind/unbind ability of
> > drivers, right?  After boot with your "trusted" drivers bound to
> > "internal" devices, turn off autobind of drivers to devices and then
> > manually bind them when you see new devices show up, as those "must" be
> > from external devices (see the bind/unbind files that all drivers export
> > for how to do this, and old lwn.net articles, this feature has been
> > around for a very long time.)
> >
> > I know for USB you can do this, odds are PCI you can turn off
> > autobinding as well, as I think this is a per-bus flag somewhere.  If
> > that's not exported to userspace, should be trivial to do so, should be
> > somewere in the driver model already...
> >
> > Ah, yes, look at the "drivers_autoprobe" and "drivers_probe" files in
> > sysfs for all busses.  Do those not work for you?
> >
> > My other points are the fact that you don't want to put policy in the
> > kernel, and I think that you can do everything you want in userspace
> > today, except maybe the fact that trying to determine what is "inside"
> > and "outside" is not always easy given that most hardware does not
> > export this information properly, if at all.  Go work with the firmware
> > people on that issue please, that would be most helpful for everyone
> > involved to get that finally straightened out.
>
> To sketch this out, my understanding of how this would work is:
>
>   - Expose the PCI pdev->untrusted bit in sysfs.  We don't expose this
>     today, but doing so would be trivial.  I think I would prefer a
>     sysfs name like "external" so it's more descriptive and less of a
>     judgment.

Yes. I think we should probably semantically differentiate between
"external" and "external facing" devices. Root ports and downstream
ports can be "external facing" but are actually internal devices.
Anything below an "external facing" device is "external". So the sysfs
attribute "external" should be set only for devices that are truly
external.

So I think we can possibly synthesize "external" sysfs attribute from
"untrusted" bit like this (Sorry code looks more complex than it is):

parent = pdev->bus->self;

if (pdev->untrusted) {
        if (parent && parent->untrusted)
                pdev->external = true;   /* Device downstream of
un-trusted device = external */
        else {
                pdev->external = false   /* Root complex or an
internal Downstream port */
} else {
        pdev->external = false; /* Trusted device = Internal device */
}

For platforms that don't expose and untrusted" device, everything is
assumed to be an "internal device".

Just a suggestion: Do you think an enum attribute may be better
instead, whose values could be "internal" / "external" /
"external-facing" in case need arises later to distinguish between
them?

>
>     This comes from either the DT "external-facing" property or the
>     ACPI "ExternalFacingPort" property.
>
>   - All devices present at boot are enumerated.  Any statically built
>     drivers will bind to them before any userspace code runs.

Yes. For our (thunderbolt / USB4) use case, we'd still be protected
because we can control the PCIe tunnels to thunderbolt / USB4 devices
and will not enable them until we are ready. So while this approach
may not work for a system that always enables PCIe connections to
external devices at boot, it works for our use case as we are looking
for only thunderbolt / USB4 devices. (Not a problem or concern, just
wanted to be clear).

>
>     If you want to keep statically built drivers from binding, you'd
>     need to invent some mechanism so pci_driver_init() could clear
>     drivers_autoprobe after registering pci_bus_type.

At present I am not planning this.

>
>   - Early userspace code prevents modular drivers from automatically
>     binding to PCI devices:
>
>       echo 0 > /sys/bus/pci/drivers_autoprobe

Yes.
I believe this setting will apply it equally to both modular and
statically linked drivers?

>
>     This prevents modular drivers from binding to all devices, whether
>     present at boot or hot-added.

Yes, at this time, the userspace will need to monitor udev events for
any new PCI devices hot added, and lookup the VID/DID in pci driver
database (Isn't it somewhere like modules.pcimap modules.dap or
something in /lib/modules?) to get the driver name. and then after
consulting a maintained whitelist, do the following:

>
>   - Userspace code uses the sysfs "bind" file to control which drivers
>     are loaded and can bind to each device, e.g.,
>
>       echo 0000:02:00.0 > /sys/bus/pci/drivers/nvme/bind
>
> Is that what you're thinking?  Is that enough for the control you
> need, Rajat?

Yes, It sounds like it. That is my current plan.

The one thing that still needs more thought is how about the
"pcieport" driver that enumerates the PCI bridges. I'm unsure if it
needs to be whitelisted for further enumeration downstream. What do
you think?

Thanks & Best Regards,

Rajat

Powered by blists - more mailing lists