hw/net/virtio-net.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Hi,
This patch addresses a memory leak bug in the usages of `timer_del()`.
The issue arisesfrom the incorrect use of the ambiguous timer API
`timer_del()`, which does not free the timer object. The leak sanitizer
report this issue during fuzzing. The correct API `timer_free()` freed
the timer object instead.
Also I'd like to ask for a way to fix all 100+ wrong usages. In my
opinion, the best way to fix this is to hide to `timer_del()` API and
eliminate all usages of it.
Signed-off-by: Zheng Huang <hz1624917200@outlook.com>
---
hw/net/virtio-net.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index de87cfadff..ca403821e0 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -422,7 +422,7 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
}
} else {
if (q->tx_timer) {
- timer_del(q->tx_timer);
+ timer_free(q->tx_timer);
} else {
qemu_bh_cancel(q->tx_bh);
}
@@ -2844,7 +2844,7 @@ static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq)
if (q->tx_waiting) {
/* We already have queued packets, immediately flush */
- timer_del(q->tx_timer);
+ timer_free(q->tx_timer);
virtio_net_tx_timer(q);
} else {
/* re-arm timer to flush it (and more) on next tick */
@@ -3982,7 +3982,7 @@ static void virtio_net_reset(VirtIODevice *vdev)
n->nobcast = 0;
/* multiqueue is disabled by default */
n->curr_queue_pairs = 1;
- timer_del(n->announce_timer.tm);
+ timer_free(n->announce_timer.tm);
n->announce_timer.round = 0;
n->status &= ~VIRTIO_NET_S_ANNOUNCE;
--
2.34.1
On Thu, 27 Mar 2025 at 16:15, Zheng Huang <hz1624917200@gmail.com> wrote: > +++ b/hw/net/virtio-net.c > @@ -422,7 +422,7 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status) > } > } else { > if (q->tx_timer) { > - timer_del(q->tx_timer); > + timer_free(q->tx_timer); * free()'ing a timer object while setting status in '_set_status()' function does not seem right, probably timer_del() is intended here. > @@ -2844,7 +2844,7 @@ static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq) > > if (q->tx_waiting) { > /* We already have queued packets, immediately flush */ > - timer_del(q->tx_timer); > + timer_free(q->tx_timer); > virtio_net_tx_timer(q); > } else { > /* re-arm timer to flush it (and more) on next tick */ > @@ -3982,7 +3982,7 @@ static void virtio_net_reset(VirtIODevice *vdev) > n->nobcast = 0; > /* multiqueue is disabled by default */ > n->curr_queue_pairs = 1; > - timer_del(n->announce_timer.tm); > + timer_free(n->announce_timer.tm); > n->announce_timer.round = 0; > n->status &= ~VIRTIO_NET_S_ANNOUNCE; * This timer_free() usage does not seem right, likely timer_del() is intended here. And a memory leak is happening due to another reason. Thank you. --- - Prasad
© 2016 - 2025 Red Hat, Inc.