cryptospore / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone
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