From patchwork Tue Jun 20 15:48: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: 9799841 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 E4B39603F1 for ; Tue, 20 Jun 2017 15:48:37 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 977262849F for ; Tue, 20 Jun 2017 15:48:37 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8BE84284BF; Tue, 20 Jun 2017 15:48:37 +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=ham 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 E1621284AF for ; Tue, 20 Jun 2017 15:48:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752265AbdFTPsf (ORCPT ); Tue, 20 Jun 2017 11:48:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32806 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751038AbdFTPsd (ORCPT ); Tue, 20 Jun 2017 11:48:33 -0400 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 795A780477; Tue, 20 Jun 2017 15:48:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 795A780477 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=alex.williamson@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 795A780477 Received: from gimli.home (ovpn-116-91.phx2.redhat.com [10.3.116.91]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6E82470460; Tue, 20 Jun 2017 15:48:31 +0000 (UTC) Subject: [PATCH v3 7/9] 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, linux@armlinux.org.uk Date: Tue, 20 Jun 2017 09:48:31 -0600 Message-ID: <20170620154830.17487.1861.stgit@gimli.home> In-Reply-To: <20170620154312.17487.66916.stgit@gimli.home> References: <20170620154312.17487.66916.stgit@gimli.home> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Tue, 20 Jun 2017 15:48:33 +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 | 145 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 138 insertions(+), 7 deletions(-) diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index f40d1508d368..20e57fecf652 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -32,6 +33,7 @@ #include #include #include +#include #include #include @@ -95,6 +97,7 @@ struct vfio_group { bool noiommu; struct kvm *kvm; struct blocking_notifier_head notifier; + unsigned char uuid[16]; }; struct vfio_device { @@ -352,6 +355,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 +733,111 @@ 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; + } + + 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 +867,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 +1470,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 +1484,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 +1557,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;