On Thu, Sep 24, 2020 at 11:21:02AM +0800, Jason Wang wrote:
> Commit 653055b9acd4 ("vhost-vdpa: support get/set backend features")
> introduces two malfunction backend features ioctls:
>
> 1) the ioctls was blindly added to vring ioctl instead of vdpa device
> ioctl
> 2) vhost_set_backend_features() was called when dev mutex has already
> been held which will lead a deadlock
>
I assume this patch requires some patch in qemu as well. Do you have
such patch?
> This patch fixes the above issues.
>
> Cc: Eli Cohen <e...@nvidia.com>
> Reported-by: Zhu Lingshan <lingshan....@intel.com>
> Fixes: 653055b9acd4 ("vhost-vdpa: support get/set backend features")
> Signed-off-by: Jason Wang <jasow...@redhat.com>
> ---
> drivers/vhost/vdpa.c | 30 ++++++++++++++++--------------
> 1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 3fab94f88894..796fe979f997 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -353,8 +353,6 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v,
> unsigned int cmd,
> struct vdpa_callback cb;
> struct vhost_virtqueue *vq;
> struct vhost_vring_state s;
> - u64 __user *featurep = argp;
> - u64 features;
> u32 idx;
> long r;
>
> @@ -381,18 +379,6 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v,
> unsigned int cmd,
>
> vq->last_avail_idx = vq_state.avail_index;
> break;
> - case VHOST_GET_BACKEND_FEATURES:
> - features = VHOST_VDPA_BACKEND_FEATURES;
> - if (copy_to_user(featurep, &features, sizeof(features)))
> - return -EFAULT;
> - return 0;
> - case VHOST_SET_BACKEND_FEATURES:
> - if (copy_from_user(&features, featurep, sizeof(features)))
> - return -EFAULT;
> - if (features & ~VHOST_VDPA_BACKEND_FEATURES)
> - return -EOPNOTSUPP;
> - vhost_set_backend_features(&v->vdev, features);
> - return 0;
> }
>
> r = vhost_vring_ioctl(&v->vdev, cmd, argp);
> @@ -440,8 +426,20 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
> struct vhost_vdpa *v = filep->private_data;
> struct vhost_dev *d = &v->vdev;
> void __user *argp = (void __user *)arg;
> + u64 __user *featurep = argp;
> + u64 features;
> long r;
>
> + if (cmd == VHOST_SET_BACKEND_FEATURES) {
> + r = copy_from_user(&features, featurep, sizeof(features));
> + if (r)
> + return r;
> + if (features & ~VHOST_VDPA_BACKEND_FEATURES)
> + return -EOPNOTSUPP;
> + vhost_set_backend_features(&v->vdev, features);
> + return 0;
> + }
> +
> mutex_lock(&d->mutex);
>
> switch (cmd) {
> @@ -476,6 +474,10 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
> case VHOST_VDPA_SET_CONFIG_CALL:
> r = vhost_vdpa_set_config_call(v, argp);
> break;
> + case VHOST_GET_BACKEND_FEATURES:
> + features = VHOST_VDPA_BACKEND_FEATURES;
> + r = copy_to_user(featurep, &features, sizeof(features));
> + break;
> default:
> r = vhost_dev_ioctl(&v->vdev, cmd, argp);
> if (r == -ENOIOCTLCMD)
> --
> 2.20.1
>
