From patchwork Tue May 7 17:25:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 1932564 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=jwxNdXpO; dkim-atps=neutral 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=patchwork.ozlabs.org) 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 (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4VYlYj0y57z1xnT for ; Wed, 8 May 2024 03:26:16 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D167F3858C35 for ; Tue, 7 May 2024 17:26:14 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-pg1-x52e.google.com (mail-pg1-x52e.google.com [IPv6:2607:f8b0:4864:20::52e]) by sourceware.org (Postfix) with ESMTPS id 8140C384AB64 for ; Tue, 7 May 2024 17:25:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8140C384AB64 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8140C384AB64 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::52e ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715102758; cv=none; b=aM7FI2Z7D0HndEpshgBzk9HTCnVuMXfmPMDwqqf+wtr5y8jlCetSITF3sO+keyCenzNZ97a8t6uj2kcTdAPHNndDnJ96/63dDnBAW9Ah0nWa1DHJqTNIUbxii+q+rz7ApeQQZfObyvQak0NDIH5i+wClxATinwVtJptWCJdCgUI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715102758; c=relaxed/simple; bh=UMGXtACnACqxp+EM45bcOXX1QTk/Iyw1iPflVvU5Kkk=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=OJ3zIAs5OkPK4tl3GwXJY98/0O2Pe63jc5httHcE867qq3YZUt020c+QD0AykYEbpvrFAVoHQBYkEAVQLuVyBaNo9rC7FLMeqjzu/CoHWGQvBGP1VueI9VwiejTANTwDR7pjo5C/Q3xigIZE68AxJ+Q7WEh8Lc/+sEQeRkKloLU= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pg1-x52e.google.com with SMTP id 41be03b00d2f7-5c6bd3100fcso2153301a12.3 for ; Tue, 07 May 2024 10:25:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1715102755; x=1715707555; darn=sourceware.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=DDQka1TF3cbQhR0pp07bWtJkjma45sGcIpW/5Xj3SOc=; b=jwxNdXpObbVnpR1JEI3X7xTGMAwOcjN9AaWOSvypWGjtnZmhD4AH+a8XZaWWcJFEHN 3CLmsDoufoVAbjE0tIMrmEJs4/cExmRwfeS0uFSwKamfveheEKYYZ6Dec4Axw477p2TA /mx5xX/6oRnqe9/RiH7j/c7hX0MgMaaRnsYItCcoPMTiD8iV8mscEaCgf6sw4nucHl4O bVx305LbSmksxbQItV+UeEDU8NvKCB9HawULkYaf7qs0qVXgwqNpMo3WY8V0qnlt9dIg kb5a0MC7PevO7HB0CNbVfdjapobbiuql2NggSUA3zCi+750csWQxgMOqfU7Y/CasOKvI R45g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715102755; x=1715707555; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=DDQka1TF3cbQhR0pp07bWtJkjma45sGcIpW/5Xj3SOc=; b=ui+3JqLjHx3DEpOYpP+LJaTZtE6NQoxExtMbAaqD6/Cd/GVI2dFDjwBKl72lz3b4Ns 0etV/Fg2hwKKpKK4BWv6B9ylRHdWPLIno1OlhfdTumcu6V9t0IOVJNwlG7Jaowjk4NlH rf1DWcOaCQguZrlgqgsUn4XWUOGVmLXbA+7UTT2/PGLGSlrLr2n4ZoYUYTRKZjd57Drm eS9ejEYObRLgPy1oN5JzPNEZJ9M0cZGTlIdJMr38lMVj2Yq/P2r/WxRbxhULeR9kGxEl YCPiP/wKEFDQb9Or00Dfp580umc/rCeL4O1OwareciB09QIpspBCyPp6+NB55XAe+c0V Euig== X-Gm-Message-State: AOJu0YwmBgnD+Xoo3I4Jz+GTvMzWXCdaRs0newxRhkI3Zz0hFlXxCq1O 7nf1adsgN39ILERqny50eOJzZ9HFj09Q4oNMd0FoXl3WS0tt0Pc5UBEWeu6NnkhelSMRxPJojy2 n X-Google-Smtp-Source: AGHT+IFAtssBXgy7ktdXZf1vW5UQqLEeZh1zM7UxP6USeW1ixXBjFX10powmSu7fm2muIoiKUB0UNw== X-Received: by 2002:a17:90b:3745:b0:2a2:579e:652f with SMTP id 98e67ed59e1d1-2b6166c3178mr221481a91.27.1715102754901; Tue, 07 May 2024 10:25:54 -0700 (PDT) Received: from mandiga.. ([2804:1b3:a7c2:6e56:e1f4:f746:9f5f:9ad4]) by smtp.gmail.com with ESMTPSA id fa13-20020a17090af0cd00b0029fbfb620cdsm10161666pjb.28.2024.05.07.10.25.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 May 2024 10:25:54 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org Cc: Yuto Maeda , Yutaro Shimizu , Siddhesh Poyarekar Subject: [COMMITTED 2.39] elf: Only process multiple tunable once (BZ 31686) Date: Tue, 7 May 2024 14:25:31 -0300 Message-ID: <20240507172548.2244928-1-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 X-Spam-Status: No, score=-13.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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 X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org The 680c597e9c3 commit made loader reject ill-formatted strings by first tracking all set tunables and then applying them. However, it does not take into consideration if the same tunable is set multiple times, where parse_tunables_string appends the found tunable without checking if it was already in the list. It leads to a stack-based buffer overflow if the tunable is specified more than the total number of tunables. For instance: GLIBC_TUNABLES=glibc.malloc.check=2:... (repeat over the number of total support for different tunable). Instead, use the index of the tunable list to get the expected tunable entry. Since now the initial list is zero-initialized, the compiler might emit an extra memset and this requires some minor adjustment on some ports. Checked on x86_64-linux-gnu and aarch64-linux-gnu. Reported-by: Yuto Maeda Reported-by: Yutaro Shimizu Reviewed-by: Siddhesh Poyarekar (cherry picked from commit bcae44ea8536b30a7119c0986ff5692bddacb672) --- elf/dl-tunables.c | 28 ++++++---- elf/tst-tunables.c | 61 +++++++++++++++++++++- sysdeps/aarch64/multiarch/memset_generic.S | 4 ++ sysdeps/sparc/sparc64/rtld-memset.c | 3 ++ 4 files changed, 84 insertions(+), 12 deletions(-) diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index 03e1a68675..614ac9c047 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -32,6 +32,7 @@ #include #include #include +#include #define TUNABLES_INTERNAL 1 #include "dl-tunables.h" @@ -223,6 +224,7 @@ parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables) { tunables[ntunables++] = (struct tunable_toset_t) { cur, value, p - value }; + break; } } @@ -234,23 +236,27 @@ parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables) static void parse_tunables (const char *valstring) { - struct tunable_toset_t tunables[tunables_list_size]; - int ntunables = parse_tunables_string (valstring, tunables); - if (ntunables == -1) + struct tunable_toset_t tunables[tunables_list_size] = { 0 }; + if (parse_tunables_string (valstring, tunables) == -1) { _dl_error_printf ( "WARNING: ld.so: invalid GLIBC_TUNABLES `%s': ignored.\n", valstring); return; } - for (int i = 0; i < ntunables; i++) - if (!tunable_initialize (tunables[i].t, tunables[i].value, - tunables[i].len)) - _dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' " - "for option `%s': ignored.\n", - (int) tunables[i].len, - tunables[i].value, - tunables[i].t->name); + for (int i = 0; i < tunables_list_size; i++) + { + if (tunables[i].t == NULL) + continue; + + if (!tunable_initialize (tunables[i].t, tunables[i].value, + tunables[i].len)) + _dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' " + "for option `%s': ignored.\n", + (int) tunables[i].len, + tunables[i].value, + tunables[i].t->name); + } } /* Initialize the tunables list from the environment. For now we only use the diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c index 095b5c81d9..dff34ed748 100644 --- a/elf/tst-tunables.c +++ b/elf/tst-tunables.c @@ -17,6 +17,10 @@ . */ #include +/* The test uses the tunable_list size, which is only exported for + ld.so. This will result in a copy of tunable_list, which is ununsed by + the test itself. */ +#define TUNABLES_INTERNAL 1 #include #include #include @@ -24,12 +28,13 @@ #include #include #include +#include static int restart; #define CMDLINE_OPTIONS \ { "restart", no_argument, &restart, 1 }, -static const struct test_t +static struct test_t { const char *name; const char *value; @@ -284,6 +289,29 @@ static const struct test_t 0, 0, }, + /* Also check for repeated tunables with a count larger than the total number + of tunables. */ + { + "GLIBC_TUNABLES", + NULL, + 2, + 0, + 0, + }, + { + "GLIBC_TUNABLES", + NULL, + 1, + 0, + 0, + }, + { + "GLIBC_TUNABLES", + NULL, + 0, + 0, + 0, + }, }; static int @@ -327,6 +355,37 @@ do_test (int argc, char *argv[]) spargv[i] = NULL; } + /* Create a tunable line with the duplicate values with a total number + larger than the different number of tunables. */ + { + enum { tunables_list_size = array_length (tunable_list) }; + const char *value = ""; + for (int i = 0; i < tunables_list_size; i++) + value = xasprintf ("%sglibc.malloc.check=2%c", + value, + i == (tunables_list_size - 1) ? '\0' : ':'); + tests[33].value = value; + } + /* Same as before, but the last tunable values is differen than the + rest. */ + { + enum { tunables_list_size = array_length (tunable_list) }; + const char *value = ""; + for (int i = 0; i < tunables_list_size - 1; i++) + value = xasprintf ("%sglibc.malloc.check=2:", value); + value = xasprintf ("%sglibc.malloc.check=1", value); + tests[34].value = value; + } + /* Same as before, but with an invalid last entry. */ + { + enum { tunables_list_size = array_length (tunable_list) }; + const char *value = ""; + for (int i = 0; i < tunables_list_size - 1; i++) + value = xasprintf ("%sglibc.malloc.check=2:", value); + value = xasprintf ("%sglibc.malloc.check=1=1", value); + tests[35].value = value; + } + for (int i = 0; i < array_length (tests); i++) { snprintf (nteststr, sizeof nteststr, "%d", i); diff --git a/sysdeps/aarch64/multiarch/memset_generic.S b/sysdeps/aarch64/multiarch/memset_generic.S index 81748bdbce..e125a5ed85 100644 --- a/sysdeps/aarch64/multiarch/memset_generic.S +++ b/sysdeps/aarch64/multiarch/memset_generic.S @@ -33,3 +33,7 @@ #endif #include <../memset.S> + +#if IS_IN (rtld) +strong_alias (memset, __memset_generic) +#endif diff --git a/sysdeps/sparc/sparc64/rtld-memset.c b/sysdeps/sparc/sparc64/rtld-memset.c index 55f3835790..a19202a620 100644 --- a/sysdeps/sparc/sparc64/rtld-memset.c +++ b/sysdeps/sparc/sparc64/rtld-memset.c @@ -1 +1,4 @@ #include +#if IS_IN(rtld) +strong_alias (memset, __memset_ultra1) +#endif