Hello,
On Thu, 27 Mar 2025 at 16:59, Zheng Huang <hz1624917...@gmail.com> wrote:
> 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.
>
> In addition, 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.
>
> @@ -379,9 +379,9 @@ static void e1000_reset_hold(Object *obj, ResetType type)
> E1000BaseClass *edc = E1000_GET_CLASS(d);
> uint8_t *macaddr = d->conf.macaddr.a;
>
> - timer_del(d->autoneg_timer);
> - timer_del(d->mit_timer);
> - timer_del(d->flush_queue_timer);
> + timer_free(d->autoneg_timer);
> + timer_free(d->mit_timer);
> + timer_free(d->flush_queue_timer);
> d->mit_timer_on = 0;
> d->mit_irq_level = 0;
> d->mit_ide = 0;
> diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
> index 2413858790..61fdc8a3e9 100644
> --- a/hw/net/e1000e_core.c
> +++ b/hw/net/e1000e_core.c
> @@ -127,7 +127,7 @@ static inline void
> e1000e_intrmgr_stop_timer(E1000IntrDelayTimer *timer)
> {
> if (timer->running) {
> - timer_del(timer->timer);
> + timer_free(timer->timer);
> timer->running = false;
> }
> }
> @@ -360,13 +360,13 @@ e1000e_intrmgr_fire_all_timers(E1000ECore *core)
> int i;
>
> if (core->itr.running) {
> - timer_del(core->itr.timer);
> + timer_free(core->itr.timer);
> e1000e_intrmgr_on_throttling_timer(&core->itr);
> }
>
> for (i = 0; i < E1000E_MSIX_VEC_NUM; i++) {
> if (core->eitr[i].running) {
> - timer_del(core->eitr[i].timer);
> + timer_free(core->eitr[i].timer);
> e1000e_intrmgr_on_msix_throttling_timer(&core->eitr[i]);
> }
> }
> @@ -3452,7 +3452,7 @@ static void e1000e_reset(E1000ECore *core, bool sw)
> {
> int i;
>
> - timer_del(core->autoneg_timer);
> + timer_free(core->autoneg_timer);
>
> e1000e_intrmgr_reset(core);
* I doubt if this is correct; Because timer_del() API explicitly says
-> /* stop a timer, but do not dealloc it */
* Secondly: autoneg_timer/mit_timer/flush_queue_timer objects are
freed in 'pci_e1000_uninit()/e1000e_pci_uninit()' functions via
timer_free() calls. So the memory leak could be because the respective
*_pci__uninit() function is not called?
Thank you.
---
- Prasad
