From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x529.google.com (mail-pg1-x529.google.com [IPv6:2607:f8b0:4864:20::529]) by sourceware.org (Postfix) with ESMTPS id 6EAF6385742A for ; Tue, 22 Jun 2021 15:43:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6EAF6385742A Received: by mail-pg1-x529.google.com with SMTP id t9so17398995pgn.4 for ; Tue, 22 Jun 2021 08:43:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=nWPJUX+yxJ95bHyYoyieqGXNAUlau6DE6+4vD23SFQc=; b=oJtKpGyaEdCced+nk8xxiM+3ZDLIdSiTsfIOnJHQKAQq7qn0tez2G07nb4QV4B/1A5 lZw57xVEmmle+GTFgqckpCPyeg3XPRhKsxmLt29/RZhopocnTEtW+ogYjrtKc/RjxvSu kq5cwpQ0ahq8G9CY8cdi4Yn3jJZ0qlgXbRzdbGFZtjqmGrUVXNyS+hkGX0F16829ySxS LIOix4F1b5M22Vp0U2gy4iWaSS0Qr9uAmH7oJwXVqIQXZz9KExdWFv/mZnbtivgP1RiW WJnlusVuN8cXGOXQKuANIAfQ/ZDyOTcDn6nZzRQCBUMfxPqCrYu6zRvyEX9Waf31B0SQ Ma8A== X-Gm-Message-State: AOAM530F0qCAZ31xOEcgTuHKFdFejyEgtPX/tbKHSHGQk3EmmYkwqVjv plRnOmkVz7zauhbzfuFQsbHaZ1PWzkSWRWwjuJGfEbQE X-Google-Smtp-Source: ABdhPJwF2vNh2X1HljIwJxGx7FhbZ/N/Jv+2bJ02S77RWjmdcb808PL4q9sY1Ko8Pw9jf7+9FulvAT7pb5LkKQLZTQ0= X-Received: by 2002:a65:4985:: with SMTP id r5mr4307275pgs.122.1624376600579; Tue, 22 Jun 2021 08:43:20 -0700 (PDT) MIME-Version: 1.0 References: <20210609205257.123944-1-goldstein.w.n@gmail.com> In-Reply-To: From: Noah Goldstein Date: Tue, 22 Jun 2021 11:43:09 -0400 Message-ID: Subject: Re: [PATCH v1 1/3] String: Add additional overflow tests for strnlen, memchr, and strncat To: "H.J. Lu" Cc: GNU C Library , "Carlos O'Donell" X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Jun 2021 15:43:24 -0000 On Wed, Jun 9, 2021 at 6:26 PM Noah Goldstein wrote: > > > On Wed, Jun 9, 2021 at 5:54 PM H.J. Lu wrote: > >> On Wed, Jun 9, 2021 at 1:53 PM Noah Goldstein >> wrote: >> > >> > 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. >> >> Please open a bug report for each standard C function. We need to >> track them for backporting to release branches. >> > > Done: https://sourceware.org/bugzilla/show_bug.cgi?id=27974 > > >> >> Thanks. >> >> > Signed-off-by: Noah Goldstein >> > --- >> > 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..0ab7541d4e 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[0] = '\0'; >> > + do_one_test (impl, s2, s1, SIZE_MAX - i); >> > + s2[0] = '\0'; >> > + do_one_test (impl, s2, s1, i - s1_addr); >> > + s2[0] = '\0'; >> > + do_one_test (impl, s2, s1, -s1_addr - i); >> > + s2[0] = '\0'; >> > + do_one_test (impl, s2, s1, SIZE_MAX - s1_addr - i); >> > + s2[0] = '\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[0] = '\0'; >> > + do_one_test (impl, s2, s1, len - i); >> > + s2[0] = '\0'; >> > + do_one_test (impl, s2, s1, len + i); >> > + s2[0] = '\0'; >> > + do_one_test (impl, s2, s1, len - s1_addr - i); >> > + s2[0] = '\0'; >> > + do_one_test (impl, s2, s1, len - s1_addr + i); >> > + >> > + s2[0] = '\0'; >> > + do_one_test (impl, s2, s1, ~len - i); >> > + s2[0] = '\0'; >> > + do_one_test (impl, s2, s1, ~len + i); >> > + s2[0] = '\0'; >> > + do_one_test (impl, s2, s1, ~len - s1_addr - i); >> > + s2[0] = '\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 >> > >> >> >> -- >> H.J. >> > Ping if we want this in 2.34