From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id A20373858434 for ; Wed, 1 Feb 2023 03:10:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A20373858434 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675221021; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Ab4ovSfH78hZOWYj5PTNVmxRKThqC/zHTAxAYSqdZvI=; b=iILivLcuvpZd0XYigrcsN0jD+Rqw1+X3cLDwFiA8vvX27Rn7vgZ0RBbgwFAoDu7YfH8ZWz O+iH7SxHqFS4LgzwdkokojB0ZRZpQwfA9iinBiQESnG822PRY2VlFUZwp9DePM5nzqDnSQ C+THxfzb+cPECoZVw3DGvra4rYDOguo= Received: from mail-il1-f199.google.com (mail-il1-f199.google.com [209.85.166.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-226-wnrOaHpMOJKbIUrnnd5Ggw-1; Tue, 31 Jan 2023 22:10:20 -0500 X-MC-Unique: wnrOaHpMOJKbIUrnnd5Ggw-1 Received: by mail-il1-f199.google.com with SMTP id w10-20020a056e021c8a00b0030efad632e0so10794413ill.22 for ; Tue, 31 Jan 2023 19:10:19 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Ab4ovSfH78hZOWYj5PTNVmxRKThqC/zHTAxAYSqdZvI=; b=Zv4LQwV+rHWTn07Z/EJrwp7cRHTTGkB++LfIYd2Yv6M96AonzJM0IRCB2ztesDJdWc AHI/LGP03WoziUWWP2RZIiCvbG/IRGNNylfYzQ0o/+Kfr+9XLL2BpV5N3DpS+qcRY4x3 2clURdCLBPl5MA2lug+UH8+kCg/q6OAwZQl+uHh/9IS9c1d77hMnQ9/hnl6nAX069WPP 6JBI5oFVsY2bhG81u3DhXZQMV/KKp8OgDpe5nTNoWwDOcE4s5jpvFLKAJvOl//JAV568 wZVor91uRuOdzH+s5kFaLsHYFBcD/non2vXmqtfwVoZirgSp9630sHpDlqDLOTopzq5a cVOg== X-Gm-Message-State: AO0yUKUHSbSwhqk4G1Po9s5gOZVOyYWoqWPn4AZ7JaCW/J0W55Q4LGkh M5Eo5m7ka7VTXlsfP2zB92K+fR1M3oVX3Rn+kfq6lmJM69CCRE+XKS1lvRp5AtYS6rGTeIoOAbu Kwa3eVv/FB1TBnmJtiD09 X-Received: by 2002:a05:6e02:214d:b0:310:c16d:9611 with SMTP id d13-20020a056e02214d00b00310c16d9611mr690585ilv.25.1675221019155; Tue, 31 Jan 2023 19:10:19 -0800 (PST) X-Google-Smtp-Source: AK7set8SkB/ru8CaHhLMPjw4GorD5JxGaNUm03GTGYW5a0fBano219ycbaFhgnGG6TmBtdabRT/RWg== X-Received: by 2002:a05:6e02:214d:b0:310:c16d:9611 with SMTP id d13-20020a056e02214d00b00310c16d9611mr690576ilv.25.1675221018872; Tue, 31 Jan 2023 19:10:18 -0800 (PST) Received: from [192.168.0.241] (192-0-145-146.cpe.teksavvy.com. [192.0.145.146]) by smtp.gmail.com with ESMTPSA id h7-20020a92c267000000b0031109a23893sm1166578ild.23.2023.01.31.19.10.17 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 31 Jan 2023 19:10:18 -0800 (PST) Message-ID: Date: Tue, 31 Jan 2023 22:10:17 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 Subject: Re: [PATCH v2] x86: Fix strncat-avx2.S reading past length [BZ #30065] To: Noah Goldstein , libc-alpha@sourceware.org Cc: hjl.tools@gmail.com, carlos@systemhalted.org References: <20230131213655.4033602-1-goldstein.w.n@gmail.com> <20230131234656.2175991-1-goldstein.w.n@gmail.com> From: Carlos O'Donell Organization: Red Hat In-Reply-To: <20230131234656.2175991-1-goldstein.w.n@gmail.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 1/31/23 18:46, Noah Goldstein via Libc-alpha wrote: > Occurs when `src` has no null-term. This has been pushed as b2c474f8de4c92bfe7435853a96805ec32d68dfa. We are now in a hard freeze as I prepare to cut the release. Please do not commit anything further. If we find other issues we can backport to the release branch after testing. I'm re-running testing with this patch included for x86_64 and i686. > Two cases: > > 1) Zero-length check is doing: > ``` > test %rdx, %rdx > jl L(zero_len) > ``` > which doesn't actually check zero (was at some point `decq` and the > flag never got updated). > > The fix is just make the flag `jle` i.e: > ``` > test %rdx, %rdx > jle L(zero_len) > ``` > > 2) Length check in page-cross case checking if we should continue is > doing: > ``` > cmpq %r8, %rdx > jb L(page_cross_small) > ``` > which means we will continue searching for null-term if length ends at > the end of a page and there was no null-term in `src`. > > The fix is to make the flag: > ``` > cmpq %r8, %rdx > jbe L(page_cross_small) > ``` > --- > string/test-strncat.c | 25 ++++++++++++++++++++++++- > sysdeps/x86_64/multiarch/strncat-avx2.S | 4 ++-- > 2 files changed, 26 insertions(+), 3 deletions(-) > > diff --git a/string/test-strncat.c b/string/test-strncat.c > index e03d329e1c..c0cde206ee 100644 > --- a/string/test-strncat.c > +++ b/string/test-strncat.c > @@ -28,6 +28,7 @@ > # define CHAR char > # define UCHAR unsigned char > # define SIMPLE_STRNCAT simple_strncat > +# define STRNLEN strnlen > # define STRLEN strlen > # define MEMSET memset > # define MEMCPY memcpy > @@ -40,6 +41,7 @@ > # define CHAR wchar_t > # define UCHAR wchar_t > # define SIMPLE_STRNCAT simple_wcsncat > +# define STRNLEN wcsnlen > # define STRLEN wcslen > # define MEMSET wmemset > # define MEMCPY wmemcpy > @@ -78,7 +80,7 @@ do_one_test (impl_t *impl, CHAR *dst, const CHAR *src, size_t n) > return; > } > > - size_t len = STRLEN (src); > + size_t len = STRNLEN (src, n); > if (MEMCMP (dst + k, src, len + 1 > n ? n : len + 1) != 0) > { > error (0, 0, "Incorrect concatenation in function %s", > @@ -95,6 +97,26 @@ do_one_test (impl_t *impl, CHAR *dst, const CHAR *src, size_t n) > } > } > > +static void > +do_test_src_no_nullterm_bz30065 (void) > +{ > + /* NB: "src does not need to be null-terminated if it contains n or more > + * bytes." */ > + CHAR *s1, *s2; > + size_t bound = page_size / sizeof (CHAR); > + s1 = (CHAR *) (buf1 + BUF1PAGES * page_size); > + s2 = (CHAR *) buf2; > + MEMSET (s1 - bound, -1, bound); > + for (size_t n = 0; n < bound; ++n) > + { > + FOR_EACH_IMPL (impl, 0) > + { > + s2[0] = '\0'; > + do_one_test (impl, s2, s1 - n, n); > + } > + } > +} > + > static void > do_test (size_t align1, size_t align2, size_t len1, size_t len2, > size_t n, int max_char) > @@ -372,6 +394,7 @@ test_main (void) > > do_random_tests (); > do_overflow_tests (); > + do_test_src_no_nullterm_bz30065 (); > return ret; > } > > diff --git a/sysdeps/x86_64/multiarch/strncat-avx2.S b/sysdeps/x86_64/multiarch/strncat-avx2.S > index b380e8e11c..c2ff202238 100644 > --- a/sysdeps/x86_64/multiarch/strncat-avx2.S > +++ b/sysdeps/x86_64/multiarch/strncat-avx2.S > @@ -66,7 +66,7 @@ ENTRY(STRNCAT) > salq $2, %rdx > # else > test %rdx, %rdx > - jl L(zero_len) > + jle L(zero_len) > # endif > vpxor %VZERO_128, %VZERO_128, %VZERO_128 > > @@ -387,7 +387,7 @@ L(page_cross): > subl %esi, %r8d > andl $(VEC_SIZE - 1), %r8d > cmpq %r8, %rdx > - jb L(page_cross_small) > + jbe L(page_cross_small) > > /* Optimizing more aggressively for space as this is very cold > code. This saves 2x cache lines. */ -- Cheers, Carlos.