If error occurs while processing the virtio request we should call
'virtqueue_detach_element' to detach the element from the virtqueue
before free the elem.
Signed-off-by: Li Qiang <liq3ea@163.com>
---
hw/virtio/virtio-mem.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 7740fc613f..5ac6c3ec67 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -318,8 +318,7 @@ static void virtio_mem_handle_request(VirtIODevice *vdev, VirtQueue *vq)
if (iov_to_buf(elem->out_sg, elem->out_num, 0, &req, len) < len) {
virtio_error(vdev, "virtio-mem protocol violation: invalid request"
" size: %d", len);
- g_free(elem);
- return;
+ goto out_free;
}
if (iov_size(elem->in_sg, elem->in_num) <
@@ -327,8 +326,7 @@ static void virtio_mem_handle_request(VirtIODevice *vdev, VirtQueue *vq)
virtio_error(vdev, "virtio-mem protocol violation: not enough space"
" for response: %zu",
iov_size(elem->in_sg, elem->in_num));
- g_free(elem);
- return;
+ goto out_free;
}
type = le16_to_cpu(req.type);
@@ -348,12 +346,15 @@ static void virtio_mem_handle_request(VirtIODevice *vdev, VirtQueue *vq)
default:
virtio_error(vdev, "virtio-mem protocol violation: unknown request"
" type: %d", type);
- g_free(elem);
- return;
+ goto out_free;
}
g_free(elem);
}
+
+out_free:
+ virtqueue_detach_element(vq, elem, 0);
+ g_free(elem);
}
static void virtio_mem_get_config(VirtIODevice *vdev, uint8_t *config_data)
--
2.17.1
On 13.08.20 18:46, Li Qiang wrote: For now we use "virtio-mem:" for the subject, without the "hw: "part. > If error occurs while processing the virtio request we should call > 'virtqueue_detach_element' to detach the element from the virtqueue > before free the elem. What's the effect of this? In all cases we trigger a virtio_error(), so do we really have to bother? > > Signed-off-by: Li Qiang <liq3ea@163.com> > --- > hw/virtio/virtio-mem.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c > index 7740fc613f..5ac6c3ec67 100644 > --- a/hw/virtio/virtio-mem.c > +++ b/hw/virtio/virtio-mem.c > @@ -318,8 +318,7 @@ static void virtio_mem_handle_request(VirtIODevice *vdev, VirtQueue *vq) > if (iov_to_buf(elem->out_sg, elem->out_num, 0, &req, len) < len) { > virtio_error(vdev, "virtio-mem protocol violation: invalid request" > " size: %d", len); > - g_free(elem); > - return; > + goto out_free; > } > > if (iov_size(elem->in_sg, elem->in_num) < > @@ -327,8 +326,7 @@ static void virtio_mem_handle_request(VirtIODevice *vdev, VirtQueue *vq) > virtio_error(vdev, "virtio-mem protocol violation: not enough space" > " for response: %zu", > iov_size(elem->in_sg, elem->in_num)); > - g_free(elem); > - return; > + goto out_free; > } > > type = le16_to_cpu(req.type); > @@ -348,12 +346,15 @@ static void virtio_mem_handle_request(VirtIODevice *vdev, VirtQueue *vq) > default: > virtio_error(vdev, "virtio-mem protocol violation: unknown request" > " type: %d", type); > - g_free(elem); > - return; > + goto out_free; > } > > g_free(elem); > } > + > +out_free: > + virtqueue_detach_element(vq, elem, 0); > + g_free(elem); > } > > static void virtio_mem_get_config(VirtIODevice *vdev, uint8_t *config_data) > -- Thanks, David / dhildenb
David Hildenbrand <david@redhat.com> 于2020年8月14日周五 上午1:15写道: > > On 13.08.20 18:46, Li Qiang wrote: > > For now we use "virtio-mem:" for the subject, without the "hw: "part. > > > If error occurs while processing the virtio request we should call > > 'virtqueue_detach_element' to detach the element from the virtqueue > > before free the elem. > > What's the effect of this? In all cases we trigger a virtio_error(), so > do we really have to bother? > Though the 'in_use' will be reset to 0 while reseting the virtio device. The mapped sglist will not be unammped. There maybe some undesired behavior. CC Paolo to make a confirmation. Thanks, Li Qiang > > > > Signed-off-by: Li Qiang <liq3ea@163.com> > > --- > > hw/virtio/virtio-mem.c | 13 +++++++------ > > 1 file changed, 7 insertions(+), 6 deletions(-) > > > > diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c > > index 7740fc613f..5ac6c3ec67 100644 > > --- a/hw/virtio/virtio-mem.c > > +++ b/hw/virtio/virtio-mem.c > > @@ -318,8 +318,7 @@ static void virtio_mem_handle_request(VirtIODevice *vdev, VirtQueue *vq) > > if (iov_to_buf(elem->out_sg, elem->out_num, 0, &req, len) < len) { > > virtio_error(vdev, "virtio-mem protocol violation: invalid request" > > " size: %d", len); > > - g_free(elem); > > - return; > > + goto out_free; > > } > > > > if (iov_size(elem->in_sg, elem->in_num) < > > @@ -327,8 +326,7 @@ static void virtio_mem_handle_request(VirtIODevice *vdev, VirtQueue *vq) > > virtio_error(vdev, "virtio-mem protocol violation: not enough space" > > " for response: %zu", > > iov_size(elem->in_sg, elem->in_num)); > > - g_free(elem); > > - return; > > + goto out_free; > > } > > > > type = le16_to_cpu(req.type); > > @@ -348,12 +346,15 @@ static void virtio_mem_handle_request(VirtIODevice *vdev, VirtQueue *vq) > > default: > > virtio_error(vdev, "virtio-mem protocol violation: unknown request" > > " type: %d", type); > > - g_free(elem); > > - return; > > + goto out_free; > > } > > > > g_free(elem); > > } > > + > > +out_free: > > + virtqueue_detach_element(vq, elem, 0); > > + g_free(elem); > > } > > > > static void virtio_mem_get_config(VirtIODevice *vdev, uint8_t *config_data) > > > > > -- > Thanks, > > David / dhildenb >
On 14.08.20 03:01, Li Qiang wrote: > David Hildenbrand <david@redhat.com> 于2020年8月14日周五 上午1:15写道: >> >> On 13.08.20 18:46, Li Qiang wrote: >> >> For now we use "virtio-mem:" for the subject, without the "hw: "part. >> >>> If error occurs while processing the virtio request we should call >>> 'virtqueue_detach_element' to detach the element from the virtqueue >>> before free the elem. >> >> What's the effect of this? In all cases we trigger a virtio_error(), so >> do we really have to bother? >> > > Though the 'in_use' will be reset to 0 while reseting the virtio device. > The mapped sglist will not be unammped. > There maybe some undesired behavior. CC Paolo to make a confirmation. Looking at hw/virtio/virtio-crypto.c, this seems to be the right thing to do. Can you please respin, avoiding adding the label, only inserting the 3 separate virtqueue_detach_element() calls? Thanks! -- Thanks, David / dhildenb
© 2016 - 2024 Red Hat, Inc.