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
next prev 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).