Re: [PATCH 09/11] PCI: add matching checks for driver_override binding

From: Alex Williamson <alex.williamson@redhat.com>
To: Max Gurtovoy <mgurtovoy@nvidia.com>
Cc: <cohuck@redhat.com>, <kvm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <jgg@nvidia.com>,
	<aviadye@nvidia.com>, <oren@nvidia.com>, <shahafs@nvidia.com>,
	<parav@nvidia.com>, <artemp@nvidia.com>, <kwankhede@nvidia.com>,
	<ACurrid@nvidia.com>, <cjia@nvidia.com>, <yishaih@nvidia.com>,
	<kevin.tian@intel.com>, <hch@infradead.org>,
	<targupta@nvidia.com>, <shameerali.kolothum.thodi@huawei.com>,
	<liulongfang@huawei.com>, <yan.y.zhao@intel.com>
Subject: Re: [PATCH 09/11] PCI: add matching checks for driver_override binding
Date: Tue, 8 Jun 2021 15:26:43 -0600	[thread overview]
Message-ID: <20210608152643.2d3400c1.alex.williamson@redhat.com> (raw)
In-Reply-To: <20210603160809.15845-10-mgurtovoy@nvidia.com>

On Thu, 3 Jun 2021 19:08:07 +0300
Max Gurtovoy <mgurtovoy@nvidia.com> wrote:

> Allowing any driver in the system to be unconditionally bound to any
> PCI HW is dangerous. Connecting a driver to a physical HW device it was
> never intended to operate may trigger exploitable kernel bugs, or worse.
> It also allows userspace to load and run kernel code that otherwise
> would never be runnable on the system.

This is just another way that an admin can do bad things, with the
intention that they know what they're doing and if not they get to
keep the pieces.  There's also still the new_id scheme for binding the
wrong drivers to devices, so the hole this claims to be addressing is
still fully present.

> driver_override was designed to make it easier to load vfio_pci, so

Actually driver_override was designed to resolve the non-deterministic
behavior of new_id, which allows inserting dynamic match entries.  The
problem is those match entries match any device that might come along
during the time window when userspace is trying to bind a specific
device to a specific driver.  driver_override flipped the problem to
match a device to a driver rather than vice versa.  Other bus types
have since adopted driver_override interfaces as well.

> focus it on that single use case. driver_override will only work on

It's used for other use cases across numerous bus types now.  For
instance, how can I user driver_override to bind pci-stub to a device
after this?  driverctl(8) uses driver_override to perform arbitrary
driver overrides, this breaks all but the vfio-pci use case.

> drivers that specifically opt into this feature and the driver now has
> the opportunity to provide a proper match table that indicates what HW
> it can properly support. vfio-pci continues to support everything.

In doing so, this also breaks the new_id method for vfio-pci.  Sorry,
with so many userspace regressions, crippling the driver_override
interface with an assumption of such a narrow focus, creating a vfio
specific match flag, I don't see where this can go.  Thanks,

Alex