qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: hreitz@redhat.com, armbru@redhat.com, richard.henderson@linaro.org
Subject: [PATCH v2 05/19] cutils: Fix wraparound parsing in qemu_strtoui
Date: Thu, 11 May 2023 21:10:19 -0500	[thread overview]
Message-ID: <20230512021033.1378730-6-eblake@redhat.com> (raw)
In-Reply-To: <20230512021033.1378730-1-eblake@redhat.com>

While we were matching 32-bit strtol in qemu_strtoi, our use of a
64-bit parse was leaking through for some inaccurate answers in
qemu_strtoui in comparison to a 32-bit strtoul.  Fix those, and update
the testsuite now that our bounds checks are correct.

Our int wrappers would be a lot easier to write if libc had a
guaranteed 32-bit parser even on platforms with 64-bit long.

Fixes: 473a2a331e ("cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int types", v2.12.0)
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tests/unit/test-cutils.c | 11 +++++------
 util/cutils.c            | 14 ++++++++++----
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 89c10f5307a..08989d1d3ac 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -858,7 +858,7 @@ static void test_qemu_strtoui_hex(void)

 static void test_qemu_strtoui_wrap(void)
 {
-    /* FIXME - wraparound should be consistent with 32-bit strtoul */
+    /* wraparound is consistent with 32-bit strtoul */
     const char *str = "-4294967295"; /* 1 mod 2^32 */
     char f = 'X';
     const char *endptr = &f;
@@ -867,8 +867,8 @@ static void test_qemu_strtoui_wrap(void)

     err = qemu_strtoui(str, &endptr, 0, &res);

-    g_assert_cmpint(err, ==, -ERANGE /* FIXME 0 */);
-    g_assert_cmphex(res, ==, UINT_MAX /* FIXME 1 */);
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmphex(res, ==, 1);
     g_assert_true(endptr == str + strlen(str));
 }

@@ -935,13 +935,12 @@ static void test_qemu_strtoui_underflow(void)
     g_assert_cmpint(res, ==, UINT_MAX);
     g_assert_true(endptr == str + strlen(str));

-    /* FIXME - overflow should be consistent with 32-bit strtoul */
     str = "-18446744073709551615"; /* -UINT64_MAX (not 1) */
     endptr = "somewhere";
     res = 999;
     err = qemu_strtoui(str, &endptr, 0, &res);
-    g_assert_cmpint(err, ==, 0 /* FIXME -ERANGE */);
-    g_assert_cmpint(res, ==, 1 /* FIXME UINT_MAX */);
+    g_assert_cmpint(err, ==, -ERANGE);
+    g_assert_cmpint(res, ==, UINT_MAX);
     g_assert_true(endptr == str + strlen(str));

     str = "-0x10000000000000000"; /* 65 bits, 32-bit sign bit clear */
