From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51153) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VlKMS-0002oC-9g for qemu-devel@nongnu.org; Tue, 26 Nov 2013 10:15:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VlKMN-0005Hu-PU for qemu-devel@nongnu.org; Tue, 26 Nov 2013 10:15:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:3678) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VlKMN-0005Hg-00 for qemu-devel@nongnu.org; Tue, 26 Nov 2013 10:15:31 -0500 Message-ID: <5294BB0F.1000805@redhat.com> Date: Tue, 26 Nov 2013 08:15:27 -0700 From: Eric Blake MIME-Version: 1.0 References: <1385478338-27433-1-git-send-email-hare@suse.de> In-Reply-To: <1385478338-27433-1-git-send-email-hare@suse.de> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="K9pG5tUHhP7thUNWTVB66m5Ef7gto1lbb" Subject: Re: [Qemu-devel] [PATCH] qdev: Validate hex properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hannes Reinecke , Andreas Faerber Cc: qemu-devel@nongnu.org, Alexander Graf This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --K9pG5tUHhP7thUNWTVB66m5Ef7gto1lbb Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 11/26/2013 08:05 AM, Hannes Reinecke wrote: > strtoull() might return '-1' to signify an overflow. Yes, but -1 (aka ULLONG_MAX) is also a valid return when there is not overflow, so testing for -1 is NOT a reliable indicator on whether overflow occurred. Also, you mention strtoull() here... >=20 > Signed-off-by: Hannes Reinecke > --- > hw/core/qdev-properties.c | 9 +++++++++ > 1 file changed, 9 insertions(+) >=20 > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c > index dc8ae69..5761295 100644 > --- a/hw/core/qdev-properties.c > +++ b/hw/core/qdev-properties.c > @@ -202,6 +202,9 @@ static int parse_hex8(DeviceState *dev, Property *p= rop, const char *str) > if ((*end !=3D '\0') || (end =3D=3D str)) { =2E..but this was calling strtoul(). > return -EINVAL; > } > + if (*ptr =3D=3D (uint8_t)-1) { > + return -ERANGE; > + } NAK. This is NOT how you check for overflow with strtoull. Instead, you should use: errno =3D 0; *ptr =3D strtoul(str, &end, 16); if (errno) { return -errno; } if (!*end || end =3D=3D str) { return -EINVAL; } return 0; > =20 > return 0; > } > @@ -333,6 +336,9 @@ static int parse_hex32(DeviceState *dev, Property *= prop, const char *str) > if ((*end !=3D '\0') || (end =3D=3D str)) { > return -EINVAL; > } > + if (*ptr =3D=3D (uint32_t)-1) { > + return -ERANGE; > + } NAK. And this function is even MORE broken. On a 32-bit platform, assigning the results of strtol() into a uint32_t may result in silent truncation. If you are trying to detect overflow, you MUST assign the results into a long, then check that the long does not overflow uint32_t, rather than checking (too late) whether the uint32_t has already been overflowed. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --K9pG5tUHhP7thUNWTVB66m5Ef7gto1lbb Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.15 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJSlLsPAAoJEKeha0olJ0NqpsEH/3nS1SbFzLlmMEw8zMMJcdOV vi+XOpyJ7s1FwdQ4VXgeGS/4Z56fNksMrp4tYeyJWQCpTuH9yAaB9dhqmZaaWGGE WURqVGKUht2Hilsu86TS87kKBnkq7Swod87rtnvFJy+vLC2EszeQF99k8W5K4rIE 9ITx/4gi2KWklCVL9JE5OVybwkhARdLceU3/qvr8XlOooe5qyOYeV4uZgTqYSMm9 zyUBMhAJ9jjx9yHy2XSd95yCDEjOOPm84gz3XwWmRIQDIs4x5+enUm5VEziYl9TQ laEhCc9YEy39pdQgsHQ31jZIXR0MVr75Bb2Xi+tsqlip+JARsW+vFyxZgB7ie8Q= =1bVK -----END PGP SIGNATURE----- --K9pG5tUHhP7thUNWTVB66m5Ef7gto1lbb--