From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Baron Subject: Re: [Qemu-devel] [PATCH] kvm: deassign irqs in reset path Date: Fri, 30 Mar 2012 16:31:40 -0400 Message-ID: <20120330203140.GC2376@redhat.com> References: <201203301918.q2UJI63c005908@int-mx02.intmail.prod.int.phx2.redhat.com> <4F760993.304@web.de> <20120330201313.GB2376@redhat.com> <4F761517.6010105@web.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: alex.williamson@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org, mst@redhat.com To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:9973 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760690Ab2C3Ubo (ORCPT ); Fri, 30 Mar 2012 16:31:44 -0400 Content-Disposition: inline In-Reply-To: <4F761517.6010105@web.de> Sender: kvm-owner@vger.kernel.org List-ID: 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