On Mon, Feb 21, 2022 at 12:49:16PM +0100, Stefano Garzarella wrote:
> vhost_vsock_stop() calls vhost_dev_check_owner() to check the device
> ownership. It expects current->mm to be valid.
> 
> vhost_vsock_stop() is also called by vhost_vsock_dev_release() when
> the user has not done close(), so when we are in do_exit(). In this
> case current->mm is invalid and we're releasing the device, so we
> should clean it anyway.
> 
> Let's check the owner only when vhost_vsock_stop() is called
> by an ioctl.




> Fixes: 433fc58e6bf2 ("VSOCK: Introduce vhost_vsock.ko")
> Cc: [email protected]
> Reported-by: [email protected]
> Signed-off-by: Stefano Garzarella <[email protected]>
> ---
>  drivers/vhost/vsock.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index d6ca1c7ad513..f00d2dfd72b7 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -629,16 +629,18 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
>       return ret;
>  }
>  
> -static int vhost_vsock_stop(struct vhost_vsock *vsock)
> +static int vhost_vsock_stop(struct vhost_vsock *vsock, bool check_owner)

>  {
>       size_t i;
>       int ret;
>  
>       mutex_lock(&vsock->dev.mutex);
>  
> -     ret = vhost_dev_check_owner(&vsock->dev);
> -     if (ret)
> -             goto err;
> +     if (check_owner) {
> +             ret = vhost_dev_check_owner(&vsock->dev);
> +             if (ret)
> +                     goto err;
> +     }
>  
>       for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
>               struct vhost_virtqueue *vq = &vsock->vqs[i];
> @@ -753,7 +755,7 @@ static int vhost_vsock_dev_release(struct inode *inode, 
> struct file *file)
>        * inefficient.  Room for improvement here. */
>       vsock_for_each_connected_socket(vhost_vsock_reset_orphans);
>  
> -     vhost_vsock_stop(vsock);

Let's add an explanation:

When invoked from release we can not fail so we don't
check return code of vhost_vsock_stop.
We need to stop vsock even if it's not the owner.

> +     vhost_vsock_stop(vsock, false);
>       vhost_vsock_flush(vsock);
>       vhost_dev_stop(&vsock->dev);
>  
> @@ -868,7 +870,7 @@ static long vhost_vsock_dev_ioctl(struct file *f, 
> unsigned int ioctl,
>               if (start)
>                       return vhost_vsock_start(vsock);
>               else
> -                     return vhost_vsock_stop(vsock);
> +                     return vhost_vsock_stop(vsock, true);
>       case VHOST_GET_FEATURES:
>               features = VHOST_VSOCK_FEATURES;
>               if (copy_to_user(argp, &features, sizeof(features)))
> -- 
> 2.35.1

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to