[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 20/21] cutils: Improve qemu_strtod* error paths
From: |
Eric Blake |
Subject: |
[PULL 20/21] cutils: Improve qemu_strtod* error paths |
Date: |
Thu, 1 Jun 2023 17:03:04 -0500 |
Previous patches changed all integral qemu_strto*() error paths to
guarantee that *value is never left uninitialized. Do likewise for
qemu_strtod. Also, tighten qemu_strtod_finite() to never return a
non-finite value (prior to this patch, we were rejecting "inf" with
-EINVAL and unspecified result 0.0, but failing "9e999" with -ERANGE
and HUGE_VAL - which is infinite on IEEE machines - despite our
function claiming to recognize only finite values).
Auditing callers, we have no external callers of qemu_strtod, and
among the callers of qemu_strtod_finite:
- qapi/qobject-input-visitor.c:qobject_input_type_number_keyval() and
qapi/string-input-visitor.c:parse_type_number() which reject all
errors (does not matter what we store)
- utils/cutils.c:do_strtosz() incorrectly assumes that *endptr points
to '.' on all failures (that is, it is not distinguishing between
EINVAL and ERANGE; and therefore still does the WRONG THING for
"9.9e999". The change here does not entirely fix that (a later
patch will tackle this more systematically), but at least it fixes
the read-out-of-bounds first diagnosed in
https://gitlab.com/qemu-project/qemu/-/issues/1629
- our testsuite, which we can update to match what we document
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
CC: qemu-stable@nongnu.org
Message-Id: <20230522190441.64278-19-eblake@redhat.com>
---
tests/unit/test-cutils.c | 63 +++++++++++++++++++++++-----------------
util/cutils.c | 32 +++++++++++---------
2 files changed, 55 insertions(+), 40 deletions(-)
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index c2dbed9eda9..0a589567461 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -2868,7 +2868,8 @@ static void test_qemu_strtod_einval(void)
res = 999;
err = qemu_strtod(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_null(endptr);
/* not recognizable */
@@ -3101,7 +3102,8 @@ static void test_qemu_strtod_finite_einval(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
/* NULL */
@@ -3110,7 +3112,8 @@ static void test_qemu_strtod_finite_einval(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_null(endptr);
/* not recognizable */
@@ -3119,7 +3122,8 @@ static void test_qemu_strtod_finite_einval(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
}
@@ -3130,24 +3134,26 @@ static void test_qemu_strtod_finite_erange(void)
int err;
double res;
- /* overflow */
+ /* overflow turns into EINVAL */
str = "9e999";
endptr = "somewhere";
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
- g_assert_cmpint(err, ==, -ERANGE);
- g_assert_cmpfloat(res, ==, HUGE_VAL);
- g_assert_true(endptr == str + 5);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
+ g_assert_true(endptr == str);
str = "-9e+999";
endptr = "somewhere";
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
- g_assert_cmpint(err, ==, -ERANGE);
- g_assert_cmpfloat(res, ==, -HUGE_VAL);
- g_assert_true(endptr == str + 7);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
+ g_assert_true(endptr == str);
- /* underflow */
+ /* underflow is still possible */
str = "-9e-999";
endptr = "somewhere";
res = 999;
@@ -3172,7 +3178,8 @@ static void test_qemu_strtod_finite_nonfinite(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
str = "-infinity";
@@ -3180,7 +3187,8 @@ static void test_qemu_strtod_finite_nonfinite(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
/* not a number */
@@ -3189,7 +3197,8 @@ static void test_qemu_strtod_finite_nonfinite(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
}
@@ -3213,7 +3222,8 @@ static void test_qemu_strtod_finite_trailing(void)
res = 999;
err = qemu_strtod_finite(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 1.0);
+ g_assert_false(signbit(res));
/* trailing e is not an exponent */
str = ".5e";
@@ -3228,7 +3238,7 @@ static void test_qemu_strtod_finite_trailing(void)
res = 999;
err = qemu_strtod_finite(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.5);
/* trailing ( not part of long NaN */
str = "nan(";
@@ -3236,14 +3246,16 @@ static void test_qemu_strtod_finite_trailing(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
endptr = "somewhere";
res = 999;
err = qemu_strtod_finite(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
}
static void test_qemu_strtod_finite_erange_junk(void)
@@ -3269,7 +3281,8 @@ static void test_qemu_strtod_finite_erange_junk(void)
res = 999;
err = qemu_strtod_finite(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
}
typedef int (*qemu_strtosz_fn)(const char *, const char **, uint64_t *);
@@ -3507,13 +3520,9 @@ static void test_qemu_strtosz_trailing(void)
0 /* FIXME EiB * 1.5 */, 0 /* FIXME 4 */,
-EINVAL, 0);
- /*
- * FIXME overflow in fraction is so buggy it can read beyond bounds
- * if we don't stuff extra \0 in our literal
- */
- do_strtosz_full("1.5E999\0\0" /* FIXME 1.5E999" */, qemu_strtosz,
- 0, 1 /* FIXME EiB * 1.5 */, 8 /* FIXME 4 */,
- 0 /* FIXME -EINVAL */, 1 /* FIXME 0 */);
+ /* FIXME overflow in fraction is still buggy */
+ do_strtosz_full("1.5E999", qemu_strtosz, 0, 1 /* FIXME EiB * 1.5 */,
+ 2 /* FIXME 4 */, -EINVAL, 0);
}
static void test_qemu_strtosz_erange(void)
diff --git a/util/cutils.c b/util/cutils.c
index e3a49209a94..bde2da59bdd 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -660,12 +660,13 @@ int qemu_strtou64(const char *nptr, const char **endptr,
int base,
*
* @nptr may be null, and no conversion is performed then.
*
- * If no conversion is performed, store @nptr in *@endptr and return
- * -EINVAL.
+ * If no conversion is performed, store @nptr in *@endptr, +0.0 in
+ * @result, and return -EINVAL.
*
* If @endptr is null, and the string isn't fully converted, return
- * -EINVAL. This is the case when the pointer that would be stored in
- * a non-null @endptr points to a character other than '\0'.
+ * -EINVAL with @result set to the parsed value. This is the case
+ * when the pointer that would be stored in a non-null @endptr points
+ * to a character other than '\0'.
*
* If the conversion overflows, store +/-HUGE_VAL in @result, depending
* on the sign, and return -ERANGE.
@@ -680,6 +681,7 @@ int qemu_strtod(const char *nptr, const char **endptr,
double *result)
char *ep;
if (!nptr) {
+ *result = 0.0;
if (endptr) {
*endptr = nptr;
}
@@ -694,24 +696,28 @@ int qemu_strtod(const char *nptr, const char **endptr,
double *result)
/**
* Convert string @nptr to a finite double.
*
- * Works like qemu_strtod(), except that "NaN" and "inf" are rejected
- * with -EINVAL and no conversion is performed.
+ * Works like qemu_strtod(), except that "NaN", "inf", and strings
+ * that cause ERANGE overflow errors are rejected with -EINVAL as if
+ * no conversion is performed, storing 0.0 into @result regardless of
+ * any sign. -ERANGE failures for underflow still preserve the parsed
+ * sign.
*/
int qemu_strtod_finite(const char *nptr, const char **endptr, double *result)
{
- double tmp;
+ const char *tmp;
int ret;
- ret = qemu_strtod(nptr, endptr, &tmp);
- if (!ret && !isfinite(tmp)) {
+ ret = qemu_strtod(nptr, &tmp, result);
+ if (!isfinite(*result)) {
if (endptr) {
*endptr = nptr;
}
+ *result = 0.0;
+ ret = -EINVAL;
+ } else if (endptr) {
+ *endptr = tmp;
+ } else if (*tmp) {
ret = -EINVAL;
- }
-
- if (ret != -EINVAL) {
- *result = tmp;
}
return ret;
}
--
2.40.1
- Re: [PULL 07/21] cutils: Fix wraparound parsing in qemu_strtoui, (continued)
- [PULL 09/21] cutils: Adjust signature of parse_uint[_full], Eric Blake, 2023/06/01
- [PULL 18/21] cutils: Set value in all integral qemu_strto* error paths, Eric Blake, 2023/06/01
- [PULL 16/21] test-cutils: Add more coverage to qemu_strtosz, Eric Blake, 2023/06/01
- [PULL 21/21] cutils: Improve qemu_strtosz handling of fractions, Eric Blake, 2023/06/01
- [PULL 04/21] test-cutils: Use g_assert_cmpuint where appropriate, Eric Blake, 2023/06/01
- [PULL 15/21] numa: Check for qemu_strtosz_MiB error, Eric Blake, 2023/06/01
- [PULL 20/21] cutils: Improve qemu_strtod* error paths,
Eric Blake <=
- [PULL 17/21] cutils: Set value in all qemu_strtosz* error paths, Eric Blake, 2023/06/01
- [PULL 06/21] test-cutils: Test more integer corner cases, Eric Blake, 2023/06/01
- [PULL 14/21] cutils: Allow NULL str in qemu_strtosz, Eric Blake, 2023/06/01
- [PULL 19/21] cutils: Use parse_uint in qemu_strtosz for negative rejection, Eric Blake, 2023/06/01
- [PULL 13/21] test-cutils: Refactor qemu_strtosz tests for less boilerplate, Eric Blake, 2023/06/01
- [PULL 12/21] test-cutils: Prepare for upcoming semantic change in qemu_strtosz, Eric Blake, 2023/06/01
- [PULL 03/21] test-cutils: Avoid g_assert in unit tests, Eric Blake, 2023/06/01
- Re: [PULL 00/21] NBD and miscellaneous patches for 2023-06-01, Richard Henderson, 2023/06/01