From patchwork Fri Jun 9 22:00:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Williamson X-Patchwork-Id: 9779415 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id BF286603B4 for ; Fri, 9 Jun 2017 22:01:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B1C2228712 for ; Fri, 9 Jun 2017 22:01:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A633428715; Fri, 9 Jun 2017 22:01:04 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A74E728712 for ; Fri, 9 Jun 2017 22:01:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751844AbdFIWAs (ORCPT ); Fri, 9 Jun 2017 18:00:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:10278 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751573AbdFIWAq (ORCPT ); Fri, 9 Jun 2017 18:00:46 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AC34663E01; Fri, 9 Jun 2017 22:00:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com AC34663E01 Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=alex.williamson@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com AC34663E01 Received: from gimli.home (ovpn-116-11.phx2.redhat.com [10.3.116.11]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2BDEA5DD70; Fri, 9 Jun 2017 22:00:31 +0000 (UTC) Subject: [PATCH 7/7] vfio: Use driver_override to avert binding to compromising drivers From: Alex Williamson To: kvm@vger.kernel.org Cc: eric.auger@redhat.com, alex.williamson@redhat.com, linux-kernel@vger.kernel.org Date: Fri, 09 Jun 2017 16:00:31 -0600 Message-ID: <20170609220031.31986.80141.stgit@gimli.home> In-Reply-To: <20170609215816.31986.89321.stgit@gimli.home> References: <20170609215816.31986.89321.stgit@gimli.home> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Fri, 09 Jun 2017 22:00:35 +0000 (UTC) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP If a device is bound to a non-vfio, non-whitelisted driver while a group is in use, then the integrity of the group is compromised and will result in hitting a BUG_ON. This code tries to avoid this case by mangling driver_override to force a no-match for the driver. The driver-core will either follow-up with a DRIVER_NOT_BOUND (preferred) or BOUND_DRIVER, at which point we can remove the driver_override mangling. A complication here is that even though the window between these notifications is expected to be extremely small, the vfio group could be removed, which would prevent us from finding the group again to remove the driver_override. We therefore take a group reference when adding to driver_override and release it when removed. A second complication is that driver_override can be modified by the system admin through sysfs. To avoid trivial interference, we add a non- user-visible UUID to the group and use this as part of the mangle string. The above blocks binding to a driver that would compromise the host, but we'd also like to avoid reaching that step if possible. For this we add a wait_event_timeout() with a short, 1 second timeout, which is highly effective in allowing empty groups to finish cleanup. Signed-off-by: Alex Williamson Reviewed-by: Eric Auger Tested-by: Eric Auger --- drivers/vfio/vfio.c | 151 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 144 insertions(+), 7 deletions(-) diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index a25ee4930200..ea786d512faa 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -32,8 +33,10 @@ #include #include #include +#include #include #include +#include #define DRIVER_VERSION "0.3" #define DRIVER_AUTHOR "Alex Williamson " @@ -95,6 +98,7 @@ struct vfio_group { bool noiommu; struct kvm *kvm; struct blocking_notifier_head notifier; + unsigned char uuid[16]; }; struct vfio_device { @@ -352,6 +356,8 @@ static struct vfio_group *vfio_create_group(struct iommu_group *iommu_group) group->nb.notifier_call = vfio_iommu_group_notifier; + generate_random_uuid(group->uuid); + /* * blocking notifiers acquire a rwsem around registering and hold * it around callback. Therefore, need to register outside of @@ -728,6 +734,116 @@ static int vfio_group_nb_verify(struct vfio_group *group, struct device *dev) return vfio_dev_viable(dev, group); } +#define VFIO_TAG_PREFIX "#vfio_group:" + +static char **vfio_find_driver_override(struct device *dev) +{ + if (dev_is_pci(dev)) { + struct pci_dev *pdev = to_pci_dev(dev); + return &pdev->driver_override; + } else if (dev->bus == &platform_bus_type) { + struct platform_device *pdev = to_platform_device(dev); + return &pdev->driver_override; +#ifdef CONFIG_ARM_AMBA + } else if (dev->bus == &amba_bustype) { + struct amba_device *adev = to_amba_device(dev); + return &adev->driver_override; +#endif + } + + return NULL; +} + +/* + * If we're about to bind to something other than a known whitelisted driver + * or known vfio bus driver, try to avert it with driver_override. + */ +static void vfio_group_nb_pre_bind(struct vfio_group *group, struct device *dev) +{ + struct vfio_bus_driver *driver; + struct device_driver *drv = ACCESS_ONCE(dev->driver); + char **driver_override; + + if (vfio_dev_whitelisted(dev, drv)) + return; /* Binding to known "innocuous" device/driver */ + + mutex_lock(&vfio.bus_drivers_lock); + list_for_each_entry(driver, &vfio.bus_drivers_list, vfio_next) { + if (driver->drv == drv) { + mutex_unlock(&vfio.bus_drivers_lock); + return; /* Binding to known vfio bus driver, ok */ + } + } + mutex_unlock(&vfio.bus_drivers_lock); + + /* Can we stall slightly to let users fall off? */ + if (list_empty(&group->device_list)) { + if (wait_event_timeout(vfio.release_q, + !atomic_read(&group->container_users), HZ)) + return; + } + + driver_override = vfio_find_driver_override(dev); + if (driver_override) { + char tag[50], *new = NULL, *old = *driver_override; + + snprintf(tag, sizeof(tag), "%s%pU", + VFIO_TAG_PREFIX, group->uuid); + + if (old && strstr(old, tag)) + return; /* block already in place */ + + new = kasprintf(GFP_KERNEL, "%s%s", old ? old : "", tag); + if (new) { + *driver_override = new; + kfree(old); + vfio_group_get(group); + dev_warn(dev, "vfio: Blocking unsafe driver bind\n"); + return; + } + } + + dev_warn(dev, "vfio: Unsafe driver binding to in-use group!\n"); +} + +/* If we've mangled driver_override, remove it */ +static void vfio_group_nb_post_bind(struct vfio_group *group, + struct device *dev) +{ + char **driver_override = vfio_find_driver_override(dev); + + if (driver_override && *driver_override) { + char tag[50], *new, *start, *end, *old = *driver_override; + + snprintf(tag, sizeof(tag), "%s%pU", + VFIO_TAG_PREFIX, group->uuid); + + start = strstr(old, tag); + if (start) { + end = start + strlen(tag); + + if (old + strlen(old) > end) + memmove(start, end, + strlen(old) - (end - old) + 1); + else + *start = 0; + + if (strlen(old)) { + new = kasprintf(GFP_KERNEL, "%s", old); + if (new) { + *driver_override = new; + kfree(old); + } /* else, in-place terminated, ok */ + } else { + *driver_override = NULL; + kfree(old); + } + + vfio_group_put(group); + } + } +} + static int vfio_iommu_group_notifier(struct notifier_block *nb, unsigned long action, void *data) { @@ -757,14 +873,23 @@ static int vfio_iommu_group_notifier(struct notifier_block *nb, */ break; case IOMMU_GROUP_NOTIFY_BIND_DRIVER: - pr_debug("%s: Device %s, group %d binding to driver\n", + pr_debug("%s: Device %s, group %d binding to driver %s\n", __func__, dev_name(dev), - iommu_group_id(group->iommu_group)); + iommu_group_id(group->iommu_group), dev->driver->name); + if (vfio_group_nb_verify(group, dev)) + vfio_group_nb_pre_bind(group, dev); + break; + case IOMMU_GROUP_NOTIFY_DRIVER_NOT_BOUND: + pr_debug("%s: Device %s, group %d binding fail to driver %s\n", + __func__, dev_name(dev), + iommu_group_id(group->iommu_group), dev->driver->name); + vfio_group_nb_post_bind(group, dev); break; case IOMMU_GROUP_NOTIFY_BOUND_DRIVER: pr_debug("%s: Device %s, group %d bound to driver %s\n", __func__, dev_name(dev), iommu_group_id(group->iommu_group), dev->driver->name); + vfio_group_nb_post_bind(group, dev); BUG_ON(vfio_group_nb_verify(group, dev)); break; case IOMMU_GROUP_NOTIFY_UNBIND_DRIVER: @@ -1351,6 +1476,7 @@ static int vfio_group_unset_container(struct vfio_group *group) if (users != 1) return -EBUSY; + wake_up(&vfio.release_q); __vfio_group_unset_container(group); return 0; @@ -1364,7 +1490,11 @@ static int vfio_group_unset_container(struct vfio_group *group) */ static void vfio_group_try_dissolve_container(struct vfio_group *group) { - if (0 == atomic_dec_if_positive(&group->container_users)) + int users = atomic_dec_if_positive(&group->container_users); + + wake_up(&vfio.release_q); + + if (!users) __vfio_group_unset_container(group); } @@ -1433,19 +1563,26 @@ static bool vfio_group_viable(struct vfio_group *group) static int vfio_group_add_container_user(struct vfio_group *group) { + int ret; + if (!atomic_inc_not_zero(&group->container_users)) return -EINVAL; if (group->noiommu) { - atomic_dec(&group->container_users); - return -EPERM; + ret = -EPERM; + goto out; } if (!group->container->iommu_driver || !vfio_group_viable(group)) { - atomic_dec(&group->container_users); - return -EINVAL; + ret = -EINVAL; + goto out; } return 0; + +out: + atomic_dec(&group->container_users); + wake_up(&vfio.release_q); + return ret; } static const struct file_operations vfio_device_fops;