All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: hreitz@redhat.com, qemu-block@nongnu.org
Subject: [PATCH v3 14/19] test-cutils: Add more coverage to qemu_strtosz
Date: Mon, 22 May 2023 14:04:36 -0500	[thread overview]
Message-ID: <20230522190441.64278-15-eblake@redhat.com> (raw)
In-Reply-To: <20230522190441.64278-1-eblake@redhat.com>

Add some more strings that the user might send our way.  In
particular, some of these additions include FIXME comments showing
where our parser doesn't quite behave the way we want.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
---

v3: Hoist hunk touching do_strtosz_full to earlier patch [Hanna], fix
commit message typo [Hanna], R-b added
---
 tests/unit/test-cutils.c | 140 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 129 insertions(+), 11 deletions(-)

diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index becac209987..84628b8880b 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -3326,8 +3326,8 @@ static void test_qemu_strtosz_simple(void)
     /* Leading 0 gives decimal results, not octal */
     do_strtosz("08", 0, 8, 2);

-    /* Leading space is ignored */
-    do_strtosz(" 12345", 0, 12345, 6);
+    /* Leading space and + are ignored */
+    do_strtosz(" +12345", 0, 12345, 7);

     /* 2^53-1 */
     do_strtosz("9007199254740991", 0, 0x1fffffffffffffULL, 16);
@@ -3354,17 +3354,27 @@ static void test_qemu_strtosz_hex(void)

     do_strtosz("0xab", 0, 171, 4);

