From patchwork Thu Jun 7 11:38:08 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 27687 Received: (qmail 691 invoked by alias); 7 Jun 2018 11:38:55 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 130517 invoked by uid 89); 7 Jun 2018 11:38:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=affected X-HELO: mx1.redhat.com From: Florian Weimer Subject: Re: [PATCH] Improve DST handling (Bug 23102, Bug 21942, Bug 18018, Bug, 23259, CVE-2011-0536 ). To: Carlos O'Donell , GNU C Library , Andreas Schwab , "Dmitry V. Levin" References: <9cf43cb6-511c-ec6c-9a87-e89a467238d9@redhat.com> Message-ID: Date: Thu, 7 Jun 2018 13:38:08 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: On 06/06/2018 10:18 PM, Carlos O'Donell wrote: > +is_dst (const char *input, const char *ref) Maybe we should fix the strncmp bug and use that? In rtld, the actual definitions are not built with the proper name, but aliases for use in IFUNCs, which we don't use in rtld. The attached patch fixes that for x86-64 and i686 (multi-arch in both cases). I don't think other architectures are affected (verified for ppc64le, build-many-glibcs.py is still running). If you want to use strncmp to express more clearly what the code is about, I can commit the fix separately (with a proper ChangeLog entry etc.). Thanks, Florian diff --git a/elf/dl-load.c b/elf/dl-load.c index ff7a95bee2..ff271c0bdf 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -192,12 +192,12 @@ is_trusted_path_normalize (const char *path, size_t len) * Must follow first two characters with zero or more [A-Za-z0-9_] (enforced by caller) or '}' (end curly quoted name). - If the sequence is a DST matching REF then the length of the DST is + If the sequence is a DST matching REF then the length of the DST + (excluding the $ sign but including curly braces, if any) is returned, otherwise 0. */ static size_t is_dst (const char *input, const char *ref) { - size_t ilen, rlen; bool is_curly = false; /* Is a ${...} input sequence? */ @@ -207,35 +207,23 @@ is_dst (const char *input, const char *ref) ++input; } - /* Find longest valid input sequence. */ - ilen = 0; - while ((input[ilen] >= 'A' && input[ilen] <= 'Z') - || (input[ilen] >= 'a' && input[ilen] <= 'z') - || (input[ilen] >= '0' && input[ilen] <= '9') - || (input[ilen] == '_')) - ++ilen; - - rlen = strlen (ref); - - /* Can't be the DST we are looking for. */ - if (rlen != ilen) - return 0; - - /* Compare the DST (no strncmp this early in startup). */ - if (memcmp (input, ref, ilen) != 0) + /* Check for matching name, following closing curly brace (if + required), or trailing characters which are part of an + identifier. */ + size_t rlen = strlen (ref); + if (strncmp (input, ref, rlen) != 0 + || (is_curly && input[rlen] != '}') + || ((input[rlen] >= 'A' && input[rlen] <= 'Z') + || (input[rlen] >= 'a' && input[rlen] <= 'z') + || (input[rlen] >= '0' && input[rlen] <= '9') + || (input[rlen] == '_'))) return 0; if (is_curly) - { - /* Invalid curly sequence! */ - if (input[ilen] != '}') - return 0; - - /* Count the two curly braces. */ - ilen += 2; - } - - return ilen; + /* Count the two curly braces. */ + return rlen + 2; + else + return rlen; } /* INPUT is the start of a DST sequence at the first '$' occurrence. diff --git a/sysdeps/i386/i686/multiarch/strncmp-c.c b/sysdeps/i386/i686/multiarch/strncmp-c.c index cc059da494..2e3eca9b2b 100644 --- a/sysdeps/i386/i686/multiarch/strncmp-c.c +++ b/sysdeps/i386/i686/multiarch/strncmp-c.c @@ -1,4 +1,4 @@ -#ifdef SHARED +#if defined (SHARED) && IS_IN (libc) # define STRNCMP __strncmp_ia32 # undef libc_hidden_builtin_def # define libc_hidden_builtin_def(name) \ diff --git a/sysdeps/x86_64/multiarch/strncmp-sse2.S b/sysdeps/x86_64/multiarch/strncmp-sse2.S index cc5252d826..0e1f52dea9 100644 --- a/sysdeps/x86_64/multiarch/strncmp-sse2.S +++ b/sysdeps/x86_64/multiarch/strncmp-sse2.S @@ -18,10 +18,13 @@ #include -#define STRCMP __strncmp_sse2 - -#undef libc_hidden_builtin_def -#define libc_hidden_builtin_def(strcmp) +#if IS_IN (libc) +# define STRCMP __strncmp_sse2 +# undef libc_hidden_builtin_def +# define libc_hidden_builtin_def(strcmp) +#else +# define STRCMP strncmp +#endif #define USE_AS_STRNCMP #include