Re: [PATCH v1] driver: base: Add driver filter support

[Dan Williams]

On Wed, Aug 4, 2021 at 12:29 PM Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Aug 04, 2021 at 10:43:22AM -0700, Kuppuswamy Sathyanarayanan wrote:
> > Intel's Trust Domain Extensions (TDX) is a new Intel technology that
> > adds support for VMs to maintain confidentiality and integrity in the
> > presence of an untrusted VMM.
> >
> > Given the VMM is an untrusted entity and the VMM presents emulated
> > hardware to the guest, a threat vector similar to Thunderclap [1] is
> > present. Also, for ease of deployment, it is useful to be able to use
> > the same kernel binary on host and guest, but that presents a wide
> > attack surface given the range of hardware supported in typical
> > builds. However, TDX guests only require a small number of drivers, a
> > number small enough to audit for Thunderclap style attacks, and the
> > rest can be disabled via policy. Add a mechanism to filter drivers
> > based on a "protected-guest trusted" indication.
>
> So you are trying to work around the "problem" that distro kernels
> provides drivers for everything by adding additional kernel complexity?
>
> What prevents you from using a sane, stripped down, kernel image for
> these vms which would keep things sane and simpler and easier to audit
> and most importantly, prove, that the image is not doing something you
> don't expect it to do?
>
> Why not just make distros that want to support this type of platform,
> also provide these tiny kernel images?  Why are you pushing this work on
> the kernel community instead?

In fact, these questions are where I started when first encountering
this proposal. Andi has addressed the single kernel image constraint,
but I want to pick up on this "pushing work to the kernel community"
contention. The small list of vetted drivers that a TDX guest needs
will be built-in and maintained in the kernel by the protected guest
developer community, so no "pushing work" there. However, given that
any driver disable mechanism needs to touch the driver core I
advocated to go ahead and make this a general purpose capability to
pick up where this [1] conversation left off. I.e. a general facility
for the corner cases that modprobe and kernel config policy can not
reach. Corner cases like VMM attacking the VM, or broken hardware with
a built-in driver that can't be unbound after the fact.

[1]: https://lore.kernel.org/lkml/20170928090901.GC12599@xxxxxxxxx/

>
> > An alternative solution is to disable untrusted drivers using a custom
> > kernel config for TDX, but such solution will break our goal of using
> > same kernel binary in guest/host or in standard OS distributions. So
> > it is not considered.
>
> Why is that a goal that you _have_ to have?  Who is making that
> requirement?
>
> > Driver filter framework adds support to filter drivers at boot
> > time by using platform specific allow list. This is a generic
> > solution that can be reused by TDX. Driver filter framework adds a
> > new hook (driver_allowed()) in driver_register() interface which
> > will update the "allowed" status of the driver based on platform
> > specific driver allow list or deny list. During driver bind process,
> > trusted status is used to determine whether to continue or deny the
> > bind process. If platform does not register any allow or deny list,
> > all devices will be allowed. Platforms can use wildcard like "ALL:ALL"
> > in bus_name and driver_name of filter node to allow or deny all
> > bus and drivers.
> >
> > Per driver opt-in model is also considered as an alternative design
> > choice, but central allow or deny list is chosen because it is
> > easier to maintain and audit. Also, "driver self serve" might be
> > abused by mistake by driver authors cutting and pasting the code.
> >
> > Also add an option in kernel command line and sysfs to update the
> > allowed or denied drivers list. Driver filter allowed status
> > can be read using following command.
> >
> > cat /sys/bus/$bus/drivers/$driver/allowed
>
> You added a sysfs file without Documentation/ABI/ update as well?
>
> {sigh}

Argh, my fault, that one slipped through in the internal review.