From patchwork Thu May 2 16:35:56 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: 794111 Delivered-To: patch@linaro.org Received: by 2002:adf:a153:0:b0:34d:5089:5a9e with SMTP id r19csp345573wrr; Thu, 2 May 2024 09:37:46 -0700 (PDT) X-Forwarded-Encrypted: i=3; AJvYcCUZbL7MqcyZEF37f++tYQ1YGYGWR6JqX+S3SPYHIXh045rk3mhnViDtYDRIfL6FpgZ0FfFsBFhfrwJWfiIv1HTY X-Google-Smtp-Source: AGHT+IF4Xc2yqZFXU+NFbvm62fxXl4ro+AC0rhsENR1to4AhD+jYLuy9k2Ufo4ksFuKVP8wp+sIK X-Received: by 2002:a05:622a:5d2:b0:43a:5f5e:c10b with SMTP id d18-20020a05622a05d200b0043a5f5ec10bmr493463qtb.20.1714667865884; Thu, 02 May 2024 09:37:45 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1714667865; cv=pass; d=google.com; s=arc-20160816; b=dJrkTIYzJcPM9LF6mEggTrZZux0yn0uBx6Vz5lZM4h08a4j+vz440i/1nmfiAwImkb 7uQ/MUZkv9Bi8VqvZvI1qZ05EVrDRfQU3qjfcQqImq87bi3eT32iqfariUDB1Npvb0y+ KMgRtfsU31hMD9phlF6HHq2pPgas0fS474WIzoxZznOIlM6Zb0ULFoZc0AelCMnX9SG/ RqD7zxwOeqRWWl5ccWLcUFoSVDYJxxg7Vu5iMzbjguELMbWUZsCI+w2GGQlubud9az/f bSw8sPzwJn5ESSqqvsLx0jVVH1rBVG8m7YrkMFb4qqn5uBNwyjsUSQz8ab0Dyr8cOMbo N2pg== 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=TBKbR7Z7AJdSU8AYRfIKNSVobUw15f5XsE8/NePKAHI=; fh=oGn0EFORSY7ye2TnB7Yfd557GIAld9yH1tQXj05miNs=; b=DirqjJVWtlpWO6W8jSwoF4Y1TYmHTSOBxomYUlvu00rjjoZJHOkEo9KHOsftt9JoaE yE6FzP0q+YyaarlpeO0okvxbLCRsQKvglpyniqC7TKN+/jbF+jFhVGfkngDP5QHboJ6C pYZBIYKdXJQFuhfvBe2LWT4990QYldCkPX1047GMRcCO4ZT4j80poGyUiHai439nslZB 1AjMhBMNXT7ajzyuTj+bn9QvuHlSfbbED6zSRlYbWKKOPvd+sFAzc13BjoS3ZOHNB+AQ K9jsDEH5wg5NUrB3jKFtWHaZ1JTh8g1KTdQ/qCxZGhxHFer/dcJbMK6Htd9HfyxX5akK VDKg==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=NAjMTPh8; 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 br9-20020a05622a1e0900b0043a353ed3b9si1337696qtb.111.2024.05.02.09.37.45 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 May 2024 09:37:45 -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=NAjMTPh8; 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 81EBA384AB5D for ; Thu, 2 May 2024 16:37:45 +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 79FA5384AB4B for ; Thu, 2 May 2024 16:37:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 79FA5384AB4B 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 79FA5384AB4B 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=1714667853; cv=none; b=RgDnfQUKTCQcmZ9iYIjjJCGtiHr+ndVyp76tNwwgAArwoE3fPMQXVmj/p6+8lN9JttKCSNCkpqaDUilezTN7oLYPakQ1ab05zL95FXwfb0EwfVCtNOxmcNcPfooTGtgwmyz92LXNHKnukcAQggMcAietWIrNA9N3/9StBkajsjg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714667853; c=relaxed/simple; bh=mVQ7kqXp3Es1g2zzhVeHJKuiPX5wD6KZgx0DxUyjlMM=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=L2WZ6080/KO766QauEdYHWxAfy3/ObuPm1oGOv63mEt3wnC1SsiHdNV46NeD0cViGN2ZlPGP/7KRybJPUEypFG9dOyMDdz31n26JO//qplBKd4RenWhEaQJ9qwu2gzjEJl38vjFkdHcoNoavDM6gGgkJ2/Ugn6nle6hhMiuZg98= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x430.google.com with SMTP id d2e1a72fcca58-6f4302187c0so949478b3a.1 for ; Thu, 02 May 2024 09:37:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1714667849; x=1715272649; 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=TBKbR7Z7AJdSU8AYRfIKNSVobUw15f5XsE8/NePKAHI=; b=NAjMTPh8/ZMCSoObZvssRKZCY7GCtjOZxhrhtC5GnUkw4CfVEDvoFq+fTi1NKnMF0m +7ioTuqPJxzaWXyGiUyMsfAtOxjsKbi2/nUxL6pkY0wKU6JmmcAUmp9JGW6od2XM/s8d HbI5pq48Ynvkhw44SX6zkdbR1Xi8srkts7UARbN7WOSJu6tLcbgKVqOlR9B6izF57EO/ IenCTiwralN1oAxEchG71r+0wxf2fN9SrFMaKNzllRkxOt2r79JoivHzOPZM+RGX0Iw7 NBxKjnr1MQJgf/5lP8LZpxYdBlqmRE9bttu1s2C969trj/VNzyhCNgaByrYYTkkNC9D7 MAxg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714667849; x=1715272649; 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=TBKbR7Z7AJdSU8AYRfIKNSVobUw15f5XsE8/NePKAHI=; b=BgiCaKPQK3htuheHvN2O59VYmTGxLLomhQuxRxifgBSwLPZCTcr1pakga8xWApqBd7 /8/0LSK4reYFwVXjyn1YI+TE3vVEicrV9qq4I3S6ETC8G8sVbybmZwWlo0K4OFAeX9M0 ENvOfJo+RkdR51n4gAeV06YAkpLWBIM9F8oI55cJhH4Ny8VfC0YiA6IMd/WpPc1TGQOU pGh6EtQV3ngtd8jsXb7cCTLW3QSIp58953ulYOMYz+5rwSuYrAKN4iUnU1KQ/YtSnAyT qEWDOaavigTVyCnEJ87a0TxqILcwyBghiDjx+f7bnjvXk6QSLBFsivJKOc2nykEZ9ix6 X5ZA== X-Gm-Message-State: AOJu0Yz5nvKTNFn9cta7Ed58+aDVXLxIctRaoT7JqnMlC3O+vOTJ6sTn 7Aw//zDfMfz5dTt7r1+UFiMEOagUJyCBuMD17cXLWW/ybyGtIiHViIXrmM03g5rGIONIrHlVW0N A X-Received: by 2002:a05:6a20:5524:b0:1ad:8ae:7423 with SMTP id ko36-20020a056a20552400b001ad08ae7423mr215678pzb.30.1714667848648; Thu, 02 May 2024 09:37:28 -0700 (PDT) Received: from mandiga.. ([2804:1b3:a7c1:e3c5:e62b:fe17:6851:b93]) by smtp.gmail.com with ESMTPSA id j4-20020a62b604000000b006ecfa91a210sm1439524pff.100.2024.05.02.09.37.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 May 2024 09:37:28 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org Cc: Joe Simmons-Talbott , Siddhesh Poyarekar , Yuto Maeda , Yutaro Shimizu Subject: [PATCH v2 1/4] elf: Only process multiple tunable once (BZ 31686) Date: Thu, 2 May 2024 13:35:56 -0300 Message-ID: <20240502163716.1107975-2-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240502163716.1107975-1-adhemerval.zanella@linaro.org> References: <20240502163716.1107975-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=unavailable 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 --- elf/dl-tunables.c | 30 ++++++----- elf/tst-tunables.c | 58 +++++++++++++++++++++- sysdeps/aarch64/multiarch/memset_generic.S | 4 ++ sysdeps/sparc/sparc64/rtld-memset.c | 3 ++ 4 files changed, 81 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..8f4ee46ad5 100644 --- a/elf/tst-tunables.c +++ b/elf/tst-tunables.c @@ -17,6 +17,7 @@ . */ #include +#define TUNABLES_INTERNAL 1 #include #include #include @@ -24,12 +25,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 +286,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 +352,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 vallues 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