From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54241) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vm2Hx-00031D-VI for qemu-devel@nongnu.org; Thu, 28 Nov 2013 09:10:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vm2Hq-0000GV-UF for qemu-devel@nongnu.org; Thu, 28 Nov 2013 09:09:53 -0500 Received: from cantor2.suse.de ([195.135.220.15]:48804 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vm2Hq-0000GJ-Jp for qemu-devel@nongnu.org; Thu, 28 Nov 2013 09:09:46 -0500 Message-ID: <52974EA6.50301@suse.de> Date: Thu, 28 Nov 2013 15:09:42 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1385627947-23147-1-git-send-email-hare@suse.de> In-Reply-To: <1385627947-23147-1-git-send-email-hare@suse.de> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCHv3] qdev: Validate hex properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hannes Reinecke , Eric Blake Cc: qemu-devel@nongnu.org, Alexander Graf Am 28.11.2013 09:39, schrieb Hannes Reinecke: > strtoul(l) might overflow, in which case it'll return '-1' and set > the appropriate error code. So update the calls to strtoul(l) when > parsing hex properties to avoid silent overflows. >=20 > Signed-off-by: Hannes Reinecke > --- > hw/core/qdev-properties.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c > index dc8ae69..4891a01 100644 > --- a/hw/core/qdev-properties.c > +++ b/hw/core/qdev-properties.c > @@ -191,6 +191,7 @@ PropertyInfo qdev_prop_uint8 =3D { > =20 > static int parse_hex8(DeviceState *dev, Property *prop, const char *st= r) > { > + unsigned long val; > uint8_t *ptr =3D qdev_get_prop_ptr(dev, prop); > char *end; > =20 > @@ -198,11 +199,18 @@ static int parse_hex8(DeviceState *dev, Property = *prop, const char *str) > return -EINVAL; > } > =20 > - *ptr =3D strtoul(str, &end, 16); > + errno =3D 0; > + val =3D strtoul(str, &end, 16); > + if (errno) { > + return -errno; > + } > + if (val > 255) { > + return -ERANGE; > + } > if ((*end !=3D '\0') || (end =3D=3D str)) { > return -EINVAL; > } > - > + *ptr =3D val; > return 0; > } > =20 This part looks okay to me. > @@ -329,7 +337,11 @@ static int parse_hex32(DeviceState *dev, Property = *prop, const char *str) > return -EINVAL; > } > =20 > + errno =3D 0; > *ptr =3D strtoul(str, &end, 16); > + if (errno) { > + return -errno; > + } I can image that on a 64-bit system long can be larger than 32 bits, so we'll need an equivalent val > UINT32_MAX check here, I guess? > if ((*end !=3D '\0') || (end =3D=3D str)) { > return -EINVAL; > } > @@ -396,7 +408,11 @@ static int parse_hex64(DeviceState *dev, Property = *prop, const char *str) > return -EINVAL; > } > =20 > + errno =3D 0; > *ptr =3D strtoull(str, &end, 16); > + if (errno) { > + return -errno; > + } > if ((*end !=3D '\0') || (end =3D=3D str)) { > return -EINVAL; > } Eric, do we have any size guarantee for long long or do we also need a symmetric if (... > UINT64_MAX) { return -ERANGE; } for the unlikely 128-bit case? Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg