From patchwork Tue Jul 4 20:03:02 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 1803370 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=tcxLKTmb; dkim-atps=neutral Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4QwYgY2379z20Pg for ; Wed, 5 Jul 2023 06:05:29 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3DF433885505 for ; Tue, 4 Jul 2023 20:05:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3DF433885505 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1688501127; bh=hiyQwMJjFbxP5IDS8Fcm/a0LtjTDzYjIaBXw+Xj9LI8=; h=To:Subject:In-Reply-To:References:Date:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=tcxLKTmbSvJj/yOAVH686LEI3nF4uV6JBvSbZlU4tI6nVUnEikvc0gTtoMjz9srjJ 9EIacbZQr7nrfUq6AwWA6BMtQJ8CpZJd3Gz1V7Gp93/4E/9LB4yOs5vP8iQGnbl20C w3HStEwgPpr76KL+Mqz+T6fBYdu/+c2/6+6Dg6+w= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org 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 92A683853556 for ; Tue, 4 Jul 2023 20:03:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 92A683853556 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-578-lLsgdw7uP_2aJfz31EHVHw-1; Tue, 04 Jul 2023 16:03:05 -0400 X-MC-Unique: lLsgdw7uP_2aJfz31EHVHw-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 0243480123E for ; Tue, 4 Jul 2023 20:03:05 +0000 (UTC) Received: from oldenburg.str.redhat.com (unknown [10.2.16.19]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0B1B0F6424 for ; Tue, 4 Jul 2023 20:03:03 +0000 (UTC) To: libc-alpha@sourceware.org Subject: [PATCH 11/33] elf: Merge the three implementations of _dl_dst_substitute In-Reply-To: Message-ID: <17f407415e7898f078bb3c9ad656403f9ce60ffe.1688499219.git.fweimer@redhat.com> References: X-From-Line: 17f407415e7898f078bb3c9ad656403f9ce60ffe Mon Sep 17 00:00:00 2001 Date: Tue, 04 Jul 2023 22:03:02 +0200 User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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-Patchwork-Original-From: Florian Weimer via Libc-alpha From: Florian Weimer Reply-To: Florian Weimer Errors-To: libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org Sender: "Libc-alpha" Use one implementation to perform the copying and the counting. The IS_RTLD check for l_origin processing is eliminated because $ORIGIN cannot used with the ld.so link map (and not with the vDSO link map either) because there is no user code associated with it that might call dlopen with an $ORIGIN path. --- elf/dl-deps.c | 70 ++++++------- elf/dl-load.c | 146 ++++++++++++---------------- elf/dl-open.c | 1 - sysdeps/generic/ldsodefs.h | 9 +- sysdeps/unix/sysv/linux/dl-origin.c | 1 - 5 files changed, 94 insertions(+), 133 deletions(-) diff --git a/elf/dl-deps.c b/elf/dl-deps.c index 0549b4a4ff..110c8953bd 100644 --- a/elf/dl-deps.c +++ b/elf/dl-deps.c @@ -28,8 +28,7 @@ #include #include #include - -#include +#include /* Whether an shared object references one or more auxiliary objects is signaled by the AUXTAG entry in l_info. */ @@ -80,47 +79,34 @@ struct list }; -/* Macro to expand DST. It is an macro since we use `alloca'. */ +/* Macro to expand DST. It is an macro since we use `alloca'. + See expand_dynamic_string_token in dl-load.c. */ #define expand_dst(l, str, fatal) \ - ({ \ - const char *__str = (str); \ - const char *__result = __str; \ - size_t __dst_cnt = _dl_dst_count (__str); \ - \ - if (__dst_cnt != 0) \ - { \ - char *__newp; \ - \ - /* DST must not appear in SUID/SGID programs. */ \ - if (__libc_enable_secure) \ - _dl_signal_error (0, __str, NULL, N_("\ -DST not allowed in SUID/SGID programs")); \ - \ - __newp = (char *) alloca (DL_DST_REQUIRED (l, __str, strlen (__str), \ - __dst_cnt)); \ - \ - __result = _dl_dst_substitute (l, __str, __newp); \ - \ - if (*__result == '\0') \ - { \ - /* The replacement for the DST is not known. We can't \ - processed. */ \ - if (fatal) \ - _dl_signal_error (0, __str, NULL, N_("\ -empty dynamic string token substitution")); \ - else \ - { \ - /* This is for DT_AUXILIARY. */ \ - if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_LIBS)) \ - _dl_debug_printf (N_("\ -cannot load auxiliary `%s' because of empty dynamic string token " \ - "substitution\n"), __str); \ - continue; \ - } \ - } \ - } \ - \ - __result; }) + ({ \ + struct alloc_buffer __buf = {}; \ + size_t __size = _dl_dst_substitute ((l), (str), &__buf); \ + char *__result = alloca (__size); \ + __buf = alloc_buffer_create (__result, __size); \ + if (_dl_dst_substitute ((l), (str), &__buf) == 0) \ + { \ + /* The replacement for the DST is not known. We can't \ + processed. */ \ + if (fatal) \ + _dl_signal_error (0, str, NULL, N_("\ +empty dynamic string token substitution")); \ + else \ + { \ + /* This is for DT_AUXILIARY. */ \ + if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_LIBS)) \ + _dl_debug_printf (N_("\ +cannot load auxiliary `%s' because of empty dynamic string token " \ + "substitution\n"), str); \ + continue; \ + } \ + } \ + assert (!alloc_buffer_has_failed (&__buf)); \ + __result; \ + }) \ static void preload (struct list *known, unsigned int *nlist, struct link_map *map) diff --git a/elf/dl-load.c b/elf/dl-load.c index fda0fe8000..f46af28944 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -32,6 +32,7 @@ #include #include #include +#include /* Type for the buffer we put the ELF header and hopefully the program header. This buffer does not really have to be too large. In most @@ -67,7 +68,6 @@ struct filebuf #include #include -#include #include #include #include @@ -227,61 +227,32 @@ is_dst (const char *input, const char *ref) return rlen; } -/* INPUT should be the start of a path e.g DT_RPATH or name e.g. - DT_NEEDED. The return value is the number of known DSTs found. We - count all known DSTs regardless of __libc_enable_secure; the caller - is responsible for enforcing the security of the substitution rules - (usually _dl_dst_substitute). */ -size_t -_dl_dst_count (const char *input) -{ - size_t cnt = 0; - - input = strchr (input, '$'); - - /* Most likely there is no DST. */ - if (__glibc_likely (input == NULL)) - return 0; - - do - { - size_t len; - - ++input; - /* All DSTs must follow ELF gABI rules, see is_dst (). */ - if ((len = is_dst (input, "ORIGIN")) != 0 - || (len = is_dst (input, "PLATFORM")) != 0 - || (len = is_dst (input, "LIB")) != 0) - ++cnt; - - /* There may be more than one DST in the input. */ - input = strchr (input + len, '$'); - } - while (input != NULL); - - return cnt; -} - -/* Process INPUT for DSTs and store in RESULT using the information +/* Process INPUT for DSTs and store in *RESULT using the information from link map L to resolve the DSTs. This function only handles one path at a time and does not handle colon-separated path lists (see - fillin_rpath ()). Lastly the size of result in bytes should be at - least equal to the value returned by DL_DST_REQUIRED. Note that it - is possible for a DT_NEEDED, DT_AUXILIARY, and DT_FILTER entries to - have colons, but we treat those as literal colons here, not as path - list delimiters. */ -char * -_dl_dst_substitute (struct link_map *l, const char *input, char *result) + fillin_rpath ()). + + A caller is expected to call this function twice, first with an + empty *RESULT buffer to obtain the total length (including the + terminating null byte) that is returned by this function. The + second call should be made with a properly sized buffer, and this + function will write the expansion to *RESULT. If that second call + returns 0, it means that the expansion is not valid and should be + ignored. + + Note that it is possible for a DT_NEEDED, + DT_AUXILIARY, and DT_FILTER entries to have colons, but we treat + those as literal colons here, not as path list delimiters. */ +size_t +_dl_dst_substitute (struct link_map *l, const char *input, + struct alloc_buffer *result) { /* Copy character-by-character from input into the working pointer - looking for any DSTs. We track the start of input and if we are - going to check for trusted paths, all of which are part of $ORIGIN - handling in SUID/SGID cases (see below). In some cases, like when - a DST cannot be replaced, we may set result to an empty string and - return. */ - char *wp = result; + looking for any DSTs. */ const char *start = input; + char *result_start = alloc_buffer_next (result, char); bool check_for_trusted = false; + size_t length = 0; do { @@ -318,7 +289,16 @@ _dl_dst_substitute (struct link_map *l, const char *input, char *result) && (input[len] == '\0' || input[len] == '/'))) repl = (const char *) -1; else - repl = l->l_origin; + { + if (l->l_origin == NULL) + { + /* For loaded DSOs, the l_origin field is set in + _dl_new_object. */ + assert (l->l_name[0] == '\0'); + l->l_origin = _dl_get_origin (); + } + repl = l->l_origin; + } check_for_trusted = (__libc_enable_secure && l->l_type == lt_executable); @@ -330,7 +310,9 @@ _dl_dst_substitute (struct link_map *l, const char *input, char *result) if (repl != NULL && repl != (const char *) -1) { - wp = __stpcpy (wp, repl); + size_t repl_len = strlen (repl); + length += repl_len; + alloc_buffer_copy_bytes (result, repl, repl_len); input += len; } else if (len != 0) @@ -338,16 +320,20 @@ _dl_dst_substitute (struct link_map *l, const char *input, char *result) /* We found a valid DST that we know about, but we could not find a replacement value for it, therefore we cannot use this path and discard it. */ - *result = '\0'; - return result; + alloc_buffer_mark_failed (result); + return 0; } else - /* No DST we recognize. */ - *wp++ = '$'; + { + /* No DST we recognize. */ + ++length; + alloc_buffer_add_byte (result, '$'); + } } else { - *wp++ = *input++; + ++length; + alloc_buffer_add_byte (result, *input++); } } while (*input != '\0'); @@ -362,15 +348,19 @@ _dl_dst_substitute (struct link_map *l, const char *input, char *result) this way because it may be manipulated in some ways with hard links. */ if (__glibc_unlikely (check_for_trusted) - && !is_trusted_path_normalize (result, wp - result)) + && !alloc_buffer_has_failed (result) + && !is_trusted_path_normalize (result_start, + alloc_buffer_next (result, char) + - result_start)) { - *result = '\0'; - return result; + alloc_buffer_mark_failed (result); + return 0; } - *wp = '\0'; + ++length; + alloc_buffer_add_byte (result, 0); - return result; + return length; } @@ -382,30 +372,18 @@ _dl_dst_substitute (struct link_map *l, const char *input, char *result) static char * expand_dynamic_string_token (struct link_map *l, const char *input) { - /* We make two runs over the string. First we determine how large the - resulting string is and then we copy it over. Since this is no - frequently executed operation we are looking here not for performance - but rather for code size. */ - size_t cnt; - size_t total; - char *result; - - /* Determine the number of DSTs. */ - cnt = _dl_dst_count (input); - - /* If we do not have to replace anything simply copy the string. */ - if (__glibc_likely (cnt == 0)) - return __strdup (input); - - /* Determine the length of the substituted string. */ - total = DL_DST_REQUIRED (l, input, strlen (input), cnt); - - /* Allocate the necessary memory. */ - result = (char *) malloc (total + 1); + struct alloc_buffer buf = {}; + size_t size = _dl_dst_substitute (l, input, &buf); + char *result = malloc (size); if (result == NULL) return NULL; - - return _dl_dst_substitute (l, input, result); + buf = alloc_buffer_create (result, size); + if (_dl_dst_substitute (l, input, &buf) == 0) + /* Mark the expanded string as to be ignored. */ + *result = '\0'; + else + assert (!alloc_buffer_has_failed (&buf)); + return result; } diff --git a/elf/dl-open.c b/elf/dl-open.c index 2d985e21d8..d88e6b5f6b 100644 --- a/elf/dl-open.c +++ b/elf/dl-open.c @@ -38,7 +38,6 @@ #include #include -#include #include diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h index 5941da3ec1..3698e34bc0 100644 --- a/sysdeps/generic/ldsodefs.h +++ b/sysdeps/generic/ldsodefs.h @@ -1224,12 +1224,11 @@ extern void _dl_nothread_init_static_tls (struct link_map *) attribute_hidden; /* Find origin of the executable. */ extern const char *_dl_get_origin (void) attribute_hidden; -/* Count DSTs. */ -extern size_t _dl_dst_count (const char *name) attribute_hidden; - /* Substitute DST values. */ -extern char *_dl_dst_substitute (struct link_map *l, const char *name, - char *result) attribute_hidden; +struct alloc_buffer; +size_t _dl_dst_substitute (struct link_map *l, const char *name, + struct alloc_buffer *result) + attribute_hidden __nonnull ((1, 2, 3)); /* Open the shared object NAME, relocate it, and run its initializer if it hasn't already been run. MODE is as for `dlopen' (see ). If diff --git a/sysdeps/unix/sysv/linux/dl-origin.c b/sysdeps/unix/sysv/linux/dl-origin.c index d87e89335d..82298c28f4 100644 --- a/sysdeps/unix/sysv/linux/dl-origin.c +++ b/sysdeps/unix/sysv/linux/dl-origin.c @@ -17,7 +17,6 @@ . */ #include -#include #include #include #include