Re: KVM device assignment and user privileges

[Avi Kivity <avi@redhat.com>]

On 11/21/2011 06:42 AM, Chris Wright wrote:
> * Avi Kivity (avi@redhat.com) wrote:
> > On 11/20/2011 04:58 PM, Sasha Levin wrote:
> > > Hi all,
> > >
> > > I've been working on adding device assignment to KVM tools, and started
> > > with the basics of just getting a device assigned using the
> > > KVM_ASSIGN_PCI_DEVICE ioctl.
> > >
> > > What I've figured is that unprivileged users can request any PCI device
> > > to be assigned to him, including devices which he shouldn't be touching.
> > >
> > > In my case, it happened with the VGA card, where an unprivileged user
> > > simply called KVM_ASSIGN_PCI_DEVICE with the bus, seg and fn of the VGA
> > > card and caused the display on the host to go apeshit.
> > >
> > > Was it supposed to work this way? 
> > 
> > No, of course not.
>
> Indeed.  A device is typically owned by a host OS driver which precludes
> device assignment from working.  If it's not, the unprivilged guest
> will not have access to the device's config space or resource bars as
> they are only rw for a privileged user.  And similarly, /dev/kvm was
> typically left as 0644.  As you can see, it's fragile.
>
> > > I couldn't find any security checks in
> > > the code paths of KVM_ASSIGN_PCI_DEVICE and it looks like any user can
> > > invoke it with any parameters he'd want - enabling him to kill the host.
> > 
> > Alex, Chris?
>
> The security checks were removed some time back.  The expectation was
> that there was nothing an unprivleged user could usefully do w/ the
> assign device ioctl, and the assign irq ioctl only works after assign
> device.  It's built on an overly fragile set of assumptions, however.
> Avi, the simplest short term thing to do now might be simply revert:
>
> 48bb09e KVM: remove CAP_SYS_RAWIO requirement from kvm_vm_ioctl_assign_irq

That does nothing for for the KVM_ASSIGN_PCI_DEVICE ioctl.

> While it's a regression for existing unprivileged users it's better than
> a hole.  And in the meantime, we can come up w/ something better to
> replace with.

How about doing an access(2) equivalent check for one of the sysfs files
used to represent the device?  If libvirt already chowns them to the
right user, then we have no regression, at least for that use case.

-- 
error compiling committee.c: too many arguments to function