From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1032.google.com (mail-pj1-x1032.google.com [IPv6:2607:f8b0:4864:20::1032]) by sourceware.org (Postfix) with ESMTPS id 2B3983858C1F for ; Wed, 16 Feb 2022 02:29:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2B3983858C1F Received: by mail-pj1-x1032.google.com with SMTP id v8-20020a17090a634800b001bb78857ccdso2818102pjs.1 for ; Tue, 15 Feb 2022 18:29:28 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=PmLgF6n1N8HjceA2XbZx1xtFfQjfFVfopn+JUApoxPA=; b=ujCxBmReVTV1DKbQAt6cwoQBzIr+DThFfXxMRMVsUYrrWsjGC7a9vXMAP41JxROLDW pg6sS+oFtxtCPQYbTwJ6cFkvbXBmB+hYKCNNG0hflqLom7oCEYkeGX4Bki+48s7lkc9s caf8rboE8IKy3TneKSz/wpWzjFUOeY6ehYs7wIr7jlvJYbUGHSSvCTTJ6jslZZFxyWjJ /msL2IjlWxdP38LAV5NyIxNPlqHcMHDw99p9Bu/qxVqqUiJm9pHjHOsNL0RyNspDgusr U8cgLeRylPgy8vDpmdfysiXOm7kOn7D/VtbuuyIT/R1/OQJL4BZvhLSSh2lpDzySv1fP h5Rg== X-Gm-Message-State: AOAM532Pi9IUpWPG0sgYEnAgo83yrGn5RlOsOtoLvGwjFY7G0LA2004g hdyLY/6iPyPupAHb4InS3F+LYP/fk/JZILkXCET/mshhUsg= X-Google-Smtp-Source: ABdhPJwhGQip7+/G+n63gHRJCOXfjYHsyVHku62IzxeAedsTjXij8tOBEvL+sRYYiPmytOOjMZFwpWGllSXJin7jreI= X-Received: by 2002:a17:902:e8c2:b0:14d:8ddc:c1eb with SMTP id v2-20020a170902e8c200b0014d8ddcc1ebmr788400plg.102.1644978567209; Tue, 15 Feb 2022 18:29:27 -0800 (PST) MIME-Version: 1.0 References: <20220215162829.282223-1-goldstein.w.n@gmail.com> <20220216022721.3267920-1-goldstein.w.n@gmail.com> In-Reply-To: <20220216022721.3267920-1-goldstein.w.n@gmail.com> From: "H.J. Lu" Date: Tue, 15 Feb 2022 18:28:51 -0800 Message-ID: Subject: Re: [PATCH v4] x86: Fix bug in strncmp-evex and strncmp-avx2 [BZ #28895] To: Noah Goldstein Cc: GNU C Library , "Carlos O'Donell" Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3026.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org 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: Wed, 16 Feb 2022 02:29:31 -0000 On Tue, Feb 15, 2022 at 6:27 PM Noah Goldstein wrote: > > Logic can read before the start of `s1` / `s2` if both `s1` and `s2` > are near the start of a page. To avoid having the result contimated by > these comparisons the `strcmp` variants would mask off these > comparisons. This was missing in the `strncmp` variants causing > the bug. This commit adds the masking to `strncmp` so that out of > range comparisons don't affect the result. > > test-strcmp, test-strncmp, test-wcscmp, and test-wcsncmp all pass as > well a full xcheck on x86_64 linux. > --- > string/test-strncmp.c | 23 +++++++++++++++++++++++ > sysdeps/x86_64/multiarch/strcmp-avx2.S | 1 + > sysdeps/x86_64/multiarch/strcmp-evex.S | 1 + > 3 files changed, 25 insertions(+) > > diff --git a/string/test-strncmp.c b/string/test-strncmp.c > index 831cb77893..df7cea4068 100644 > --- a/string/test-strncmp.c > +++ b/string/test-strncmp.c > @@ -423,6 +423,28 @@ check3 (void) > } > } > > +static void > +check4 (void) > +{ > + /* To trigger bug 28895; We need 1) both s1 and s2 to be within 32 bytes of > + the end of the page. 2) For there to be no mismatch/null byte before the > + first page cross. 3) For length (`n`) to be large enough for one string to > + cross the page. And 4) for there to be either mismatch/null bytes before > + the start of the strings. */ > + > + size_t size = 10; > + size_t addr_mask = (getpagesize () - 1) ^ (sizeof (CHAR) - 1); > + CHAR *s1 = (CHAR *)(buf1 + (addr_mask & 0xffa)); > + CHAR *s2 = (CHAR *)(buf2 + (addr_mask & 0xfed)); > + int exp_result; > + > + STRCPY (s1, L ("tst-tlsmod%")); > + STRCPY (s2, L ("tst-tls-manydynamic73mod")); > + exp_result = SIMPLE_STRNCMP (s1, s2, size); > + FOR_EACH_IMPL (impl, 0) > + check_result (impl, s1, s2, size, exp_result); > +} > + > static void > check_overflow (void) > { > @@ -546,6 +568,7 @@ test_main (void) > check1 (); > check2 (); > check3 (); > + check4 (); > > printf ("%23s", ""); > FOR_EACH_IMPL (impl, 0) > diff --git a/sysdeps/x86_64/multiarch/strcmp-avx2.S b/sysdeps/x86_64/multiarch/strcmp-avx2.S > index 99e5349be8..07a5a2c889 100644 > --- a/sysdeps/x86_64/multiarch/strcmp-avx2.S > +++ b/sysdeps/x86_64/multiarch/strcmp-avx2.S > @@ -661,6 +661,7 @@ L(ret8): > # ifdef USE_AS_STRNCMP > .p2align 4,, 10 > L(return_page_cross_end_check): > + andl %r10d, %ecx > tzcntl %ecx, %ecx > leal -VEC_SIZE(%rax, %rcx), %ecx > cmpl %ecx, %edx > diff --git a/sysdeps/x86_64/multiarch/strcmp-evex.S b/sysdeps/x86_64/multiarch/strcmp-evex.S > index 6f42e3155a..56d8c118e4 100644 > --- a/sysdeps/x86_64/multiarch/strcmp-evex.S > +++ b/sysdeps/x86_64/multiarch/strcmp-evex.S > @@ -689,6 +689,7 @@ L(ret8): > # ifdef USE_AS_STRNCMP > .p2align 4,, 10 > L(return_page_cross_end_check): > + andl %r10d, %ecx > tzcntl %ecx, %ecx > leal -VEC_SIZE(%rax, %rcx, SIZE_OF_CHAR), %ecx > # ifdef USE_AS_WCSCMP > -- > 2.25.1 > LGTM. Reviewed-by: H.J. Lu Thanks. -- H.J.