| 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
| ||
|
Message-ID: <4EF1B63A.6050509@siemens.com>
Date: Wed, 21 Dec 2011 11:34:34 +0100
From: Jan Kiszka <jan.kiszka@...mens.com>
To: Alex Williamson <alex.williamson@...hat.com>
CC: "avi@...hat.com" <avi@...hat.com>,
"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"levinsasha928@...il.com" <levinsasha928@...il.com>
Subject: Re: [PATCH v3 2/2] kvm: Device assignment permission checks
On 2011-12-21 05:59, Alex Williamson wrote:
> Only allow KVM device assignment to attach to devices which:
>
> - Are not bridges
> - Have BAR resources (assume others are special devices)
> - The user has permissions to use
>
> Assigning a bridge is a configuration error, it's not supported, and
> typically doesn't result in the behavior the user is expecting anyway.
> Devices without BAR resources are typically chipset components that
> also don't have host drivers. We don't want users to hold such devices
> captive or cause system problems by fencing them off into an iommu
> domain. We determine "permission to use" by testing whether the user
> has access to the PCI sysfs resource files. By default a normal user
> will not have access to these files, so it provides a good indication
> that an administration agent has granted the user access to the device.
>
> Signed-off-by: Alex Williamson <alex.williamson@...hat.com>
> ---
>
> Documentation/virtual/kvm/api.txt | 4 ++
> virt/kvm/assigned-dev.c | 72 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index ee2c96b..4df9af4 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -1154,6 +1154,10 @@ following flags are specified:
> The KVM_DEV_ASSIGN_ENABLE_IOMMU flag is a mandatory option to ensure
> isolation of the device. Usages not specifying this flag are deprecated.
>
> +Only PCI header type 0 devices with PCI BAR resources are supported by
> +device assignment. The user requesting this ioctl must have read/write
> +access to the PCI sysfs resource files associated with the device.
> +
> 4.49 KVM_DEASSIGN_PCI_DEVICE
>
> Capability: KVM_CAP_DEVICE_DEASSIGNMENT
> diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
> index a251a28..da9690e 100644
> --- a/virt/kvm/assigned-dev.c
> +++ b/virt/kvm/assigned-dev.c
> @@ -17,6 +17,7 @@
> #include <linux/pci.h>
> #include <linux/interrupt.h>
> #include <linux/slab.h>
> +#include <linux/namei.h>
> #include "irq.h"
>
> static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,
> @@ -480,12 +481,71 @@ out:
> return r;
> }
>
> +/* We want to test whether the caller has been granted permissions to
> + * use this device. To be able to configure and control the device,
> + * the user needs access to PCI configuration space and BAR resources.
> + * These are accessed through PCI sysfs. PCI config space is often
> + * passed to the process calling this ioctl via file descriptor, so we
> + * can't rely on access to that file. We can check for permissions
> + * on each of the BAR resource files, which is a pretty clear
> + * indicator that the user has been granted access to the device. */
Minor nit, but
/*
* Comment
* block.
*/
would look more appealing.
> +static int probe_sysfs_permissions(struct pci_dev *dev)
> +{
> +#ifdef CONFIG_SYSFS
> + int i;
> + bool bar_found = false;
> +
> + for (i = PCI_STD_RESOURCES; i <= PCI_STD_RESOURCE_END; i++) {
> + char *kpath, *syspath;
> + struct path path;
> + struct inode *inode;
> + int r;
> +
> + if (!pci_resource_len(dev, i))
> + continue;
> +
> + kpath = kobject_get_path(&dev->dev.kobj, GFP_KERNEL);
> + if (!kpath)
> + return -ENOMEM;
> +
> + /* Per sysfs-rules, sysfs is always at /sys */
> + syspath = kasprintf(GFP_KERNEL, "/sys%s/resource%d", kpath, i);
> + kfree(kpath);
> + if (!syspath)
> + return -ENOMEM;
> +
> + r = kern_path(syspath, LOOKUP_FOLLOW, &path);
> + kfree(syspath);
> + if (r)
> + return r;
> +
> + inode = path.dentry->d_inode;
> +
> + r = inode_permission(inode, MAY_READ | MAY_WRITE | MAY_ACCESS);
> + path_put(&path);
> + if (r)
> + return r;
> +
> + bar_found = true;
> + }
> +
> + /* If no resources, probably something special */
> + if (!bar_found)
> + return -EPERM;
> +
> + return 0;
> +#else
> + return -EINVAL; /* No way to control the device without sysfs */
> +#endif
> +}
> +
> static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
> struct kvm_assigned_pci_dev *assigned_dev)
> {
> int r = 0, idx;
> struct kvm_assigned_dev_kernel *match;
> struct pci_dev *dev;
> + u8 header_type;
>
> if (!(assigned_dev->flags & KVM_DEV_ASSIGN_ENABLE_IOMMU))
> return -EINVAL;
> @@ -516,6 +576,18 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
> r = -EINVAL;
> goto out_free;
> }
> +
> + /* Don't allow bridges to be assigned */
> + pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type);
> + if ((header_type & PCI_HEADER_TYPE) != PCI_HEADER_TYPE_NORMAL) {
> + r = -EPERM;
> + goto out_put;
> + }
> +
> + r = probe_sysfs_permissions(dev);
> + if (r)
> + goto out_put;
> +
> if (pci_enable_device(dev)) {
> printk(KERN_INFO "%s: Could not enable PCI device\n", __func__);
> r = -EBUSY;
>
Looks good to me otherwise, also patch 1.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
--
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/
Powered by blists - more mailing lists