public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Noah Goldstein <goldstein.w.n@gmail.com>
To: libc-alpha@sourceware.org
Subject: [PATCH v2 1/3] String: Add overflow tests for strnlen, memchr, and strncat [BZ #27974]
Date: Tue, 22 Jun 2021 14:11:10 -0400	[thread overview]
Message-ID: <20210622181111.185897-1-goldstein.w.n@gmail.com> (raw)
In-Reply-To: <20210609205257.123944-1-goldstein.w.n@gmail.com>

This commit adds tests for a bug in the wide char variant of the
functions where the implementation may assume that maxlen for wcsnlen
or n for wmemchr/strncat will not overflow when multiplied by
sizeof(wchar_t).

These tests show the following implementations failing on x86_64:

wcsnlen-sse4_1
wcsnlen-avx2

wmemchr-sse2
wmemchr-avx2

strncat would fail as well if it where on a system that prefered
either of the wcsnlen implementations that failed as it relies on
wcsnlen.

Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
---
Some notes:

I only tested this patch (and the subsequent fixes) on a machine that
prefers EVEX.

The fix for wcsnlen-sse2 is possibly invalid. What it does is checks
if the computation is maxlen * sizeof(wchar_t) + s overflows, and if
so just calls wcslen. The rational is that either the end of the
string will be found in readable memory or the user invoked UB by
calling wcsnlen on a string that is not contained in valid memory
and without a maxlen to that will bound it in valid memory.

 string/test-memchr.c  | 39 ++++++++++++++++++++++++---
 string/test-strncat.c | 61 +++++++++++++++++++++++++++++++++++++++++++
 string/test-strnlen.c | 33 +++++++++++++++++++++++
 3 files changed, 130 insertions(+), 3 deletions(-)

diff --git a/string/test-memchr.c b/string/test-memchr.c
index 665edc32af..ce964284aa 100644
--- a/string/test-memchr.c
+++ b/string/test-memchr.c
@@ -65,8 +65,8 @@ do_one_test (impl_t *impl, const CHAR *s, int c, size_t n, CHAR *exp_res)
   CHAR *res = CALL (impl, s, c, n);
   if (res != exp_res)
     {
-      error (0, 0, "Wrong result in function %s %p %p", impl->name,
-	     res, exp_res);
+      error (0, 0, "Wrong result in function %s (%p, %d, %zu) -> %p != %p",
+             impl->name, s, c, n, res, exp_res);
       ret = 1;
       return;
     }
@@ -91,7 +91,7 @@ do_test (size_t align, size_t pos, size_t len, size_t n, int seek_char)
     }
   buf[align + len] = 0;
 
-  if (pos < len)
+  if (pos < MIN(n, len))
     {
       buf[align + pos] = seek_char;
       buf[align + len] = -seek_char;
@@ -107,6 +107,38 @@ do_test (size_t align, size_t pos, size_t len, size_t n, int seek_char)
     do_one_test (impl, (CHAR *) (buf + align), seek_char, n, result);
 }
 
+static void
+do_overflow_tests (void)
+{
+  size_t i, j, len;
+  const size_t one = 1;
+  uintptr_t buf_addr = (uintptr_t) buf1;
+
+  for (i = 0; i < 750; ++i)
+    {
+        do_test (0, i, 751, SIZE_MAX - i, BIG_CHAR);
+        do_test (0, i, 751, i - buf_addr, BIG_CHAR);
+        do_test (0, i, 751, -buf_addr - i, BIG_CHAR);
+        do_test (0, i, 751, SIZE_MAX - buf_addr - i, BIG_CHAR);
+        do_test (0, i, 751, SIZE_MAX - buf_addr + i, BIG_CHAR);
+
+      len = 0;
+      for (j = 8 * sizeof(size_t) - 1; j ; --j)
+        {
+          len |= one << j;
+          do_test (0, i, 751, len - i, BIG_CHAR);
+          do_test (0, i, 751, len + i, BIG_CHAR);
+          do_test (0, i, 751, len - buf_addr - i, BIG_CHAR);
+          do_test (0, i, 751, len - buf_addr + i, BIG_CHAR);
+
+          do_test (0, i, 751, ~len - i, BIG_CHAR);
+          do_test (0, i, 751, ~len + i, BIG_CHAR);
+          do_test (0, i, 751, ~len - buf_addr - i, BIG_CHAR);
+          do_test (0, i, 751, ~len - buf_addr + i, BIG_CHAR);
+        }
+    }
+}
+
 static void
 do_random_tests (void)
 {
@@ -221,6 +253,7 @@ test_main (void)
     do_test (page_size / 2 - i, i, i, 1, 0x9B);
 
   do_random_tests ();
+  do_overflow_tests ();
   return ret;
 }
 
