The tx_bh or tx_timer will free in virtio_net_del_queue() function, when
removing virtio-net queues if the guest doesn't support multiqueue. But
it might be still referenced by virtio_net_set_status(), which needs to
be set NULL. And also the tx_waiting needs to be set zero to prevent
virtio_net_set_status() accessing tx_bh or tx_timer.
Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
hw/net/virtio-net.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 7d091c9..98bd683 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1522,9 +1522,12 @@ static void virtio_net_del_queue(VirtIONet *n, int index)
if (q->tx_timer) {
timer_del(q->tx_timer);
timer_free(q->tx_timer);
+ q->tx_timer = NULL;
} else {
qemu_bh_delete(q->tx_bh);
+ q->tx_bh = NULL;
}
+ q->tx_waiting = 0;
virtio_del_queue(vdev, index * 2 + 1);
}
--
1.8.3.1
On 2017年04月26日 14:45, Yunjian Wang wrote: > The tx_bh or tx_timer will free in virtio_net_del_queue() function, when > removing virtio-net queues if the guest doesn't support multiqueue. But > it might be still referenced by virtio_net_set_status(), which needs to > be set NULL. And also the tx_waiting needs to be set zero to prevent > virtio_net_set_status() accessing tx_bh or tx_timer. > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > --- > hw/net/virtio-net.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 7d091c9..98bd683 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -1522,9 +1522,12 @@ static void virtio_net_del_queue(VirtIONet *n, int index) > if (q->tx_timer) { > timer_del(q->tx_timer); > timer_free(q->tx_timer); > + q->tx_timer = NULL; > } else { > qemu_bh_delete(q->tx_bh); > + q->tx_bh = NULL; > } > + q->tx_waiting = 0; > virtio_del_queue(vdev, index * 2 + 1); > } > Thanks for the patch. It looks to me that clearing tx_waiting is sufficient or is there any other reason that you need set tx_timer/tx_bh to NULL? Thanks
> -----Original Message----- > From: Jason Wang [mailto:jasowang@redhat.com] > Sent: Wednesday, April 26, 2017 5:18 PM > To: wangyunjian <wangyunjian@huawei.com>; mst@redhat.com; qemu-devel@nongnu.org > Cc: caihe <caihe@huawei.com> > Subject: Re: [Qemu-devel] [PATCH] virtio-net: fix wild pointer when remove virtio-net queues > > > > On 2017年04月26日 14:45, Yunjian Wang wrote: > > The tx_bh or tx_timer will free in virtio_net_del_queue() function, > > when removing virtio-net queues if the guest doesn't support > > multiqueue. But it might be still referenced by > > virtio_net_set_status(), which needs to be set NULL. And also the > > tx_waiting needs to be set zero to prevent > > virtio_net_set_status() accessing tx_bh or tx_timer. > > > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > > --- > > hw/net/virtio-net.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index > > 7d091c9..98bd683 100644 > > --- a/hw/net/virtio-net.c > > +++ b/hw/net/virtio-net.c > > @@ -1522,9 +1522,12 @@ static void virtio_net_del_queue(VirtIONet *n, int index) > > if (q->tx_timer) { > > timer_del(q->tx_timer); > > timer_free(q->tx_timer); > > + q->tx_timer = NULL; > > } else { > > qemu_bh_delete(q->tx_bh); > > + q->tx_bh = NULL; > > } > > + q->tx_waiting = 0; > > virtio_del_queue(vdev, index * 2 + 1); > > } > > > > Thanks for the patch. > > It looks to me that clearing tx_waiting is sufficient or is there any > other reason that youneed set tx_timer/tx_bh to NULL? It's just coding habit to avoid access wild pointer, such as used-after-free porblem, it hard to locate the code that cause the bug. Thanks > > Thanks
On 2017年04月26日 18:45, wangyunjian wrote: >> -----Original Message----- >> From: Jason Wang [mailto:jasowang@redhat.com] >> Sent: Wednesday, April 26, 2017 5:18 PM >> To: wangyunjian <wangyunjian@huawei.com>; mst@redhat.com; qemu-devel@nongnu.org >> Cc: caihe <caihe@huawei.com> >> Subject: Re: [Qemu-devel] [PATCH] virtio-net: fix wild pointer when remove virtio-net queues >> >> >> >> On 2017年04月26日 14:45, Yunjian Wang wrote: >>> The tx_bh or tx_timer will free in virtio_net_del_queue() function, >>> when removing virtio-net queues if the guest doesn't support >>> multiqueue. But it might be still referenced by >>> virtio_net_set_status(), which needs to be set NULL. And also the >>> tx_waiting needs to be set zero to prevent >>> virtio_net_set_status() accessing tx_bh or tx_timer. >>> >>> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> >>> --- >>> hw/net/virtio-net.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index >>> 7d091c9..98bd683 100644 >>> --- a/hw/net/virtio-net.c >>> +++ b/hw/net/virtio-net.c >>> @@ -1522,9 +1522,12 @@ static void virtio_net_del_queue(VirtIONet *n, int index) >>> if (q->tx_timer) { >>> timer_del(q->tx_timer); >>> timer_free(q->tx_timer); >>> + q->tx_timer = NULL; >>> } else { >>> qemu_bh_delete(q->tx_bh); >>> + q->tx_bh = NULL; >>> } >>> + q->tx_waiting = 0; >>> virtio_del_queue(vdev, index * 2 + 1); >>> } >>> >> Thanks for the patch. >> >> It looks to me that clearing tx_waiting is sufficient or is there any >> other reason that youneed set tx_timer/tx_bh to NULL? > It's just coding habit to avoid access wild pointer, such as > used-after-free porblem, it hard to locate the code that cause the bug. > > Thanks Applied and queued for -stable. Thanks >> Thanks
© 2016 - 2024 Red Hat, Inc.