lkml.org 
[lkml]   [2020]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Subject[RFC PATCH 3/7] vfio/pci: Introduce VF token
    From
    Date
    If we enable SR-IOV on a vfio-pci owned PF, the resulting VFs are not
    fully isolated from the PF. The PF can always cause a denial of
    service to the VF, if not access data passed through the VF directly.
    This is why vfio-pci currently does not bind to PFs with SR-IOV enabled
    and does not provide access itself to enabling SR-IOV on a PF. The
    IOMMU grouping mechanism might allow us a solution to this lack of
    isolation, however the deficiency isn't actually in the DMA path, so
    much as the potential cooperation between PF and VF devices. Also,
    if we were to force VFs into the same IOMMU group as the PF, we severely
    limit the utility of having independent drivers managing PFs and VFs
    with vfio.

    Therefore we introduce the concept of a VF token. The token is
    implemented as a UUID and represents a shared secret which must be set
    by the PF driver and used by the VF drivers in order to access a vfio
    device file descriptor for the VF. The ioctl to set the VF token will
    be provided in a later commit, this commit implements the underlying
    infrastructure. The concept here is to augment the string the user
    passes to match a device within a group in order to retrieve access to
    the device descriptor. For example, rather than passing only the PCI
    device name (ex. "0000:03:00.0") the user would also pass a vf_token
    UUID (ex. "2ab74924-c335-45f4-9b16-8569e5b08258"). The device match
    string therefore becomes:

    "0000:03:00.0 vf_token=2ab74924-c335-45f4-9b16-8569e5b08258"

    This syntax is expected to be extensible to future options as well, with
    the standard being:

    "$DEVICE_NAME $OPTION1=$VALUE1 $OPTION2=$VALUE2"

    The device name must be first and option=value pairs are separated by
    spaces.

    The vf_token option is only required for VFs where the PF device is
    bound to vfio-pci. There is no change for PFs using existing host
    drivers.

    Note that in order to protect existing VF users, not only is it required
    to set a vf_token on the PF before VFs devices can be accessed, but also
    if there are existing VF users, (re)opening the PF device must also
    provide the current vf_token as authentication. This is intended to
    prevent a VF driver starting with a trusted PF driver and later being
    replaced by an unknown driver. A vf_token is not required to open the
    PF device when none of the VF devices are in use by vfio-pci drivers.

    Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
    ---
    drivers/vfio/pci/vfio_pci.c | 125 +++++++++++++++++++++++++++++++++++
    drivers/vfio/pci/vfio_pci_private.h | 8 ++
    2 files changed, 132 insertions(+), 1 deletion(-)

    diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
    index 6b3e73a33cbf..ad45ed3e0432 100644
    --- a/drivers/vfio/pci/vfio_pci.c
    +++ b/drivers/vfio/pci/vfio_pci.c
    @@ -463,6 +463,34 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev)
    vfio_pci_set_power_state(vdev, PCI_D3hot);
    }

    +static struct pci_driver vfio_pci_driver;
    +
    +static void vfio_pci_vf_token_user_add(struct vfio_pci_device *vdev, int val)
    +{
    + struct vfio_device *pf_dev;
    + struct vfio_pci_device *pf_vdev;
    +
    + if (!vdev->pdev->is_virtfn)
    + return;
    +
    + pf_dev = vfio_device_get_from_dev(&vdev->pdev->physfn->dev);
    + if (!pf_dev)
    + return;
    +
    + if (pci_dev_driver(vdev->pdev->physfn) != &vfio_pci_driver) {
    + vfio_device_put(pf_dev);
    + return;
    + }
    +
    + pf_vdev = vfio_device_data(pf_dev);
    +
    + mutex_lock(&pf_vdev->vf_token->lock);
    + pf_vdev->vf_token->users += val;
    + mutex_unlock(&pf_vdev->vf_token->lock);
    +
    + vfio_device_put(pf_dev);
    +}
    +
    static void vfio_pci_release(void *device_data)
    {
    struct vfio_pci_device *vdev = device_data;
    @@ -472,6 +500,7 @@ static void vfio_pci_release(void *device_data)
    if (!(--vdev->refcnt)) {
    vfio_spapr_pci_eeh_release(vdev->pdev);
    vfio_pci_disable(vdev);
    + vfio_pci_vf_token_user_add(vdev, -1);
    }

    mutex_unlock(&vdev->reflck->lock);
    @@ -490,6 +519,7 @@ static int vfio_pci_open(void *device_data)
    mutex_lock(&vdev->reflck->lock);

    if (!vdev->refcnt) {
    + vfio_pci_vf_token_user_add(vdev, 1);
    ret = vfio_pci_enable(vdev);
    if (ret)
    goto error;
    @@ -1275,11 +1305,85 @@ static void vfio_pci_request(void *device_data, unsigned int count)
    mutex_unlock(&vdev->igate);
    }

    +#define VF_TOKEN_ARG "vf_token="
    +
    +/* Called with vdev->vf_token->lock */
    +static int vfio_pci_vf_token_match(struct vfio_pci_device *vdev, char *opts)
    +{
    + char *token;
    + uuid_t uuid;
    + int ret;
    +
    + if (!opts)
    + return -EINVAL;
    +
    + token = strstr(opts, VF_TOKEN_ARG);
    + if (!token)
    + return -EINVAL;
    +
    + token += strlen(VF_TOKEN_ARG);
    +
    + ret = uuid_parse(token, &uuid);
    + if (ret)
    + return ret;
    +
    + if (!uuid_equal(&uuid, &vdev->vf_token->uuid))
    + return -EACCES;
    +
    + return 0;
    +}
    +
    static int vfio_pci_match(void *device_data, char *buf)
    {
    struct vfio_pci_device *vdev = device_data;
    + char *opts;
    + int ret;
    +
    + opts = strchr(buf, ' ');
    + if (opts) {
    + *opts = 0;
    + opts++;
    + }
    +
    + ret = strcmp(pci_name(vdev->pdev), buf);
    + if (ret)
    + return -ENODEV;
    +
    + if (vdev->pdev->is_virtfn) {
    + struct vfio_device *pf_dev =
    + vfio_device_get_from_dev(&vdev->pdev->physfn->dev);

    - return strcmp(pci_name(vdev->pdev), buf) ? -ENODEV : 0;
    + if (pf_dev) {
    + if (pci_dev_driver(vdev->pdev->physfn) ==
    + &vfio_pci_driver) {
    + struct vfio_pci_device *pf_vdev =
    + vfio_device_data(pf_dev);
    +
    + mutex_lock(&pf_vdev->vf_token->lock);
    + ret = vfio_pci_vf_token_match(pf_vdev, opts);
    + mutex_unlock(&pf_vdev->vf_token->lock);
    + }
    +
    + vfio_device_put(pf_dev);
    +
    + if (ret)
    + return -EACCES;
    + }
    + }
    +
    + if (vdev->vf_token) {
    + mutex_lock(&vdev->vf_token->lock);
    +
    + if (vdev->vf_token->users)
    + ret = vfio_pci_vf_token_match(vdev, opts);
    +
    + mutex_unlock(&vdev->vf_token->lock);
    +
    + if (ret)
    + return -EACCES;
    + }
    +
    + return 0;
    }

    static const struct vfio_device_ops vfio_pci_ops = {
    @@ -1351,6 +1455,19 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
    return ret;
    }

    + if (pdev->sriov) {
    + vdev->vf_token = kzalloc(sizeof(*vdev->vf_token), GFP_KERNEL);
    + if (!vdev->vf_token) {
    + vfio_pci_reflck_put(vdev->reflck);
    + vfio_del_group_dev(&pdev->dev);
    + vfio_iommu_group_put(group, &pdev->dev);
    + kfree(vdev);
    + return -ENOMEM;
    + }
    + mutex_init(&vdev->vf_token->lock);
    + uuid_gen(&vdev->vf_token->uuid);
    + }
    +
    if (vfio_pci_is_vga(pdev)) {
    vga_client_register(pdev, vdev, NULL, vfio_pci_set_vga_decode);
    vga_set_legacy_decoding(pdev,
    @@ -1384,6 +1501,12 @@ static void vfio_pci_remove(struct pci_dev *pdev)
    if (!vdev)
    return;

    + if (vdev->vf_token) {
    + WARN_ON(vdev->vf_token->users);
    + mutex_destroy(&vdev->vf_token->lock);
    + kfree(vdev->vf_token);
    + }
    +
    vfio_pci_reflck_put(vdev->reflck);

    vfio_iommu_group_put(pdev->dev.iommu_group, &pdev->dev);
    diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
    index ee6ee91718a4..4ca250207ab6 100644
    --- a/drivers/vfio/pci/vfio_pci_private.h
    +++ b/drivers/vfio/pci/vfio_pci_private.h
    @@ -12,6 +12,7 @@
    #include <linux/pci.h>
    #include <linux/irqbypass.h>
    #include <linux/types.h>
    +#include <linux/uuid.h>

    #ifndef VFIO_PCI_PRIVATE_H
    #define VFIO_PCI_PRIVATE_H
    @@ -84,6 +85,12 @@ struct vfio_pci_reflck {
    struct mutex lock;
    };

    +struct vfio_pci_vf_token {
    + struct mutex lock;
    + uuid_t uuid;
    + u32 users;
    +};
    +
    struct vfio_pci_device {
    struct pci_dev *pdev;
    void __iomem *barmap[PCI_STD_RESOURCE_END + 1];
    @@ -122,6 +129,7 @@ struct vfio_pci_device {
    struct list_head dummy_resources_list;
    struct mutex ioeventfds_lock;
    struct list_head ioeventfds_list;
    + struct vfio_pci_vf_token *vf_token;
    };

    #define is_intx(vdev) (vdev->irq_type == VFIO_PCI_INTX_IRQ_INDEX)
    \
     
     \ /
      Last update: 2020-02-05 00:06    [W:6.195 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site