netdev - 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220519144801.m7ioxoa5beo5jzv7@sgarzare-redhat>
Date:   Thu, 19 May 2022 16:48:01 +0200
From:   Stefano Garzarella <sgarzare@...hat.com>
To:     Eugenio Pérez <eperezma@...hat.com>
Cc:     mst@...hat.com, jasowang@...hat.com, linux-kernel@...r.kernel.org,
        gdawar@...inx.com, lingshan.zhu@...el.com, kvm@...r.kernel.org,
        lulu@...hat.com, netdev@...r.kernel.org, lvivier@...hat.com,
        eli@...lanox.com, virtualization@...ts.linux-foundation.org,
        parav@...dia.com
Subject: Re: [PATCH] vdpasim: allow to enable a vq repeatedly

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.

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

Thanks,
Stefano