Re: [Qemu-devel] [PATCH] kvm: deassign irqs in reset path

[Jason Baron]

On Fri, Mar 30, 2012 at 10:18:31PM +0200, Jan Kiszka wrote:
> >>> The root cause of the problem is that the 'reset_assigned_device()' code
> >>> first writes a 0 to the command register. Then, when qemu subsequently 
> >>> does
> >>> a kvm_deassign_irq() (called by assign_irq(), in the system_reset path),
> >>> the kernel ends up calling '__msix_mask_irq()', which performs a write to
> >>> the memory mapped msi vector space. Since, we've explicitly told the 
> >>> device
> >>> to disallow mmio access (via the 0 write to the command register), we end
> >>> up with the above 'Unsupported Request'.
> >>>
> >>> The fix here is to first call kvm_deassign_irq(), before doing the reset,
> >>
> >> s/fix/workaround/. This is a kernel bug if userspace can crash the
> >> system like this, no? Let's fix the kernel first and then look at what
> >> needs to be changed here.
> >>
> >> Jan
> >>
> > 
> > But don't I need special privalege to run the device assignment bits?
> 
> Yes, but even that might be moderated by a management component like
> libvirt.
> 
> > For example, this crash is precipitated by a write of '0' to the pci
> > device config register from userspace. Surely, not every is allowed to
> > do that write. So it seems to me, that this patch is in keeping with the
> > current model of how things work.
> 
> No user should needlessly be able to crash the host by issuing valid
> commands in a special order.
> 
> Jan
> 

Right, but as I see device-assign.c, we are essentially programming the
pci device directly from userspace. Put another way, the kernel could
crash the system if it programmed a pci device in the wrong order. So I
don't see how this is different. But maybe I'm misunderstanding the
model here?

Thanks,

-Jason