diff --git a/util/cutils.c b/util/cutils.c
index 5887e744140..997ddcd09e5 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -466,10 +466,16 @@ int qemu_strtoui(const char *nptr, const char **endptr, int base,
     if (errno == ERANGE) {
         *result = -1;
     } else {
-        if (lresult > UINT_MAX) {
-            *result = UINT_MAX;
-            errno = ERANGE;
-        } else if (lresult < INT_MIN) {
+        /*
+         * Note that platforms with 32-bit strtoul accept input in the
+         * range [-4294967295, 4294967295]; but we used 64-bit
+         * strtoull which wraps -18446744073709551615 to 1.  Reject
+         * positive values that contain '-', and wrap all valid
+         * negative values.
+         */
+        if (lresult > UINT_MAX ||
+            lresult < -(long long)UINT_MAX ||
+            (lresult > 0 && memchr(nptr, '-', ep - nptr))) {
             *result = UINT_MAX;
             errno = ERANGE;
         } else {
-- 
2.40.1



  parent reply	other threads:[~2023-05-12  2:12 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-12  2:10 [PATCH v2 00/19] Fix qemu_strtosz() read-out-of-bounds Eric Blake
2023-05-12  2:10 ` [PATCH v2 01/19] test-cutils: Avoid g_assert in unit tests Eric Blake
2023-05-12  3:20   ` Philippe Mathieu-Daudé
2023-05-12 12:11   ` Eric Blake
2023-05-12  2:10 ` [PATCH v2 02/19] test-cutils: Use g_assert_cmpuint where appropriate Eric Blake
2023-05-12  2:10 ` [PATCH v2 03/19] test-cutils: Test integral qemu_strto* value on failures Eric Blake
2023-05-12  2:10 ` [PATCH v2 04/19] test-cutils: Test more integer corner cases Eric Blake
2023-05-19 14:27   ` Hanna Czenczek
2023-05-19 15:17     ` Eric Blake
2023-05-12  2:10 ` Eric Blake [this message]
2023-05-18 13:34   ` [PATCH v2 05/19] cutils: Fix wraparound parsing in qemu_strtoui Eric Blake
2023-05-19 14:42   ` Hanna Czenczek
2023-05-19 16:31     ` Eric Blake
2023-05-12  2:10 ` [PATCH v2 06/19] cutils: Document differences between parse_uint and qemu_strtou64 Eric Blake
2023-05-19 14:44   ` Hanna Czenczek
2023-05-12  2:10 ` [PATCH v2 07/19] cutils: Adjust signature of parse_uint[_full] Eric Blake
2023-05-12 16:25   ` Eric Blake
2023-05-19 14:51   ` Hanna Czenczek
2023-05-12  2:10 ` [PATCH v2 08/19] cutils: Allow NULL endptr in parse_uint() Eric Blake
2023-05-12 16:44   ` Eric Blake
2023-05-19 14:54   ` Hanna Czenczek
2023-05-12  2:10 ` [PATCH v2 09/19] test-cutils: Add coverage of qemu_strtod Eric Blake
2023-05-19 15:05   ` Hanna Czenczek
2023-05-19 17:52     ` Eric Blake
2023-05-22 10:56       ` Hanna Czenczek
2023-05-22 12:59         ` Eric Blake
2023-05-12  2:10 ` [PATCH v2 10/19] test-cutils: Prepare for upcoming semantic change in qemu_strtosz Eric Blake
2023-05-12  2:10 ` [PATCH v2 11/19] test-cutils: Refactor qemu_strtosz tests for less boilerplate Eric Blake
2023-05-19 15:13   ` Hanna Czenczek
2023-05-19 17:54     ` Eric Blake
2023-05-12  2:10 ` [PATCH v2 12/19] cutils: Allow NULL str in qemu_strtosz Eric Blake
2023-05-12  3:25   ` Philippe Mathieu-Daudé
2023-05-19 15:15   ` Hanna Czenczek
2023-05-12  2:10 ` [PATCH v2 13/19] numa: Check for qemu_strtosz_MiB error Eric Blake
2023-05-12  2:10 ` [PATCH v2 14/19] test-cutils: Add more coverage to qemu_strtosz11; rgb:1e1e/1e1e/1e1e Eric Blake
2023-05-19 15:26   ` [PATCH v2 14/19] test-cutils: Add more coverage to qemu_strtosz11;rgb:1e1e/1e1e/1e1e Hanna Czenczek
2023-05-19 18:02     ` Eric Blake
2023-05-12  2:10 ` [PATCH v2 15/19] cutils: Set value in all qemu_strtosz* error paths Eric Blake
2023-05-19 15:29   ` Hanna Czenczek
2023-05-12  2:10 ` [PATCH v2 16/19] cutils: Set value in all integral qemu_strto* " Eric Blake
2023-05-12  2:10 ` [PATCH v2 17/19] cutils: Use parse_uint in qemu_strtosz for negative rejection Eric Blake
2023-05-12 19:34   ` Eric Blake
2023-05-19 15:32     ` Hanna Czenczek
2023-05-12  2:10 ` [PATCH v2 18/19] cutils: Improve qemu_strtod* error paths Eric Blake
2023-05-18 13:47   ` Eric Blake
2023-05-12  2:10 ` [PATCH v2 19/19] cutils: Improve qemu_strtosz handling of fractions Eric Blake
2023-05-19 15:36   ` Hanna Czenczek
2023-05-12 12:24 ` [PATCH v2 00/19] Fix qemu_strtosz() read-out-of-bounds Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230512021033.1378730-6-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).