[libvirt] question about PCI new_id sysfs interface

[Alex Williamson]

On Wed, 29 Jun 2016 09:47:55 +0100
"Daniel P. Berrange" <berrange at redhat.com> wrote:

> On Tue, Jun 28, 2016 at 02:30:47PM -0400, Bandan Das wrote:
> > "Daniel P. Berrange" <berrange at redhat.com> writes:
> >   
> > > Adding Alex & Bandan, since they signed off the kernel patch which
> > > I'm thinking either pci-back should be made to work more like
> > > vfio, or the kernel patch should be reverted or fixed to take
> > > account of the way pci-back works.
> > >
> > > Whichever way, I don't consider this a libvirt problem to solve. As
> > > Linus' always says - the kernel must never break existing userspace  
> > 
> > Agreed, but in this specific case, the usage is unsafe since unknown indexes
> > are potentially being passed to the driver operations. It should always have been
> > 3. to begin with.  
> 
> Whether the userspace usage is good or not is irrelevant - this kernel change
> has broken existing userspace apps and that is not acceptable and must be fixed.
> 
> I'm fine with suggestions to change future libvirt to work in a better way,
> but we need to fix the regressions seen by *current* libvirt releases

I don't think this is a reasonable demand.  For one, the change was
made 2yrs ago and nobody noticed until now, I don't think there are
stable kernel releases to cover all those kernels.  There must be some
sort of statute of limitations.  Second, the fix was arguably for
security, the previous interface allowed the user to casually override
driver data in ways known to cause driver segfaults.  Match that with
the racy nature of the new_id interface and a system becomes
susceptible to exploit.  Finally, the kernel is returning -EEXIST,
indicating to the user that the entry already exists in the device ID
table, libvirt is choosing to handle that as a failure rather than
acknowledge that the entry exists, there's no need to add or remove
it.  So there's clearly a user bug at play here as well.  If
there's a fix to be made, I think it would be to make xen-pciback
use a fully dynamic device ID table like the other meta drivers
rather than pre-populate a PCI_ANY_ID table.  Thanks,

Alex