linux-kernel - Re: [PATCH 1/2] kvm: Remove ability to assign a device without iommu support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 20 Dec 2011 07:28:54 -0700
From:	Alex Williamson <alex.williamson@...hat.com>
To:	Sasha Levin <levinsasha928@...il.com>
Cc:	Jan Kiszka <jan.kiszka@...mens.com>,
	"avi@...hat.com" <avi@...hat.com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] kvm: Remove ability to assign a device without
 iommu support

On Tue, 2011-12-20 at 11:08 +0200, Sasha Levin wrote:
> On Tue, 2011-12-20 at 10:03 +0100, Jan Kiszka wrote:
> > On 2011-12-20 09:49, Sasha Levin wrote:
> > > On Mon, 2011-12-19 at 20:19 -0700, Alex Williamson wrote:
> > >> This option has no users and it exposes a security hole that we
> > >> can allow devices to be assigned without iommu protection.  Make
> > >> KVM_DEV_ASSIGN_ENABLE_IOMMU a mandatory option.
> > >>
> > >> Signed-off-by: Alex Williamson <alex.williamson@...hat.com>
> > >> ---
> > >>
> > >>  virt/kvm/assigned-dev.c |   18 +++++++++---------
> > >>  1 files changed, 9 insertions(+), 9 deletions(-)
> > >>
> > >> diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
> > >> index 3ad0925..a251a28 100644
> > >> --- a/virt/kvm/assigned-dev.c
> > >> +++ b/virt/kvm/assigned-dev.c
> > >> @@ -487,6 +487,9 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
> > >>  	struct kvm_assigned_dev_kernel *match;
> > >>  	struct pci_dev *dev;
> > >>  
> > >> +	if (!(assigned_dev->flags & KVM_DEV_ASSIGN_ENABLE_IOMMU))
> > >> +		return -EINVAL;
> > > 
> > > Could we just drop KVM_DEV_ASSIGN_ENABLE_IOMMU and do it by default?
> > > calling KVM_ASSIGN_PCI_DEVICE without that flag set it pretty
> > > meaningless.
> > 
> > There is that thing called "backward compatibility". :)
> 
> Well, Alex suggested skipping deprecation period because there are
> currently no users of KVM_ASSIGN_PCI_DEVICE without
> KVM_DEV_ASSIGN_ENABLE_IOMMU, so it should be fine to just make it the
> default behavior, no?
> 
> We can leave KVM_DEV_ASSIGN_ENABLE_IOMMU itself so userspace won't
> break, but theres no reason to enforce it being set in the kernel code.

As Jan said, the option does have historical meaning.  Ignoring the flag
adds ambiguity to the API, so I think it's best to make it clear which
path is no longer supported.  Either way, we don't get to take the flag
back, so it might as well serve as a sanity check.  Thanks,

Alex 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/