linux-kernel - Re: [PATCH] vdpasim: allow to enable a vq repeatedly

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAJaqyWfywBEe21P1a1LG2v=Ab2d-L5x19MrtWLvDqccOxFLMqQ@mail.gmail.com>
Date:   Thu, 19 May 2022 16:57:18 +0200
From:   Eugenio Perez Martin <eperezma@...hat.com>
To:     Stefano Garzarella <sgarzare@...hat.com>
Cc:     Michael Tsirkin <mst@...hat.com>, Jason Wang <jasowang@...hat.com>,
        linux-kernel@...r.kernel.org, Gautam Dawar <gdawar@...inx.com>,
        Zhu Lingshan <lingshan.zhu@...el.com>,
        kvm list <kvm@...r.kernel.org>, Cindy Lu <lulu@...hat.com>,
        netdev@...r.kernel.org, Laurent Vivier <lvivier@...hat.com>,
        Eli Cohen <eli@...lanox.com>,
        virtualization <virtualization@...ts.linux-foundation.org>,
        Parav Pandit <parav@...dia.com>
Subject: Re: [PATCH] vdpasim: allow to enable a vq repeatedly

On Thu, May 19, 2022 at 4:48 PM Stefano Garzarella <sgarzare@...hat.com> wrote:
>
> On Thu, May 19, 2022 at 04:31:45PM +0200, Eugenio Pérez wrote:
> >Code must be resilient to enable a queue many times.
> >
> >At the moment the queue is resetting so it's definitely not the expected
> >behavior.
> >
> >Fixes: 2c53d0f64c06 ("vdpasim: vDPA device simulator")
> >Cc: stable@...r.kernel.org
> >Signed-off-by: Eugenio Pérez <eperezma@...hat.com>
> >---
> > drivers/vdpa/vdpa_sim/vdpa_sim.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> >index ddbe142af09a..b53cd00ad161 100644
> >--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> >+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> >@@ -355,9 +355,10 @@ static void vdpasim_set_vq_ready(struct vdpa_device *vdpa, u16 idx, bool ready)
> >       struct vdpasim_virtqueue *vq = &vdpasim->vqs[idx];
> >
> >       spin_lock(&vdpasim->lock);
> >-      vq->ready = ready;
> >-      if (vq->ready)
> >+      if (!vq->ready) {
> >+              vq->ready = ready;
> >               vdpasim_queue_ready(vdpasim, idx);
> >+      }
>
> But this way the first time vq->ready is set to true, then it will never
> be set back to false.
>

You're right, I had in mind to reset the flow before enabling as the
only possibility.

Sending v2 with that part fixed, thanks!

> Should we leave the assignment out of the block?
> Maybe after the if block to avoid the problem we are fixing.
>
> Thanks,
> Stefano
>