diff --git a/string/test-strncat.c b/string/test-strncat.c
index 2ef917b820..37ea26ea05 100644
--- a/string/test-strncat.c
+++ b/string/test-strncat.c
@@ -134,6 +134,66 @@ do_test (size_t align1, size_t align2, size_t len1, size_t len2,
     }
 }
 
+static void
+do_overflow_tests (void)
+{
+  size_t i, j, len;
+  const size_t one = 1;
+  CHAR *s1, *s2;
+  uintptr_t s1_addr;
+  s1 = (CHAR *) buf1;
+  s2 = (CHAR *) buf2;
+  s1_addr = (uintptr_t)s1;
+ for (j = 0; j < 200; ++j)
+      s2[j] = 32 + 23 * j % (BIG_CHAR - 32);
+ s2[200] = 0;
+  for (i = 0; i < 750; ++i) {
+    for (j = 0; j < i; ++j)
+      s1[j] = 32 + 23 * j % (BIG_CHAR - 32);
+    s1[i] = '\0';
+
+       FOR_EACH_IMPL (impl, 0)
+    {
+      s2[200] = '\0';
+      do_one_test (impl, s2, s1, SIZE_MAX - i);
+      s2[200] = '\0';
+      do_one_test (impl, s2, s1, i - s1_addr);
+      s2[200] = '\0';
+      do_one_test (impl, s2, s1, -s1_addr - i);
+      s2[200] = '\0';
+      do_one_test (impl, s2, s1, SIZE_MAX - s1_addr - i);
+      s2[200] = '\0';
+      do_one_test (impl, s2, s1, SIZE_MAX - s1_addr + i);
+    }
+
+    len = 0;
+    for (j = 8 * sizeof(size_t) - 1; j ; --j)
+      {
+        len |= one << j;
+        FOR_EACH_IMPL (impl, 0)
+          {
+            s2[200] = '\0';
+            do_one_test (impl, s2, s1, len - i);
+            s2[200] = '\0';
+            do_one_test (impl, s2, s1, len + i);
+            s2[200] = '\0';
+            do_one_test (impl, s2, s1, len - s1_addr - i);
+            s2[200] = '\0';
+            do_one_test (impl, s2, s1, len - s1_addr + i);
+
+            s2[200] = '\0';
+            do_one_test (impl, s2, s1, ~len - i);
+            s2[200] = '\0';
+            do_one_test (impl, s2, s1, ~len + i);
+            s2[200] = '\0';
+            do_one_test (impl, s2, s1, ~len - s1_addr - i);
+            s2[200] = '\0';
+            do_one_test (impl, s2, s1, ~len - s1_addr + i);
+          }
+      }
+  }
+}
+
 static void
 do_random_tests (void)
 {
@@ -316,6 +376,7 @@ test_main (void)
     }
 
   do_random_tests ();
+  do_overflow_tests ();
   return ret;
 }
 
diff --git a/string/test-strnlen.c b/string/test-strnlen.c
index 920f58e97b..f53e09263f 100644
--- a/string/test-strnlen.c
+++ b/string/test-strnlen.c
@@ -89,6 +89,38 @@ do_test (size_t align, size_t len, size_t maxlen, int max_char)
     do_one_test (impl, (CHAR *) (buf + align), maxlen, MIN (len, maxlen));
 }
 