-    do_strtosz("0xae", 0, 174, 4);
+    do_strtosz(" +0xae", 0, 174, 6);
 }

 static void test_qemu_strtosz_units(void)
 {
-    /* default is M */
+    /* default scale depends on function */
+    do_strtosz("1", 0, 1, 1);
     do_strtosz_MiB("1", 0, MiB, 1);
+    do_strtosz_metric("1", 0, 1, 1);

+    /* Explicit byte suffix works for all functions */
     do_strtosz("1B", 0, 1, 2);
+    do_strtosz_MiB("1B", 0, 1, 2);
+    do_strtosz_metric("1B", 0, 1, 2);

+    /* Expose the scale */
     do_strtosz("1K", 0, KiB, 2);
+    do_strtosz_MiB("1K", 0, KiB, 2);
+    do_strtosz_metric("1K", 0, 1000, 2);
+
+    /* Other suffixes, see also test_qemu_strtosz_metric */
     do_strtosz("1M", 0, MiB, 2);
     do_strtosz("1G", 0, GiB, 2);
     do_strtosz("1T", 0, TiB, 2);
@@ -3376,14 +3386,37 @@ static void test_qemu_strtosz_float(void)
 {
     do_strtosz("0.5E", 0, EiB / 2, 4);

+    /* Implied M suffix okay */
+    do_strtosz_MiB("0.5", 0, MiB / 2, 3);
+
     /* For convenience, a fraction of 0 is tolerated even on bytes */
     do_strtosz("1.0B", 0, 1, 4);

-    /* An empty fraction is tolerated */
+    /* An empty fraction tail is tolerated */
     do_strtosz("1.k", 0, 1024, 3);

+    /* FIXME An empty fraction head should be tolerated */
+    do_strtosz(" .5k", -EINVAL /* FIXME 0 */, 0xbaadf00d /* FIXME 512 */,
+               0 /* FIXME 4 */);
+
     /* For convenience, we permit values that are not byte-exact */
     do_strtosz("12.345M", 0, (uint64_t) (12.345 * MiB + 0.5), 7);
+
+    /* FIXME Fraction tail should round correctly */
+    do_strtosz("1.9999k", 0, 2048, 7);
+    do_strtosz("1.9999999999999999999999999999999999999999999999999999k", 0,
+               1024 /* FIXME 2048 */, 55);
+
+    /* FIXME ERANGE underflow in the fraction tail should not matter for 'k' */
+    do_strtosz("1."
+               "00000000000000000000000000000000000000000000000000"
+               "00000000000000000000000000000000000000000000000000"
+               "00000000000000000000000000000000000000000000000000"
+               "00000000000000000000000000000000000000000000000000"
+               "00000000000000000000000000000000000000000000000000"
+               "00000000000000000000000000000000000000000000000000"
+               "00000000000000000000000000000000000000000000000000"
+               "1k", 0, 1 /* FIXME 1024 */, 354);
 }

 static void test_qemu_strtosz_invalid(void)
@@ -3393,57 +3426,142 @@ static void test_qemu_strtosz_invalid(void)
     /* Must parse at least one digit */
     do_strtosz("", -EINVAL, 0xbaadf00d, 0);
     do_strtosz(" \t ", -EINVAL, 0xbaadf00d, 0);
-    do_strtosz("crap", -EINVAL, 0xbaadf00d, 0);
+    do_strtosz(".", -EINVAL, 0xbaadf00d, 0);
+    do_strtosz(" .", -EINVAL, 0xbaadf00d, 0);
+    do_strtosz(" .k", -EINVAL, 0xbaadf00d, 0);
     do_strtosz("inf", -EINVAL, 0xbaadf00d, 0);
     do_strtosz("NaN", -EINVAL, 0xbaadf00d, 0);

+    /* Lone suffix is not okay */
+    do_strtosz("k", -EINVAL, 0xbaadf00d, 0);
+    do_strtosz(" M", -EINVAL, 0xbaadf00d, 0);
+
     /* Fractional values require scale larger than bytes */
     do_strtosz("1.1B", -EINVAL, 0xbaadf00d, 0);
     do_strtosz("1.1", -EINVAL, 0xbaadf00d, 0);

+    /* FIXME underflow in the fraction tail should matter for 'B' */
+    do_strtosz("1.00001B", -EINVAL, 0xbaadf00d, 0);
+    do_strtosz("1.00000000000000000001B", 0 /* FIXME -EINVAL */,
+               1 /* FIXME 0xbaadf00d */, 23 /* FIXME 0 */);
+    do_strtosz("1."
+               "00000000000000000000000000000000000000000000000000"
+               "00000000000000000000000000000000000000000000000000"
+               "00000000000000000000000000000000000000000000000000"
+               "00000000000000000000000000000000000000000000000000"
+               "00000000000000000000000000000000000000000000000000"
+               "00000000000000000000000000000000000000000000000000"
+               "00000000000000000000000000000000000000000000000000"
+               "1B", 0 /* FIXME -EINVAL */, 1 /* FIXME 0xbaadf00d */,
+               354 /* FIXME 0 */);
+
     /* No hex fractions */
     do_strtosz("0x1.8k", -EINVAL, 0xbaadf00d, 0);
+    do_strtosz("0x1.k", -EINVAL, 0xbaadf00d, 0);

-    /* No suffixes */
+    /* No hex suffixes */
     do_strtosz("0x18M", -EINVAL, 0xbaadf00d, 0);
+    do_strtosz("0x1p1", -EINVAL, 0xbaadf00d, 0);

-    /* No negative values */
-    do_strtosz("-0", -EINVAL, 0xbaadf00d, 0);
-    do_strtosz("-1", -EINVAL, 0xbaadf00d, 0);
+    /* decimal in place of scaling suffix */
+    do_strtosz("1.1.k", -EINVAL, 0xbaadf00d, 0);
+    do_strtosz("1.1.", -EINVAL, 0xbaadf00d, 0);
 }

 static void test_qemu_strtosz_trailing(void)
 {
+    /* Trailing whitespace */
+    do_strtosz_full("1k ", qemu_strtosz, 0, 1024, 2, -EINVAL, 0xbaadf00d);
+
+    /* Unknown suffix overrides even implied scale*/
+    do_strtosz_full("123xxx", qemu_strtosz, 0, 123, 3, -EINVAL, 0xbaadf00d);
+
+    /* Implied scale allows partial parse */
     do_strtosz_full("123xxx", qemu_strtosz_MiB, 0, 123 * MiB, 3,
                     -EINVAL, 0xbaadf00d);
+    do_strtosz_full("1.5.k", qemu_strtosz_MiB, 0, 1.5 * MiB, 3,
+                    -EINVAL, 0xbaadf00d);

+    /* Junk after one-byte suffix */
     do_strtosz_full("1kiB", qemu_strtosz, 0, 1024, 2, -EINVAL, 0xbaadf00d);
+
+    /* Incomplete hex is an unknown suffix */
     do_strtosz_full("0x", qemu_strtosz, 0, 0, 1, -EINVAL, 0xbaadf00d);
+
+    /* Hex literals use only one leading zero */
+    do_strtosz_full("00x1", qemu_strtosz, 0, 0, 2, -EINVAL, 0xbaadf00d);
+
+    /* No support for binary literals; 'b' is valid suffix */
+    do_strtosz_full("0b1000", qemu_strtosz, 0, 0, 2, -EINVAL, 0xbaadf00d);
+
+    /* Junk after decimal */
     do_strtosz_full("0.NaN", qemu_strtosz, 0, 0, 2, -EINVAL, 0xbaadf00d);
+
+    /* Although negatives are invalid, '-' may be in trailing junk */
     do_strtosz_full("123-45", qemu_strtosz, 0, 123, 3, -EINVAL, 0xbaadf00d);
+    do_strtosz_full(" 123 - 45", qemu_strtosz, 0, 123, 4, -EINVAL, 0xbaadf00d);

     /* FIXME should stop parse after 'e'. No floating point exponents */
     do_strtosz_full("1.5e1k", qemu_strtosz, -EINVAL /* FIXME 0 */,
                     0xbaadf00d /* FIXME EiB * 1.5 */, 0 /* FIXME 4 */,
                     -EINVAL, 0xbaadf00d);
-
     do_strtosz_full("1.5E+0k", qemu_strtosz, -EINVAL /* FIXME 0 */,
                     0xbaadf00d /* FIXME EiB * 1.5 */, 0 /* FIXME 4 */,
                     -EINVAL, 0xbaadf00d);
+
+    /*
+     * 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 0xbaadf00d */);
 }

 static void test_qemu_strtosz_erange(void)
 {
+    /* FIXME negative values fit better as ERANGE */
+    do_strtosz(" -0", -EINVAL /* FIXME -ERANGE */, 0xbaadf00d, 0 /* FIXME 3 */);
+    do_strtosz("-1", -EINVAL /* FIXME -ERANGE */, 0xbaadf00d, 0 /* FIXME 2 */);
+    do_strtosz_full("-2M", qemu_strtosz, -EINVAL /* FIXME -ERANGE */,
+                    0xbaadf00d, 0 /* FIXME 2 */, -EINVAL, 0xbaadf00d);
+    do_strtosz(" -.0", -EINVAL /* FIXME -ERANGE */, 0xbaadf00d,
+               0 /* FIXME 4 */);
+    do_strtosz_full("-.1k", qemu_strtosz, -EINVAL /* FIXME -ERANGE */,
+                    0xbaadf00d, 0 /* FIXME 3 */, -EINVAL, 0xbaadf00d);
+    do_strtosz_full(" -."
+                    "00000000000000000000000000000000000000000000000000"
+                    "00000000000000000000000000000000000000000000000000"
+                    "00000000000000000000000000000000000000000000000000"
+                    "00000000000000000000000000000000000000000000000000"
+                    "00000000000000000000000000000000000000000000000000"
+                    "00000000000000000000000000000000000000000000000000"
+                    "00000000000000000000000000000000000000000000000000"
+                    "1M", qemu_strtosz, -EINVAL /* FIXME -ERANGE */,
+                    0xbaadf00d, 0 /* FIXME 354 */, -EINVAL, 0xbaadf00d);
+
     /* 2^64; see strtosz_simple for 2^64-1 */
     do_strtosz("18446744073709551616", -ERANGE, 0xbaadf00d, 20);

     do_strtosz("20E", -ERANGE, 0xbaadf00d, 3);
+
+    /* FIXME Fraction tail can cause ERANGE overflow */
+    do_strtosz("15.9999999999999999999999999999999999999999999999999999E",
+               0 /* FIXME -ERANGE */, 15ULL * EiB /* FIXME 0xbaadf00d */, 56);
+
+    /* EINVAL has priority over ERANGE */
+    do_strtosz_full("100000Pjunk", qemu_strtosz, -ERANGE, 0xbaadf00d, 7,
+                    -EINVAL, 0xbaadf00d);
 }

 static void test_qemu_strtosz_metric(void)
 {
     do_strtosz_metric("12345k", 0, 12345000, 6);
     do_strtosz_metric("12.345M", 0, 12345000, 7);
+
+    /* Fraction is affected by floating-point rounding */
+    /* This would be 0xfffffffffffffbff with infinite precision */
+    do_strtosz_metric("18.446744073709550591E", 0, 0xfffffffffffffc0cULL, 22);
 }

 static void test_freq_to_str(void)
