[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 2/2] hw: move timer_new from init() into realize() to avoi

From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v2 2/2] hw: move timer_new from init() into realize() to avoid memleaks
Date: Thu, 20 Feb 2020 19:51:35 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1

On 2/20/20 6:56 PM, Peter Maydell wrote:
On Mon, 17 Feb 2020 at 03:22, <address@hidden> wrote:

From: Pan Nengyuan <address@hidden>

There are some memleaks when we call 'device_list_properties'. This patch move 
timer_new from init into realize to fix it.
Meanwhile, do the null check in mos6522_reset() to avoid null deref if we move 
timer_new into realize().

Reported-by: Euler Robot <address@hidden>
Signed-off-by: Pan Nengyuan <address@hidden>
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
index 19e154b870..980eda7599 100644
--- a/hw/misc/mos6522.c
+++ b/hw/misc/mos6522.c
@@ -465,11 +465,15 @@ static void mos6522_reset(DeviceState *dev)
      s->timers[0].frequency = s->frequency;
      s->timers[0].latch = 0xffff;
      set_counter(s, &s->timers[0], 0xffff);
-    timer_del(s->timers[0].timer);
+    if (s->timers[0].timer) {
+        timer_del(s->timers[0].timer);
+    }

      s->timers[1].frequency = s->frequency;
      s->timers[1].latch = 0xffff;
-    timer_del(s->timers[1].timer);
+    if (s->timers[1].timer) {
+        timer_del(s->timers[1].timer);
+    }

What code path calls a device 'reset' method on a device
that has not yet been realized ? I wasn't expecting that
to be valid...

This is not valid. What I understood while reviewing this patch is on reset the timer is removed from the timers list. But this patch miss setting timer = NULL in case the device is reset multiple times, here can happen a NULL deref.

-- PMM

reply via email to

[Prev in Thread] Current Thread [Next in Thread]