[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 04/14] i6300esb: Fix signed integer overflow
From: |
Paolo Bonzini |
Subject: |
[Qemu-devel] [PULL 04/14] i6300esb: Fix signed integer overflow |
Date: |
Thu, 26 Mar 2015 17:02:24 +0100 |
From: David Gibson <address@hidden>
If the guest programs a sufficiently large timeout value an integer
overflow can occur in i6300esb_restart_timer(). e.g. if the maximum
possible timer preload value of 0xfffff is programmed then we end up with
the calculation:
timeout = get_ticks_per_sec() * (0xfffff << 15) / 33000000;
get_ticks_per_sec() returns 1000000000 (10^9) giving:
10^9 * (0xfffff * 2^15) == 0x1dcd632329b000000 (65 bits)
Obviously the division by 33MHz brings it back under 64-bits, but the
overflow has already occurred.
Since signed integer overflow has undefined behaviour in C, in theory this
could be arbitrarily bad. In practice, the overflowed value wraps around
to something negative, causing the watchdog to immediately expire, killing
the guest, which is still fairly bad.
The bug can be triggered by running a Linux guest, loading the i6300esb
driver with parameter "heartbeat=2046" and opening /dev/watchdog. The
watchdog will trigger as soon as the device is opened.
This patch corrects the problem by using muldiv64(), which effectively
allows a 128-bit intermediate value between the multiplication and
division.
Signed-off-by: David Gibson <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
---
hw/watchdog/wdt_i6300esb.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
index e694fa9..4ebdbb8 100644
--- a/hw/watchdog/wdt_i6300esb.c
+++ b/hw/watchdog/wdt_i6300esb.c
@@ -125,8 +125,14 @@ static void i6300esb_restart_timer(I6300State *d, int
stage)
else
timeout <<= 5;
- /* Get the timeout in units of ticks_per_sec. */
- timeout = get_ticks_per_sec() * timeout / 33000000;
+ /* Get the timeout in units of ticks_per_sec.
+ *
+ * ticks_per_sec is typically 10^9 == 0x3B9ACA00 (30 bits), with
+ * 20 bits of user supplied preload, and 15 bits of scale, the
+ * multiply here can exceed 64-bits, before we divide by 33MHz, so
+ * we use a higher-precision intermediate result.
+ */
+ timeout = muldiv64(get_ticks_per_sec(), timeout, 33000000);
i6300esb_debug("stage %d, timeout %" PRIi64 "\n", d->stage, timeout);
--
2.3.3
- [Qemu-devel] [PULL 00/14] Misc bugfixes for 2.3.0-rc2, Paolo Bonzini, 2015/03/26
- [Qemu-devel] [PULL 01/14] rcu tests: fix compilation on 32-bit ppc, Paolo Bonzini, 2015/03/26
- [Qemu-devel] [PULL 04/14] i6300esb: Fix signed integer overflow,
Paolo Bonzini <=
- [Qemu-devel] [PULL 05/14] cpus: Don't kick un-realized cpus., Paolo Bonzini, 2015/03/26
- [Qemu-devel] [PULL 02/14] fw_cfg: factor out initialization of FW_CFG_ID (rev. number), Paolo Bonzini, 2015/03/26
- [Qemu-devel] [PULL 07/14] vmw_pvscsi: use PCI DMA APIs, Paolo Bonzini, 2015/03/26
- [Qemu-devel] [PULL 08/14] nbd: Fix up comment after commit e140177, Paolo Bonzini, 2015/03/26
- [Qemu-devel] [PULL 06/14] megasas: use PCI DMA APIs, Paolo Bonzini, 2015/03/26
- [Qemu-devel] [PULL 03/14] i6300esb: Correct endiannness, Paolo Bonzini, 2015/03/26
- [Qemu-devel] [PULL 13/14] misc: fix typos in copyright declaration, Paolo Bonzini, 2015/03/26
- [Qemu-devel] [PULL 12/14] exec: avoid possible overwriting of mmaped area in qemu_ram_remap, Paolo Bonzini, 2015/03/26
- [Qemu-devel] [PULL 09/14] m68k: memory: Replace memory_region_init_ram with memory_region_allocate_system_memory, Paolo Bonzini, 2015/03/26
- [Qemu-devel] [PULL 10/14] mips: memory: Replace memory_region_init_ram with memory_region_allocate_system_memory, Paolo Bonzini, 2015/03/26