netdev - Re: [PATCH 1/2] vhost: remove mutex ops in vhost_set_backend

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20200921083506-mutt-send-email-mst@kernel.org>
Date:   Mon, 21 Sep 2020 08:35:16 -0400
From:   "Michael S. Tsirkin" <mst@...hat.com>
To:     "Zhu, Lingshan" <lingshan.zhu@...el.com>
Cc:     jasowang@...hat.com, virtualization@...ts.linux-foundation.org,
        netdev@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH 1/2] vhost: remove mutex ops in vhost_set_backend_features

On Tue, Sep 08, 2020 at 09:00:19PM +0800, Zhu, Lingshan wrote:
> 
> On 9/8/2020 8:05 PM, Michael S. Tsirkin wrote:
> 
>     On Mon, Sep 07, 2020 at 06:52:19PM +0800, Zhu Lingshan wrote:
> 
>         In vhost_vdpa ioctl SET_BACKEND_FEATURES path, currect code
>         would try to acquire vhost dev mutex twice
>         (first shown in vhost_vdpa_unlocked_ioctl), which can lead
>         to a dead lock issue.
>         This commit removed mutex operations in vhost_set_backend_features.
>         As a compensation for vhost_net, a followinig commit will add
>         needed mutex lock/unlock operations in a new function
>         vhost_net_set_backend_features() which is a wrap of
>         vhost_set_backend_features().
> 
>         Signed-off-by: Zhu Lingshan <lingshan.zhu@...el.com>
> 
>     I think you need to squash these two or reorder, we can't first
>     make code racy then fix it up.
> 
> OK, I will send a V2 series with Jason's fixes tomorrow (handle SET/GET_BACKEND_FEATURES in vhost_vdpa ioctl than vring ioctl).
> 
> Thanks,
> BR
> Zhu Lingshan


this never materialized ...

> 
> 
>         ---
>          drivers/vhost/vhost.c | 2 --
>          1 file changed, 2 deletions(-)
> 
>         diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>         index b45519ca66a7..e03c9e6f058f 100644
>         --- a/drivers/vhost/vhost.c
>         +++ b/drivers/vhost/vhost.c
>         @@ -2591,14 +2591,12 @@ void vhost_set_backend_features(struct vhost_dev *dev, u64 features)
>                 struct vhost_virtqueue *vq;
>                 int i;
> 
>         -       mutex_lock(&dev->mutex);
>                 for (i = 0; i < dev->nvqs; ++i) {
>                         vq = dev->vqs[i];
>                         mutex_lock(&vq->mutex);
>                         vq->acked_backend_features = features;
>                         mutex_unlock(&vq->mutex);
>                 }
>         -       mutex_unlock(&dev->mutex);
>          }
>          EXPORT_SYMBOL_GPL(vhost_set_backend_features);
> 
>         --
>         2.18.4
>