|
 |
9ae3a8 |
From 7f476950b0f5780d1112f8e9d0d92ece55ae6912 Mon Sep 17 00:00:00 2001
|
|
|
9ae3a8 |
From: Richard Jones <rjones@redhat.com>
|
|
|
9ae3a8 |
Date: Wed, 1 Nov 2017 11:33:00 +0100
|
|
|
9ae3a8 |
Subject: [PATCH 5/7] i6300esb: Fix signed integer overflow
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
RH-Author: Richard Jones <rjones@redhat.com>
|
|
|
9ae3a8 |
Message-id: <1509535982-27927-2-git-send-email-rjones@redhat.com>
|
|
|
9ae3a8 |
Patchwork-id: 77461
|
|
|
9ae3a8 |
O-Subject: [RHEL-7.5 qemu-kvm PATCH v3 1/3] i6300esb: Fix signed integer overflow
|
|
|
9ae3a8 |
Bugzilla: 1470244
|
|
|
9ae3a8 |
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
|
|
|
9ae3a8 |
RH-Acked-by: Thomas Huth <thuth@redhat.com>
|
|
|
9ae3a8 |
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
From: David Gibson <david@gibson.dropbear.id.au>
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
If the guest programs a sufficiently large timeout value an integer
|
|
|
9ae3a8 |
overflow can occur in i6300esb_restart_timer(). e.g. if the maximum
|
|
|
9ae3a8 |
possible timer preload value of 0xfffff is programmed then we end up with
|
|
|
9ae3a8 |
the calculation:
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
timeout = get_ticks_per_sec() * (0xfffff << 15) / 33000000;
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
get_ticks_per_sec() returns 1000000000 (10^9) giving:
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
10^9 * (0xfffff * 2^15) == 0x1dcd632329b000000 (65 bits)
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
Obviously the division by 33MHz brings it back under 64-bits, but the
|
|
|
9ae3a8 |
overflow has already occurred.
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
Since signed integer overflow has undefined behaviour in C, in theory this
|
|
|
9ae3a8 |
could be arbitrarily bad. In practice, the overflowed value wraps around
|
|
|
9ae3a8 |
to something negative, causing the watchdog to immediately expire, killing
|
|
|
9ae3a8 |
the guest, which is still fairly bad.
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
The bug can be triggered by running a Linux guest, loading the i6300esb
|
|
|
9ae3a8 |
driver with parameter "heartbeat=2046" and opening /dev/watchdog. The
|
|
|
9ae3a8 |
watchdog will trigger as soon as the device is opened.
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
This patch corrects the problem by using muldiv64(), which effectively
|
|
|
9ae3a8 |
allows a 128-bit intermediate value between the multiplication and
|
|
|
9ae3a8 |
division.
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
|
|
|
9ae3a8 |
Message-Id: <1427075508-12099-3-git-send-email-david@gibson.dropbear.id.au>
|
|
|
9ae3a8 |
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
|
|
|
9ae3a8 |
(cherry picked from commit 4bc7b4d56657ebf75b986ad46e959cf7232ff26a)
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1470244
|
|
|
9ae3a8 |
Upstream-status: 4bc7b4d56657ebf75b986ad46e959cf7232ff26a
|
|
|
9ae3a8 |
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
|
|
|
9ae3a8 |
---
|
|
|
9ae3a8 |
hw/watchdog/wdt_i6300esb.c | 10 ++++++++--
|
|
|
9ae3a8 |
1 file changed, 8 insertions(+), 2 deletions(-)
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
|
|
|
9ae3a8 |
index a2ace52..be35034 100644
|
|
|
9ae3a8 |
--- a/hw/watchdog/wdt_i6300esb.c
|
|
|
9ae3a8 |
+++ b/hw/watchdog/wdt_i6300esb.c
|
|
|
9ae3a8 |
@@ -125,8 +125,14 @@ static void i6300esb_restart_timer(I6300State *d, int stage)
|
|
|
9ae3a8 |
else
|
|
|
9ae3a8 |
timeout <<= 5;
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
- /* Get the timeout in units of ticks_per_sec. */
|
|
|
9ae3a8 |
- timeout = get_ticks_per_sec() * timeout / 33000000;
|
|
|
9ae3a8 |
+ /* Get the timeout in units of ticks_per_sec.
|
|
|
9ae3a8 |
+ *
|
|
|
9ae3a8 |
+ * ticks_per_sec is typically 10^9 == 0x3B9ACA00 (30 bits), with
|
|
|
9ae3a8 |
+ * 20 bits of user supplied preload, and 15 bits of scale, the
|
|
|
9ae3a8 |
+ * multiply here can exceed 64-bits, before we divide by 33MHz, so
|
|
|
9ae3a8 |
+ * we use a higher-precision intermediate result.
|
|
|
9ae3a8 |
+ */
|
|
|
9ae3a8 |
+ timeout = muldiv64(get_ticks_per_sec(), timeout, 33000000);
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
i6300esb_debug("stage %d, timeout %" PRIi64 "\n", d->stage, timeout);
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
--
|
|
|
9ae3a8 |
1.8.3.1
|
|
|
9ae3a8 |
|