Commit 4bc94d5d authored by Alex Williamson's avatar Alex Williamson
Browse files

vfio: Fix lockdep issue 



When we open a device file descriptor, we currently have the
following:

vfio_group_get_device_fd()
  mutex_lock(&group->device_lock);
    open()
    ...
    if (ret)
      release()

If we hit that error case, we call the backend driver release path,
which for vfio-pci looks like this:

vfio_pci_release()
  vfio_pci_disable()
    vfio_pci_try_bus_reset()
      vfio_pci_get_devs()
        vfio_device_get_from_dev()
          vfio_group_get_device()
            mutex_lock(&group->device_lock);

Whoops, we've stumbled back onto group.device_lock and created a
deadlock.  There's a low likelihood of ever seeing this play out, but
obviously it needs to be fixed.  To do that we can use a reference to
the vfio_device for vfio_group_get_device_fd() rather than holding the
lock.  There was a loop in this function, theoretically allowing
multiple devices with the same name, but in practice we don't expect
such a thing to happen and the code is already aborting from the loop
with break on any sort of error rather than continuing and only
parsing the first match anyway, so the loop was effectively unused
already.

Signed-off-by: default avatarAlex Williamson <alex.williamson@redhat.com>
Fixes: 20f30017 ("vfio/pci: Fix racy vfio_device_get_from_dev() call")
Reported-by: default avatarJoerg Roedel <joro@8bytes.org>
Tested-by: default avatarJoerg Roedel <jroedel@suse.de>
parent afdf0b91

drivers/vfio/vfio.c

+54 −37
Original line number Diff line number Diff line
@@ -689,6 +689,23 @@ struct vfio_device *vfio_device_get_from_dev(struct device *dev) 
}

EXPORT_SYMBOL_GPL(vfio_device_get_from_dev);



static struct vfio_device *vfio_device_get_from_name(struct vfio_group *group,

						     char *buf)

{

	struct vfio_device *device;



	mutex_lock(&group->device_lock);

	list_for_each_entry(device, &group->device_list, group_next) {

		if (!strcmp(dev_name(device->dev), buf)) {

			vfio_device_get(device);

			break;

		}

	}

	mutex_unlock(&group->device_lock);



	return device;

}



/*

 * Caller must hold a reference to the vfio_device

 */
@@ -1198,20 +1215,22 @@ static int vfio_group_get_device_fd(struct vfio_group *group, char *buf) 
{

	struct vfio_device *device;

	struct file *filep;

	int ret = -ENODEV;

	int ret;



	if (0 == atomic_read(&group->container_users) ||

	    !group->container->iommu_driver || !vfio_group_viable(group))

		return -EINVAL;



	mutex_lock(&group->device_lock);

	list_for_each_entry(device, &group->device_list, group_next) {

		if (strcmp(dev_name(device->dev), buf))

			continue;

	device = vfio_device_get_from_name(group, buf);

	if (!device)

		return -ENODEV;



	ret = device->ops->open(device->device_data);

		if (ret)

			break;

	if (ret) {

		vfio_device_put(device);

		return ret;

	}



	/*

	 * We can't use anon_inode_getfd() because we need to modify

	 * the f_mode flags directly to allow more than just ioctls
@@ -1219,7 +1238,8 @@ static int vfio_group_get_device_fd(struct vfio_group *group, char *buf) 
	ret = get_unused_fd_flags(O_CLOEXEC);

	if (ret < 0) {

		device->ops->release(device->device_data);

			break;

		vfio_device_put(device);

		return ret;

	}



	filep = anon_inode_getfile("[vfio-device]", &vfio_device_fops,
@@ -1228,7 +1248,8 @@ static int vfio_group_get_device_fd(struct vfio_group *group, char *buf) 
		put_unused_fd(ret);

		ret = PTR_ERR(filep);

		device->ops->release(device->device_data);

			break;

		vfio_device_put(device);

		return ret;

	}



	/*
@@ -1238,13 +1259,9 @@ static int vfio_group_get_device_fd(struct vfio_group *group, char *buf) 
	 */

	filep->f_mode |= (FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE);



		vfio_device_get(device);

	atomic_inc(&group->container_users);



	fd_install(ret, filep);

		break;

	}

	mutex_unlock(&group->device_lock);



	return ret;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE