diff mbox series

[09/11] cutils: Set value in all integral qemu_strto* error paths

Message ID 20230508200343.791450-10-eblake@redhat.com
State New
Headers show
Series Fix qemu_strtosz() read-out-of-bounds | expand

Commit Message

Eric Blake May 8, 2023, 8:03 p.m. UTC 
Our goal in writing qemu_strtoi() and friends is to have an interface
harder to abuse than libc's strtol().  Leaving the return value
initialized on some error paths does not lend itself well to this
goal; and our documentation wasn't helpful on the matter.

Note that the previous patch changed all qemu_strtosz() EINVAL error
paths to slam value to 0 rather than stay uninitialized, even when the
EINVAL eror occurs because of trailing junk.  But for the remaining
integral qemu_strto*, it's easier to return the parsed value than to
force things back to zero, in part because of how check_strtox_error
works; and doing so creates less churn in the testsuite.

Here, the list of affected callers is much longer ('git grep
"qemu_strto[ui]" *.c **/*.c | grep -v tests/ |wc -l' outputs 87,
although a few of those are the implementation in in cutils.c), so
touching as little as possible is the wisest course of action.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tests/unit/test-cutils.c | 24 +++++++++++------------
 util/cutils.c            | 42 +++++++++++++++++++++++++---------------
 2 files changed, 38 insertions(+), 28 deletions(-)
diff mbox series

Patch

diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 9cf00a810e4..2cb33e41ae4 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -250,7 +250,7 @@  static void test_qemu_strtoi_null(void)
     err = qemu_strtoi(NULL, &endptr, 0, &res);

     g_assert_cmpint(err, ==, -EINVAL);
-    g_assert_cmpint(res, ==, 999);
+    g_assert_cmpint(res, ==, 0);
     g_assert_null(endptr);
 }

@@ -479,7 +479,7 @@  static void test_qemu_strtoi_full_null(void)
     err = qemu_strtoi(NULL, &endptr, 0, &res);

     g_assert_cmpint(err, ==, -EINVAL);
-    g_assert_cmpint(res, ==, 999);
+    g_assert_cmpint(res, ==, 0);
     g_assert_null(endptr);
 }

@@ -557,7 +557,7 @@  static void test_qemu_strtoui_null(void)
     err = qemu_strtoui(NULL, &endptr, 0, &res);

     g_assert_cmpint(err, ==, -EINVAL);
-    g_assert_cmpuint(res, ==, 999);
+    g_assert_cmpuint(res, ==, 0);
     g_assert_null(endptr);
 }

@@ -784,7 +784,7 @@  static void test_qemu_strtoui_full_null(void)
     err = qemu_strtoui(NULL, NULL, 0, &res);

     g_assert_cmpint(err, ==, -EINVAL);
-    g_assert_cmpuint(res, ==, 999);
+    g_assert_cmpuint(res, ==, 0);
 }

 static void test_qemu_strtoui_full_empty(void)
@@ -860,7 +860,7 @@  static void test_qemu_strtol_null(void)
     err = qemu_strtol(NULL, &endptr, 0, &res);

     g_assert_cmpint(err, ==, -EINVAL);
-    g_assert_cmpint(res, ==, 999);
+    g_assert_cmpint(res, ==, 0);
     g_assert_null(endptr);
 }

@@ -1087,7 +1087,7 @@  static void test_qemu_strtol_full_null(void)
     err = qemu_strtol(NULL, &endptr, 0, &res);

     g_assert_cmpint(err, ==, -EINVAL);
-    g_assert_cmpint(res, ==, 999);
+    g_assert_cmpint(res, ==, 0);
     g_assert_null(endptr);
 }

@@ -1165,7 +1165,7 @@  static void test_qemu_strtoul_null(void)
     err = qemu_strtoul(NULL, &endptr, 0, &res);

     g_assert_cmpint(err, ==, -EINVAL);
-    g_assert_cmpuint(res, ==, 999);
+    g_assert_cmpuint(res, ==, 0);
     g_assert_null(endptr);
 }

@@ -1390,7 +1390,7 @@  static void test_qemu_strtoul_full_null(void)
     err = qemu_strtoul(NULL, NULL, 0, &res);

     g_assert_cmpint(err, ==, -EINVAL);
-    g_assert_cmpuint(res, ==, 999);
+    g_assert_cmpuint(res, ==, 0);
 }

 static void test_qemu_strtoul_full_empty(void)
@@ -1466,7 +1466,7 @@  static void test_qemu_strtoi64_null(void)
     err = qemu_strtoi64(NULL, &endptr, 0, &res);

     g_assert_cmpint(err, ==, -EINVAL);
-    g_assert_cmpint(res, ==, 999);
+    g_assert_cmpint(res, ==, 0);
     g_assert_null(endptr);
 }

@@ -1691,7 +1691,7 @@  static void test_qemu_strtoi64_full_null(void)
     err = qemu_strtoi64(NULL, NULL, 0, &res);

     g_assert_cmpint(err, ==, -EINVAL);
-    g_assert_cmpint(res, ==, 999);
+    g_assert_cmpint(res, ==, 0);
 }

 static void test_qemu_strtoi64_full_empty(void)
@@ -1769,7 +1769,7 @@  static void test_qemu_strtou64_null(void)
     err = qemu_strtou64(NULL, &endptr, 0, &res);

     g_assert_cmpint(err, ==, -EINVAL);
-    g_assert_cmpuint(res, ==, 999);
+    g_assert_cmpuint(res, ==, 0);
     g_assert_null(endptr);
 }

@@ -1994,7 +1994,7 @@  static void test_qemu_strtou64_full_null(void)
     err = qemu_strtou64(NULL, NULL, 0, &res);

     g_assert_cmpint(err, ==, -EINVAL);
-    g_assert_cmpuint(res, ==, 999);
+    g_assert_cmpuint(res, ==, 0);
 }

 static void test_qemu_strtou64_full_empty(void)
diff --git a/util/cutils.c b/util/cutils.c
index 8bacf349383..83948926ec9 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -384,12 +384,13 @@  static int check_strtox_error(const char *nptr, char *ep,
  *
  * @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 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 @result, store INT_MAX in @result,
  * and return -ERANGE.
@@ -407,6 +408,7 @@  int qemu_strtoi(const char *nptr, const char **endptr, int base,

     assert((unsigned) base <= 36 && base != 1);
     if (!nptr) {
+        *result = 0;
         if (endptr) {
             *endptr = nptr;
         }
@@ -436,12 +438,13 @@  int qemu_strtoi(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 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 @result, store UINT_MAX in @result,
  * and return -ERANGE.
@@ -460,6 +463,7 @@  int qemu_strtoui(const char *nptr, const char **endptr, int base,

     assert((unsigned) base <= 36 && base != 1);
     if (!nptr) {
+        *result = 0;
         if (endptr) {
             *endptr = nptr;
         }
@@ -495,12 +499,13 @@  int qemu_strtoui(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 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 @result, store LONG_MAX in @result,
  * and return -ERANGE.
@@ -517,6 +522,7 @@  int qemu_strtol(const char *nptr, const char **endptr, int base,

     assert((unsigned) base <= 36 && base != 1);
     if (!nptr) {
+        *result = 0;
         if (endptr) {
             *endptr = nptr;
         }
@@ -537,12 +543,13 @@  int qemu_strtol(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 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 @result, store ULONG_MAX in @result,
  * and return -ERANGE.
@@ -560,6 +567,7 @@  int qemu_strtoul(const char *nptr, const char **endptr, int base,

     assert((unsigned) base <= 36 && base != 1);
     if (!nptr) {
+        *result = 0;
         if (endptr) {
             *endptr = nptr;
         }
@@ -588,6 +596,7 @@  int qemu_strtoi64(const char *nptr, const char **endptr, int base,

     assert((unsigned) base <= 36 && base != 1);
     if (!nptr) {
+        *result = 0;
         if (endptr) {
             *endptr = nptr;
         }
@@ -613,6 +622,7 @@  int qemu_strtou64(const char *nptr, const char **endptr, int base,

     assert((unsigned) base <= 36 && base != 1);
     if (!nptr) {
+        *result = 0;
         if (endptr) {
             *endptr = nptr;
         }