Re: [PATCH v3 7/9] vfio: Use driver_override to avert binding to compromising drivers

From: Greg KH <greg@kroah.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: Russell King - ARM Linux <linux@armlinux.org.uk>,
	kvm@vger.kernel.org, eric.auger@redhat.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 7/9] vfio: Use driver_override to avert binding to compromising drivers
Date: Fri, 14 Jul 2017 22:09:39 +0200	[thread overview]
Message-ID: <20170714200939.GA30901@kroah.com> (raw)
In-Reply-To: <20170714100327.569b0f37@w520.home>

On Fri, Jul 14, 2017 at 10:03:27AM -0600, Alex Williamson wrote:
> Hi Greg,
> 
> On Thu, 13 Jul 2017 10:23:14 +0200
> Greg KH <greg@kroah.com> wrote:
> 
> > On Tue, Jul 11, 2017 at 10:41:16AM -0600, Alex Williamson wrote:
> > > Let me give a concrete scenario, I have a dual-port conventional PCI
> > > e1000 NIC.  The IOMMU operates on PCIe requester IDs and therefore both
> > > NIC functions are masked behind the requester ID of a PCIe-to-PCI
> > > bridge.  We cannot have the e1000 driver managing one function and a
> > > user managing the other (via vfio-pci).  
> > 
> > Agreed, but really, if a user asks to do such a thing, they deserve the
> > pieces the kernel ends up in, right?
> 
> I think that's asking a fair bit from users to understand these
> nuances;

There is no "nuance" here.

> at one point in time they can bind the device to e1000 and it
> works,

Yeah, they got lucky!

> at another point in time the same operation crashes the system.

And now they didn't!  What did they do differently?  Oh look, one other
device needed to be unbound/bound/whatever to get this to work properly,
let's do that instead next time.

> Perhaps the user is not even using manual binding, maybe the e1000
> driver is freshly loaded and probes any devices that are not bound.

No, that's not the issue here at all, that should always work as the
kernel driver is telling us that it can support this device just fine.
Nothing "grey" or "nuanced" here at all.

> Maybe the device is passed through /sys/bus/pci/drivers_probe.

Then the user gets to keep the pieces of the kernel that might get spit
out at them.

Again, doing manual binding is a risk that if a user takes, it might or
might not work.  It's always been that way.

> If the user attempts to bind a device to the wrong driver we don't
> intentionally run the system into the ground to make that work.

Are you kidding?  That happens all the time, try to do it yourself and
bind a device to a driver that doesn't expect to be handling it.  If you
are lucky your kernel will crash, if unlucky, it will limp along and do
odd things to the device.  It's been this way since these sysfs files
were added over a decade ago.

> Of course if the user is overriding a match with dynamic IDs or
> driver_override, then I fully agree, the user should be responsible
> for the action they've dictated.  I tend to think of this more towards
> the former than the latter.

The user is always responsible if they are using sysfs to bind/unbind
devices from drivers.  It's not complex or nuanced at all.

thanks,

greg k-h