usb: ohci avoid multiple eof timers

Commit Message

Gerd Hoffmann Feb. 16, 2016, 2:17 p.m. UTC

On Di, 2016-02-16 at 15:34 +0530, P J P wrote:
> When transitioning an OHCI controller to the OHCI_USB_OPERATIONAL
> state, it creates an eof timer object in 'ohci_bus_start'.
> It does not check if one already exists. This results in memory
> leakage and null dereference issue. Add a check to avoid it.

Well, allocating and deallocating the timer all the time isn't a great
idea in the first place.  Can you try the attached patch instead?

thanks,
  Gerd

Comments

Prasad Pandit Feb. 16, 2016, 5:32 p.m. UTC | #1

Hello Gerd,

+-- On Tue, 16 Feb 2016, Gerd Hoffmann wrote --+
| Can you try the attached patch instead?

  Yes, this one works too.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Patch

From d1b07becc481e09225cfe905ec357807ae07f095 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <[e-mail]>
Date: Tue, 16 Feb 2016 15:15:04 +0100
Subject: [PATCH] ohci timer fix

Signed-off-by: Gerd Hoffmann <[e-mail]>
---
 hw/usb/hcd-ohci.c | 31 +++++--------------------------
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index bed55dd..3d1270d 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1347,16 +1347,6 @@  static void ohci_frame_boundary(void *opaque)
  */
 static int ohci_bus_start(OHCIState *ohci)
 {
-    ohci->eof_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
-                    ohci_frame_boundary,
-                    ohci);
-
-    if (ohci->eof_timer == NULL) {
-        trace_usb_ohci_bus_eof_timer_failed(ohci->name);
-        ohci_die(ohci);
-        return 0;
-    }
-
     trace_usb_ohci_start(ohci->name);
 
     /* Delay the first SOF event by one frame time as
@@ -1373,11 +1363,7 @@  static int ohci_bus_start(OHCIState *ohci)
 static void ohci_bus_stop(OHCIState *ohci)
 {
     trace_usb_ohci_stop(ohci->name);
-    if (ohci->eof_timer) {
-        timer_del(ohci->eof_timer);
-        timer_free(ohci->eof_timer);
-    }
-    ohci->eof_timer = NULL;
+    timer_del(ohci->eof_timer);
 }
 
 /* Sets a flag in a port status register but only set it if the port is
@@ -1907,6 +1893,9 @@  static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
     usb_packet_init(&ohci->usb_packet);
 
     ohci->async_td = 0;
+
+    ohci->eof_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+                                   ohci_frame_boundary, ohci);
 }
 
 #define TYPE_PCI_OHCI "pci-ohci"
@@ -2041,23 +2030,13 @@  static bool ohci_eof_timer_needed(void *opaque)
 {
     OHCIState *ohci = opaque;
 
-    return ohci->eof_timer != NULL;
-}
-
-static int ohci_eof_timer_pre_load(void *opaque)
-{
-    OHCIState *ohci = opaque;
-
-    ohci_bus_start(ohci);
-
-    return 0;
+    return timer_pending(ohci->eof_timer);
 }
 
 static const VMStateDescription vmstate_ohci_eof_timer = {
     .name = "ohci-core/eof-timer",
     .version_id = 1,
     .minimum_version_id = 1,
-    .pre_load = ohci_eof_timer_pre_load,
     .needed = ohci_eof_timer_needed,
     .fields = (VMStateField[]) {
         VMSTATE_TIMER_PTR(eof_timer, OHCIState),
-- 
1.8.3.1