From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50296) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YZq3p-0007PG-Ec for qemu-devel@nongnu.org; Sun, 22 Mar 2015 20:17:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YZq3o-0001co-5c for qemu-devel@nongnu.org; Sun, 22 Mar 2015 20:17:41 -0400 Date: Mon, 23 Mar 2015 11:18:26 +1100 From: David Gibson Message-ID: <20150323001826.GA25043@voom.fritz.box> References: <1426821116-16617-1-git-send-email-david@gibson.dropbear.id.au> <1426821116-16617-3-git-send-email-david@gibson.dropbear.id.au> <550BE4B6.2040405@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="MGYHOYXEY6WxJCY8" Content-Disposition: inline In-Reply-To: <550BE4B6.2040405@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/2] i6300esb: Fix signed integer overflow List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: agraf@suse.de, mst@redhat.com, qemu-ppc@nongnu.org, rjones@redhat.com, qemu-devel@nongnu.org --MGYHOYXEY6WxJCY8 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Mar 20, 2015 at 10:13:26AM +0100, Paolo Bonzini wrote: >=20 >=20 > 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 wi= th > > the calculation: > >=20 > > timeout =3D get_ticks_per_sec() * (0xfffff << 15) / 33000000; > >=20 > > get_ticks_per_sec() returns 1000000000 (10^9) giving: > >=20 > > 10^9 * (0xfffff * 2^15) =3D=3D 0x1dcd632329b000000 (65 bits) > >=20 > > Obviously the division by 33MHz brings it back under 64-bits, but the > > overflow has already occurred. > >=20 > > Since signed integer overflow has undefined behaviour in C, in theory t= his > > could be arbitrarily bad. In practice, the overflowed value wraps arou= nd > > to something negative, causing the watchdog to immediately expire, kill= ing > > the guest, which is still fairly bad. > >=20 > > The bug can be triggered by running a Linux guest, loading the i6300esb > > driver with parameter "heartbeat=3D2046" and opening /dev/watchdog. The > > watchdog will trigger as soon as the device is opened. > >=20 > > This patch corrects the problem by using an __int128_t temporary. With > > suitable rearrangement of the calculations, I expect it would be possib= le > > to avoid the __int128_t. But since we already use __int128_t extensive= ly > > in the memory region code, and this is not a hot path, the super-wide > > integer seems like the simplest approach. >=20 > 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. >=20 > Instead, you can use muldiv64 which has exactly this purpose. Ah, good point. I'll repost using muldiv64. --=20 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 --MGYHOYXEY6WxJCY8 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVD1vSAAoJEGw4ysog2bOSMqMP/2WetD3hwCjZdPS309PGTDeO +32cI30ud8Y+u+rjZneH12WHHaoU2X521Zed7lJut1zVfYWFx6FFWQ5nWLlGUB+c KEne6CFnkO+jRnUEVJX26FBdqxl8IlFKbhJ3uJtmYUbgTgv3GKNbquesYY+SxIaN yONVwKpLkVVSmeujaqLeENJ+nHF0wlZ0HNyjMq7jwrH2AHPjHUzIItCREskylW1U I8kxO20X67/zofuBaa+YxKK8w2s5YMWGlTC4M7wQqhAaQYXkOJJNjksOVYeKqSYc XLsJ55llbU+/Ou90cW5njqw7EVgk4vlEkTOw/9Hg4YqPzdwCu8wYIlH2+3YOtG4g xqQ0auQH8LuDNEPboSgqlNuLs2yuOkMX+85nPs3gMv17Mes/7I27AmzcHHFg25N/ u+kePyc8xKBgDDJEBnKbOR8hK442pm5RuyebtV1sZVmn1recO6IzUM2/WLq+4mJW wXl4wYvMeqBDG1tDcByl9W5pBPsF4h+PONkD958qz2jDJM6DvzGAqz8lFdS8Num9 7hq4gM6odCIvt1afhiFWAvecSuA12BYYSF4MlHu5PZNJ+JpgHHJATTNZqOBdKaEP rRz+WF8mRLhRAZSGbL5sbyUO5phk/ClrmYppYSR9hkLCxvLyBYp9hPEeqPyx860D 6YrmkGmKqb2EM5IdMtHo =y7UF -----END PGP SIGNATURE----- --MGYHOYXEY6WxJCY8--