On Fri, Mar 20, 2015 at 10:13:26AM +0100, Paolo Bonzini wrote: > > > On 20/03/2015 04:11, David Gibson wrote: > > 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 an __int128_t temporary. With > > suitable rearrangement of the calculations, I expect it would be possible > > to avoid the __int128_t. But since we already use __int128_t extensively > > in the memory region code, and this is not a hot path, the super-wide > > integer seems like the simplest approach. > > We don't use __int128_t, we use the Int128 struct---which however > doesn't have a multiplication function. __int128_t is not available on > 32-bit machines, and is only used under #ifdef CONFIG_INT128. > > Instead, you can use muldiv64 which has exactly this purpose.
Ah, good point. I'll repost using muldiv64.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
pgpJJ_yx9EKIp.pgp
Description: PGP signature