-- 
2.40.1



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

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-22 19:04 [PATCH v3 00/19] Fix qemu_strtosz() read-out-of-bounds Eric Blake
2023-05-22 19:04 ` [PATCH v3 01/19] test-cutils: Avoid g_assert in unit tests Eric Blake
2023-05-22 19:04 ` [PATCH v3 02/19] test-cutils: Use g_assert_cmpuint where appropriate Eric Blake
2023-05-22 19:04 ` [PATCH v3 03/19] test-cutils: Test integral qemu_strto* value on failures Eric Blake
2023-05-22 19:04 ` [PATCH v3 04/19] test-cutils: Test more integer corner cases Eric Blake
2023-05-23 17:25   ` Hanna Czenczek
2023-05-23 19:19     ` Eric Blake
2023-05-22 19:04 ` [PATCH v3 05/19] cutils: Fix wraparound parsing in qemu_strtoui Eric Blake
2023-05-23 17:36   ` Hanna Czenczek
2023-05-22 19:04 ` [PATCH v3 06/19] cutils: Document differences between parse_uint and qemu_strtou64 Eric Blake
2023-05-22 19:04 ` [PATCH v3 07/19] cutils: Adjust signature of parse_uint[_full] Eric Blake
2023-05-22 19:04 ` [PATCH v3 08/19] cutils: Allow NULL endptr in parse_uint() Eric Blake
2023-05-22 19:04 ` [PATCH v3 09/19] test-cutils: Add coverage of qemu_strtod Eric Blake
2023-05-22 19:04 ` [PATCH v3 10/19] test-cutils: Prepare for upcoming semantic change in qemu_strtosz Eric Blake
2023-05-22 19:04 ` [PATCH v3 11/19] test-cutils: Refactor qemu_strtosz tests for less boilerplate Eric Blake
2023-05-22 19:04 ` [PATCH v3 12/19] cutils: Allow NULL str in qemu_strtosz Eric Blake
2023-05-22 19:04 ` [PATCH v3 13/19] numa: Check for qemu_strtosz_MiB error Eric Blake
2023-05-22 19:04 ` Eric Blake [this message]
2023-05-22 19:04 ` [PATCH v3 15/19] cutils: Set value in all qemu_strtosz* error paths Eric Blake
2023-05-22 19:04 ` [PATCH v3 16/19] cutils: Set value in all integral qemu_strto* " Eric Blake
2023-05-22 19:04 ` [PATCH v3 17/19] cutils: Use parse_uint in qemu_strtosz for negative rejection Eric Blake
2023-05-22 19:04 ` [PATCH v3 18/19] cutils: Improve qemu_strtod* error paths Eric Blake
2023-05-22 19:04 ` [PATCH v3 19/19] cutils: Improve qemu_strtosz handling of fractions Eric Blake
2023-06-01 21:46   ` Eric Blake
2023-06-01 21:28 ` [PATCH v3 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=20230522190441.64278-15-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.