From patchwork Mon May 6 16:18:45 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 795022 Delivered-To: patch@linaro.org Received: by 2002:adf:a453:0:b0:34e:ceec:bfcd with SMTP id e19csp1006822wra; Mon, 6 May 2024 09:20:20 -0700 (PDT) X-Forwarded-Encrypted: i=3; AJvYcCWPiAvpSH1m7LkM5XXHzkYcAc2lnHmnhQDBebzT5mSDKwmY445dF2PCCMAAwOLqjBTGh3WWetPGa2pSokW9GGJe X-Google-Smtp-Source: AGHT+IELHIH8RdiSCZ2UTCntRh9eMHbR6W+cP4DT78KsjgX4axJl8QVlGak3r46lfS5q6eOUy5dp X-Received: by 2002:a67:b648:0:b0:47e:ef7a:2163 with SMTP id e8-20020a67b648000000b0047eef7a2163mr12833976vsm.5.1715012420018; Mon, 06 May 2024 09:20:20 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1715012420; cv=pass; d=google.com; s=arc-20160816; b=dFa69ncPSaSUV/2qp7pQ2Bcwl13aA7sa6YE2wn1rzMfclE9/qUtiY6d0l4+3NUG8iI EPxK/esiU9KyED2vq6esfzfa0OMwWq1BP3laQlPC7UKaef1XJbiNXAPmsemgQX0rOx1a kyb556+7ujPe/BI7Bej2XSRhMUsWLUE4Whez7q9aHo7xwtS1x70p85XFSmWkoYMgpVm8 UgvSppjYgcgDzQbZMsoUdsGogAHRb9R6FPHpoxWEyXsyrU+WeCTvtEIKeub/WeRlDyuL cxh+3pvyMqSlJoMErnnUR+OW+60uqeW7OfrGCU8wOAp1W670c+bINP+0lKe/d3DR2MPG K9MQ== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature:arc-filter:dmarc-filter:delivered-to; bh=JdIyqjgopqnPVPBcV8efwJkiueJ3EZUkKVX9lnewc4E=; fh=oGn0EFORSY7ye2TnB7Yfd557GIAld9yH1tQXj05miNs=; b=i5IzSeIETtuTbd8e8gysPyuqExwdhwahlvrZpjrTEKI2CzOtUs42/NzSW0Ojf6/ME5 1y2qIBQkN6FJqy6B3Lya6k0BIGD+YfygqYgMoJtw9sw5WQLK7q/GALqsMIxYNgQ3Sh/8 5D2VK9jVMUpaqX+rAb3JyiBVFnSIms+JN5V3RSYuAjChuts1f95JJf3uqO4ttv84GOGO Kpymiw0BacpzQeVXnpV7vncPkGVbu1QMxDHY9L5V7xrEhqY7PCZHTsW3pbJT0Tv4324B EXIeDQNOVJPkn6awOoicCX68BetM7QVdZwUUHo+Md3NsLcDMV3CIVs0FFCT69tpGvevK SBjg==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=aupmfQiv; arc=pass (i=1); spf=pass (google.com: domain of libc-alpha-bounces+patch=linaro.org@sourceware.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="libc-alpha-bounces+patch=linaro.org@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from server2.sourceware.org (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id x12-20020a67be0c000000b0047eebd25b42si1663289vsq.382.2024.05.06.09.20.19 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 May 2024 09:20:19 -0700 (PDT) Received-SPF: pass (google.com: domain of libc-alpha-bounces+patch=linaro.org@sourceware.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=aupmfQiv; arc=pass (i=1); spf=pass (google.com: domain of libc-alpha-bounces+patch=linaro.org@sourceware.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="libc-alpha-bounces+patch=linaro.org@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 73D9C3858CD1 for ; Mon, 6 May 2024 16:20:18 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-pf1-x430.google.com (mail-pf1-x430.google.com [IPv6:2607:f8b0:4864:20::430]) by sourceware.org (Postfix) with ESMTPS id 63CF23858C31 for ; Mon, 6 May 2024 16:20:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 63CF23858C31 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 63CF23858C31 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::430 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715012410; cv=none; b=mdRFmgaYqvJ0n7PK+4bCQPVZXbnWMBlNG2xU5h942/kGYpGcxPluddzrZPZZJAXOkk3GTpxK4BQkyatEh1GI34YR43iyQ9n/uxln+nezlucuQ+9o6bG74QwVBvM/kYmf66MhC8klYpiAgfjE64QjaQdr56I7A8AHVVCn0XM7gIQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715012410; c=relaxed/simple; bh=HvOqX6LamIK67ZZvV9jqTEoeNFy+a8dRyF+cVSgw728=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=U279HhL6nptY5j0QuhdDd4xA4bfz8UcLCKFJN6iH7SZI6mpwR/x6MWQ6nXz1lcgUZxsF/LpgiHyoKueo3ly2VdJinjiyax+8M8fg9inwOBrM2ZqFdxIeJYO581ztda5SA2V5IDHtATWgpIDl9QRK52TikjhrWWlsgTQLNdb0Lx0= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x430.google.com with SMTP id d2e1a72fcca58-6f4521ad6c0so1470509b3a.0 for ; Mon, 06 May 2024 09:20:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1715012405; x=1715617205; darn=sourceware.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=JdIyqjgopqnPVPBcV8efwJkiueJ3EZUkKVX9lnewc4E=; b=aupmfQivZ/MG5MKcpbTCWKGXUpYzXq7iZt37ehm0uNZ+EvOaoXmoUMjVB45+/FWrbl 2cGfUxNdGnRgowx+dVx9IJV4DGONja7li6SpXSHgTMe8Y/So2cd+YbsL8Qlu5ODRWc4F viCc9jmZUb+yyFrdirDJL1j54BNVrBg7sRC1lRuWLOMmOoytPpNJC72CX9GL6bU6zUh9 pXxkm+qCReZdrih31BW4ktzqERuK4AHrAxwcJ9TAJsuQLSAWY+YrApHSH0ZTgyBqaz6S TvpAagM+/6TilVU9BbjArei1Ecelk5mLn0F44CaFle7xpih0gY5pWtbv8idfXSBfaS8r IK5Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715012405; x=1715617205; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=JdIyqjgopqnPVPBcV8efwJkiueJ3EZUkKVX9lnewc4E=; b=KYvGdHYIsU64NWWC2CPg8bEf6QUF+tPrFye6SB23uOTvLsh3od3f0MzN7UTXq/aXpq pHCVhY+Yu9wNePAS2Dhlh5Bp5OiJ+RntvLuagmX2lbP/QBEtwl3jNHhMZrSN/uASWsjV BulKQEB+RX1WU66VyQ7kd1zp1mUvZ03vEEkkgfOovRMV0SXO5aPwD5bqYBEvd325hvWV WKU3QNtIQmwPWyyZ60zVh0Mb5z7txUFRYy8bPB3KZmyyP3NWMWEaz9O43XEd1/swNEZ0 gIqx9ict6e0PvdBhDqW45kxSxSqHBthzMa92snJImKdyOwjnkCifEhaToKAUf5+P3O/9 JcZg== X-Gm-Message-State: AOJu0YzpkHbpDraJY7QwnxR6tHtkoBWijc34vIqXmet40xLq8iioCSyF 8b/Wee5H/f+g+StwcqbYOfbqBSc2LqqiEglpGyjMBRQDer+TV7KZQpV1KtZxgTlLaPdFTekpb5T f5BY= X-Received: by 2002:a05:6a21:3a41:b0:1ad:8755:976 with SMTP id zu1-20020a056a213a4100b001ad87550976mr11401403pzb.13.1715012404742; Mon, 06 May 2024 09:20:04 -0700 (PDT) Received: from mandiga.. ([2804:1b3:a7c2:6e56:fc45:bb45:8b35:9b81]) by smtp.gmail.com with ESMTPSA id ll12-20020a056a00728c00b006ed0b798f1fsm7883347pfb.119.2024.05.06.09.20.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 May 2024 09:20:04 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org Cc: Joe Simmons-Talbott , Siddhesh Poyarekar , Yuto Maeda , Yutaro Shimizu Subject: [PATCH v3 1/4] elf: Only process multiple tunable once (BZ 31686) Date: Mon, 6 May 2024 13:18:45 -0300 Message-ID: <20240506161955.1570278-2-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240506161955.1570278-1-adhemerval.zanella@linaro.org> References: <20240506161955.1570278-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.1 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+patch=linaro.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 --- elf/dl-tunables.c | 30 ++++++----- elf/tst-tunables.c | 61 +++++++++++++++++++++- sysdeps/aarch64/multiarch/memset_generic.S | 4 ++ sysdeps/sparc/sparc64/rtld-memset.c | 3 ++ 4 files changed, 84 insertions(+), 14 deletions(-) diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index d3ccd2ecd4..1db80e0f92 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" @@ -221,8 +222,7 @@ parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables) if (tunable_is_name (cur->name, name)) { - tunables[ntunables++] = - (struct tunable_toset_t) { cur, value, p - value }; + tunables[i] = (struct tunable_toset_t) { cur, value, p - value }; /* Ignore tunables if enable_secure is set */ if (tunable_is_name ("glibc.rtld.enable_secure", name)) @@ -245,23 +245,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