From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43181) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8QNn-0000ch-Ny for qemu-devel@nongnu.org; Fri, 26 Jun 2015 05:57:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z8QNg-0004iv-Pt for qemu-devel@nongnu.org; Fri, 26 Jun 2015 05:57:15 -0400 Date: Fri, 26 Jun 2015 10:57:05 +0100 From: Stefan Hajnoczi Message-ID: <20150626095705.GH15457@stefanha-thinkpad.redhat.com> References: <4273ff9b577ffc2b2b3ff7137beabfb831a38d65.1435175476.git.jcody@redhat.com> <20150625142835.GP4419@stefanha-thinkpad.redhat.com> <20150625150520.GB13877@localhost.localdomain> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="tcC6YSqBgqqkz7Sb" Content-Disposition: inline In-Reply-To: <20150625150520.GB13877@localhost.localdomain> Subject: Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] block: vpc - prevent overflow if max_table_entries >= 0x40000000 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: kwolf@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org --tcC6YSqBgqqkz7Sb Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jun 25, 2015 at 11:05:20AM -0400, Jeff Cody wrote: > On Thu, Jun 25, 2015 at 03:28:35PM +0100, Stefan Hajnoczi wrote: > > On Wed, Jun 24, 2015 at 03:54:27PM -0400, Jeff Cody wrote: > > > @@ -269,7 +270,9 @@ static int vpc_open(BlockDriverState *bs, QDict *= options, int flags, > > > goto fail; > > > } > > > =20 > > > - s->pagetable =3D qemu_try_blockalign(bs->file, s->max_table_= entries * 4); > > > + pagetable_size =3D (size_t) s->max_table_entries * 4; > > > + > > > + s->pagetable =3D qemu_try_blockalign(bs->file, pagetable_siz= e); > >=20 > > On 32-bit hosts size_t is 32-bit so the overflow hasn't been solved. > >=20 > > Does it make sense to impose a limit on pagetable_size? >=20 > Good point. Yes, it does. >=20 > The VHD spec says that the "Max Table Entries" field should be equal > to the disk size / block size. I don't know if there are images out > there that treat that as ">=3D disk size / block size" rather than "=3D= =3D", > however. But if we assume max size of 2TB for a VHD disk, and a > minimal block size of 512 bytes, that would give us a > max_table_entries of 0x100000000, which exceeds 32 bits by itself. >=20 > For pagetable_size to fit in a 32-bit, that means to support 2TB on a > 32-bit host in the current implementation, the minimum block size is > 4096. >=20 > We could check during open / create that=20 > (disk_size / block_size) * 4 < SIZE_MAX, and refuse to open if this is > not true (and also validate max_table_entries to fit in the same). Sounds good. --tcC6YSqBgqqkz7Sb Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVjSHxAAoJEJykq7OBq3PIjNgIAJWH6uHm0VSDzRtKV6Q1qa06 HwjkpDlOn7pKTIcK8sYWeTG7tBQ0Svgq7E7CurJSsUFecO8phh05jIpItgj0BO35 vEYmqoW+FeeIUfrQ0plIa7UzmvKIejAmifcpOD/4ymXC2FltQ+mSF0B3y325BIDt H1KHBtKyDEaI3w5yFlN+92wful5S1iEQcm5zhIzI1JCFyUGJmxQYzlgHyBU808Lh Z2Br9nYaMK3mgJNKbRVfXxCUAfIWkVHkwikf+nbezJHuS0GEUf0383XSVgfSYAS/ juwGxPwN4IEgbXb6ucpXFtnAxdAKgVnrN9Ik3KIKbypUAhumd41sExmg2oOItPk= =4oPh -----END PGP SIGNATURE----- --tcC6YSqBgqqkz7Sb--