The problem has been reported by gcc with CFLAGS=-O3:
.../hw/virtio/vhost-shadow-virtqueue.c: In function ‘vhost_svq_poll’:
.../hw/virtio/vhost-shadow-virtqueue.c:538:12:
error: ‘len’ may be used uninitialized [-Werror=maybe-uninitialized]
538 | return len;
| ^~~
vhost_svq_get_buf() returns NULL if SVQ is empty but doesn't set len to 0,
and vhost_svq_poll() returns len without checking the return of
vhost_svq_get_buf(). So if the SVQ is empty vhost_svq_poll() can return
an random value.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
hw/virtio/vhost-shadow-virtqueue.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
index 430729635815..31cf642db267 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -420,6 +420,7 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
vring_used_elem_t used_elem;
uint16_t last_used, last_used_chain, num;
+ *len = 0;
if (!vhost_svq_more_used(svq)) {
return NULL;
}
--
2.39.1
On Fri, Feb 17, 2023 at 11:42 AM Laurent Vivier <lvivier@redhat.com> wrote: > > The problem has been reported by gcc with CFLAGS=-O3: > > .../hw/virtio/vhost-shadow-virtqueue.c: In function ‘vhost_svq_poll’: > .../hw/virtio/vhost-shadow-virtqueue.c:538:12: > error: ‘len’ may be used uninitialized [-Werror=maybe-uninitialized] > 538 | return len; > | ^~~ > > vhost_svq_get_buf() returns NULL if SVQ is empty but doesn't set len to 0, > and vhost_svq_poll() returns len without checking the return of > vhost_svq_get_buf(). So if the SVQ is empty vhost_svq_poll() can return > an random value. > s/an random/a random/. I think this solves the same as https://www.mail-archive.com/qemu-devel@nongnu.org/msg939383.html ? Thanks! > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > hw/virtio/vhost-shadow-virtqueue.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c > index 430729635815..31cf642db267 100644 > --- a/hw/virtio/vhost-shadow-virtqueue.c > +++ b/hw/virtio/vhost-shadow-virtqueue.c > @@ -420,6 +420,7 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq, > vring_used_elem_t used_elem; > uint16_t last_used, last_used_chain, num; > > + *len = 0; > if (!vhost_svq_more_used(svq)) { > return NULL; > } > -- > 2.39.1 >
On 2/17/23 12:24, Eugenio Perez Martin wrote: > On Fri, Feb 17, 2023 at 11:42 AM Laurent Vivier <lvivier@redhat.com> wrote: >> >> The problem has been reported by gcc with CFLAGS=-O3: >> >> .../hw/virtio/vhost-shadow-virtqueue.c: In function ‘vhost_svq_poll’: >> .../hw/virtio/vhost-shadow-virtqueue.c:538:12: >> error: ‘len’ may be used uninitialized [-Werror=maybe-uninitialized] >> 538 | return len; >> | ^~~ >> >> vhost_svq_get_buf() returns NULL if SVQ is empty but doesn't set len to 0, >> and vhost_svq_poll() returns len without checking the return of >> vhost_svq_get_buf(). So if the SVQ is empty vhost_svq_poll() can return >> an random value. >> > > s/an random/a random/. > > I think this solves the same as > https://www.mail-archive.com/qemu-devel@nongnu.org/msg939383.html ? Yes, exactly. Thanks, Laurent > > Thanks! > >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >> --- >> hw/virtio/vhost-shadow-virtqueue.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c >> index 430729635815..31cf642db267 100644 >> --- a/hw/virtio/vhost-shadow-virtqueue.c >> +++ b/hw/virtio/vhost-shadow-virtqueue.c >> @@ -420,6 +420,7 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq, >> vring_used_elem_t used_elem; >> uint16_t last_used, last_used_chain, num; >> >> + *len = 0; >> if (!vhost_svq_more_used(svq)) { >> return NULL; >> } >> -- >> 2.39.1 >> >
© 2016 - 2024 Red Hat, Inc.