+static void
+do_overflow_tests (void)
+{
+  size_t i, j, len;
+  const size_t one = 1;
+  uintptr_t buf_addr = (uintptr_t) buf1;
+
+  for (i = 0; i < 750; ++i)
+    {
+      do_test (0, i, SIZE_MAX - i, BIG_CHAR);
+      do_test (0, i, i - buf_addr, BIG_CHAR);
+      do_test (0, i, -buf_addr - i, BIG_CHAR);
+      do_test (0, i, SIZE_MAX - buf_addr - i, BIG_CHAR);
+      do_test (0, i, SIZE_MAX - buf_addr + i, BIG_CHAR);
+
+      len = 0;
+      for (j = 8 * sizeof(size_t) - 1; j ; --j)
+        {
+          len |= one << j;
+          do_test (0, i, len - i, BIG_CHAR);
+          do_test (0, i, len + i, BIG_CHAR);
+          do_test (0, i, len - buf_addr - i, BIG_CHAR);
+          do_test (0, i, len - buf_addr + i, BIG_CHAR);
+
+          do_test (0, i, ~len - i, BIG_CHAR);
+          do_test (0, i, ~len + i, BIG_CHAR);
+          do_test (0, i, ~len - buf_addr - i, BIG_CHAR);
+          do_test (0, i, ~len - buf_addr + i, BIG_CHAR);
+        }
+    }
+}
+
 static void
 do_random_tests (void)
 {
@@ -283,6 +315,7 @@ test_main (void)
   do_random_tests ();
   do_page_tests ();
   do_page_2_tests ();
+  do_overflow_tests ();
   return ret;
 }
 
-- 
2.25.1


  parent reply	other threads:[~2021-06-22 18:20 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-09 20:52 [PATCH v1 1/3] String: Add additional overflow tests for strnlen, memchr, and strncat Noah Goldstein
2021-06-09 20:52 ` [PATCH v1 2/3] x86: Fix overflow bug with wmemchr-sse2 and wmemchr-avx2 Noah Goldstein
2021-06-09 20:52 ` [PATCH v1 3/3] x86: Fix overflow bug in wcsnlen-sse4_1 and wcsnlen-avx2 Noah Goldstein
2021-06-09 21:53 ` [PATCH v1 1/3] String: Add additional overflow tests for strnlen, memchr, and strncat H.J. Lu
2021-06-09 22:26   ` Noah Goldstein
2021-06-22 15:43     ` Noah Goldstein
2021-06-22 16:18       ` H.J. Lu
2021-06-22 18:23         ` Noah Goldstein
2021-06-22 18:11 ` Noah Goldstein [this message]
2021-06-22 21:24   ` [PATCH v2 1/3] String: Add overflow tests for strnlen, memchr, and strncat [BZ #27974] H.J. Lu
2021-06-22 18:11 ` [PATCH v2 2/3] x86: Fix overflow bug with wmemchr-sse2 and wmemchr-avx2 " Noah Goldstein
2021-06-22 21:24   ` H.J. Lu
2021-06-22 18:11 ` [PATCH v2 3/3] x86: Fix overflow bug in wcsnlen-sse4_1 and wcsnlen-avx2 " Noah Goldstein
2021-06-22 21:33   ` H.J. Lu
2021-06-22 23:16     ` Noah Goldstein
2021-06-22 23:28       ` H.J. Lu
2021-06-23  3:11         ` Noah Goldstein
2021-06-23  3:58           ` H.J. Lu
2021-06-23  4:55             ` Noah Goldstein
2021-06-23  6:31 ` [PATCH v3 1/3] String: Add overflow tests for strnlen, memchr, and strncat " Noah Goldstein
2021-06-23 17:30   ` H.J. Lu
2021-06-23 18:30     ` Noah Goldstein
2022-01-27 21:06       ` H.J. Lu
2021-06-23  6:31 ` [PATCH v3 2/3] x86: Fix overflow bug with wmemchr-sse2 and wmemchr-avx2 " Noah Goldstein
2021-06-23 17:30   ` H.J. Lu
2021-06-23  6:31 ` [PATCH v3 3/3] x86: Fix overflow bug in wcsnlen-sse4_1 and wcsnlen-avx2 " Noah Goldstein
2021-06-23 17:27   ` H.J. Lu

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=20210622181111.185897-1-goldstein.w.n@gmail.com \
    --to=goldstein.w.n@gmail.com \
    --cc=libc-alpha@sourceware.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 read-only IMAP folder(s) and NNTP newsgroup(s).