* [PATCH v5 0/5] Improve loader environment variable handling
@ 2023-11-22 20:43 Adhemerval Zanella
2023-11-22 20:43 ` [PATCH v5 1/5] elf: Do not duplicate the GLIBC_TUNABLES string Adhemerval Zanella
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Adhemerval Zanella @ 2023-11-22 20:43 UTC (permalink / raw)
To: libc-alpha, siddhesh
The first patch removes the tunable_strdup and make the GLIBC_TUNABLE
parsing in place (no more possible allocation failure). The parsing now
tracks the tunable string start and its size. The dl-tunable-parse.h
adds helper functions to help to parse, like an strcmp that also checks
for size and an iterator for suboptions that are comma-separated
(used on hwcap parsing by x86, powerpc, and s390x).
The second and third patch make loader ignore all but just
LD_PRELOAD and LD_AUDIT for setuid binaries. For both options, loader
ensures that pathnames containing slashes are ignored and shared
libraries are loaded only from the standard search directories and only
if they have set-user-ID mode bit enabled.
Changes from v4:
* Improve tunables value handling, now warnings for invalid and out of
range numbers.
Changes from v3:
* Fixed tunable_initialize for strong aliases (it used the key length,
instead of the value length).
* Added a assert on tunable_str_comma_init to ensure its value is non
null.
* Added LD_WARN and LD_VERBOSE to filtered environment variables.
Changes from v2:
* Extend tst-tunables with tunables aliases tests.
* Use warning instead of an error to indicate invalid tunables.
* Fixed tunable_initialize for string aliases.
Changes from v1:
* Ignore most of the environment variables on security-sensitive mode.
* Extend tests.
Adhemerval Zanella (5):
elf: Do not duplicate the GLIBC_TUNABLES string
elf: Do not set invalid tunables values
elf: Ignore loader debug env vars for setuid
elf: Ignore LD_BIND_NOW and LD_BIND_NOT for setuid binaries
elf: Refactor process_envvars
elf/dl-misc.c | 5 +-
elf/dl-tunables.c | 123 +++++++------
elf/dl-tunables.h | 6 +-
elf/rtld.c | 108 ++++++++----
elf/tst-env-setuid.c | 8 +-
elf/tst-tunables.c | 96 +++++++++-
sysdeps/generic/dl-tunables-parse.h | 134 ++++++++++++++
sysdeps/generic/unsecvars.h | 4 +
sysdeps/s390/cpu-features.c | 165 +++++++-----------
.../unix/sysv/linux/aarch64/cpu-features.c | 33 ++--
.../unix/sysv/linux/powerpc/cpu-features.c | 45 ++---
.../sysv/linux/powerpc/tst-hwcap-tunables.c | 6 +-
sysdeps/x86/Makefile | 4 +-
sysdeps/x86/cpu-tunables.c | 118 +++++--------
sysdeps/x86/tst-hwcap-tunables.c | 148 ++++++++++++++++
15 files changed, 682 insertions(+), 321 deletions(-)
create mode 100644 sysdeps/generic/dl-tunables-parse.h
create mode 100644 sysdeps/x86/tst-hwcap-tunables.c
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 1/5] elf: Do not duplicate the GLIBC_TUNABLES string
2023-11-22 20:43 [PATCH v5 0/5] Improve loader environment variable handling Adhemerval Zanella
@ 2023-11-22 20:43 ` Adhemerval Zanella
2023-12-01 15:20 ` Siddhesh Poyarekar
2023-11-22 20:43 ` [PATCH v5 2/5] elf: Do not set invalid tunables values Adhemerval Zanella
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Adhemerval Zanella @ 2023-11-22 20:43 UTC (permalink / raw)
To: libc-alpha, siddhesh
The tunable parsing duplicates the tunable environment variable so it
null-terminates each one since it simplifies the later parsing. It has
the drawback of adding another point of failure (__minimal_malloc
failing), and the memory copy requires tuning the compiler to avoid mem
operations calls.
The parsing now tracks the tunable start and its size. The
dl-tunable-parse.h adds helper functions to help parsing, like a strcmp
that also checks for size and an iterator for suboptions that are
comma-separated (used on hwcap parsing by x86, powerpc, and s390x).
Since the environment variable is allocated on the stack by the kernel,
it is safe to keep the references to the suboptions for later parsing
of string tunables (as done by set_hwcaps by multiple architectures).
Checked on x86_64-linux-gnu, powerpc64le-linux-gnu, and
aarch64-linux-gnu.
---
elf/dl-tunables.c | 90 +++++-----
elf/dl-tunables.h | 6 +-
elf/tst-tunables.c | 66 ++++++-
sysdeps/generic/dl-tunables-parse.h | 134 ++++++++++++++
sysdeps/s390/cpu-features.c | 165 +++++++-----------
.../unix/sysv/linux/aarch64/cpu-features.c | 33 ++--
.../unix/sysv/linux/powerpc/cpu-features.c | 45 ++---
.../sysv/linux/powerpc/tst-hwcap-tunables.c | 6 +-
sysdeps/x86/Makefile | 4 +-
sysdeps/x86/cpu-tunables.c | 118 +++++--------
sysdeps/x86/tst-hwcap-tunables.c | 148 ++++++++++++++++
11 files changed, 532 insertions(+), 283 deletions(-)
create mode 100644 sysdeps/generic/dl-tunables-parse.h
create mode 100644 sysdeps/x86/tst-hwcap-tunables.c
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 83265bc00b..26161c68e7 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -36,48 +36,32 @@
#define TUNABLES_INTERNAL 1
#include "dl-tunables.h"
-#include <not-errno.h>
-
-static char *
-tunables_strdup (const char *in)
-{
- size_t i = 0;
-
- while (in[i++] != '\0');
- char *out = __minimal_malloc (i + 1);
-
- /* For most of the tunables code, we ignore user errors. However,
- this is a system error - and running out of memory at program
- startup should be reported, so we do. */
- if (out == NULL)
- _dl_fatal_printf ("failed to allocate memory to process tunables\n");
-
- while (i-- > 0)
- out[i] = in[i];
-
- return out;
-}
-
static char **
-get_next_env (char **envp, char **name, size_t *namelen, char **val,
+get_next_env (char **envp, char **name, char **val, size_t *vallen,
char ***prev_envp)
{
while (envp != NULL && *envp != NULL)
{
char **prev = envp;
char *envline = *envp++;
- int len = 0;
+ char *penv = envline;
+ size_t len;
- while (envline[len] != '\0' && envline[len] != '=')
- len++;
+ for (; *penv != '\0' && *penv != '='; penv++);
/* Just the name and no value, go to the next one. */
- if (envline[len] == '\0')
+ if (*penv == '\0')
continue;
*name = envline;
- *namelen = len;
- *val = &envline[len + 1];
+ /* Skip '='. */
+ *val = ++penv;
+
+ len = 0;
+ while (*penv++ != '\0')
+ len++;
+ *vallen = len;
+
*prev_envp = prev;
return envp;
@@ -134,14 +118,14 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
/* Validate range of the input value and initialize the tunable CUR if it looks
good. */
static void
-tunable_initialize (tunable_t *cur, const char *strval)
+tunable_initialize (tunable_t *cur, const char *strval, size_t len)
{
- tunable_val_t val;
+ tunable_val_t val = { 0 };
if (cur->type.type_code != TUNABLE_TYPE_STRING)
val.numval = (tunable_num_t) _dl_strtoul (strval, NULL);
else
- val.strval = strval;
+ val.strval = (struct tunable_str_t) { strval, len };
do_tunable_update_val (cur, &val, NULL, NULL);
}
@@ -158,29 +142,29 @@ struct tunable_toset_t
{
tunable_t *t;
const char *value;
+ size_t len;
};
enum { tunables_list_size = array_length (tunable_list) };
/* Parse the tunable string VALSTRING and set TUNABLES with the found tunables
- and their respective strings. VALSTRING is a duplicated values, where
- delimiters ':' are replaced with '\0', so string tunables are null
- terminated.
+ and their respectibles values. The VALSTRING is parsed in place, with the
+ tunable start and size recorded in TUNABLES.
Return the number of tunables found (including 0 if the string is empty)
or -1 if for an ill-formatted definition. */
static int
-parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
+parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables)
{
if (valstring == NULL || *valstring == '\0')
return 0;
- char *p = valstring;
+ const char *p = valstring;
bool done = false;
int ntunables = 0;
while (!done)
{
- char *name = p;
+ const char *name = p;
/* First, find where the name ends. */
while (*p != '=' && *p != ':' && *p != '\0')
@@ -202,7 +186,7 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
/* Skip the '='. */
p++;
- char *value = p;
+ const char *value = p;
while (*p != '=' && *p != ':' && *p != '\0')
p++;
@@ -211,8 +195,6 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
return -1;
else if (*p == '\0')
done = true;
- else
- *p++ = '\0';
/* Add the tunable if it exists. */
for (size_t i = 0; i < tunables_list_size; i++)
@@ -221,7 +203,8 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
if (tunable_is_name (cur->name, name))
{
- tunables[ntunables++] = (struct tunable_toset_t) { cur, value };
+ tunables[ntunables++] =
+ (struct tunable_toset_t) { cur, value, p - value };
break;
}
}
@@ -231,7 +214,7 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
}
static void
-parse_tunables (char *valstring)
+parse_tunables (const char *valstring)
{
struct tunable_toset_t tunables[tunables_list_size];
int ntunables = parse_tunables_string (valstring, tunables);
@@ -243,7 +226,7 @@ parse_tunables (char *valstring)
}
for (int i = 0; i < ntunables; i++)
- tunable_initialize (tunables[i].t, tunables[i].value);
+ tunable_initialize (tunables[i].t, tunables[i].value, tunables[i].len);
}
/* Initialize the tunables list from the environment. For now we only use the
@@ -254,19 +237,22 @@ __tunables_init (char **envp)
{
char *envname = NULL;
char *envval = NULL;
- size_t len = 0;
+ size_t envvallen = 0;
char **prev_envp = envp;
/* Ignore tunables for AT_SECURE programs. */
if (__libc_enable_secure)
return;
- while ((envp = get_next_env (envp, &envname, &len, &envval,
+ while ((envp = get_next_env (envp, &envname, &envval, &envvallen,
&prev_envp)) != NULL)
{
+ /* The environment variable is allocated on the stack by the kernel, so
+ it is safe to keep the references to the suboptions for later parsing
+ of string tunables. */
if (tunable_is_name ("GLIBC_TUNABLES", envname))
{
- parse_tunables (tunables_strdup (envval));
+ parse_tunables (envval);
continue;
}
@@ -284,7 +270,7 @@ __tunables_init (char **envp)
/* We have a match. Initialize and move on to the next line. */
if (tunable_is_name (name, envname))
{
- tunable_initialize (cur, envval);
+ tunable_initialize (cur, envval, envvallen);
break;
}
}
@@ -298,7 +284,7 @@ __tunables_print (void)
{
const tunable_t *cur = &tunable_list[i];
if (cur->type.type_code == TUNABLE_TYPE_STRING
- && cur->val.strval == NULL)
+ && cur->val.strval.str == NULL)
_dl_printf ("%s:\n", cur->name);
else
{
@@ -324,7 +310,9 @@ __tunables_print (void)
(size_t) cur->type.max);
break;
case TUNABLE_TYPE_STRING:
- _dl_printf ("%s\n", cur->val.strval);
+ _dl_printf ("%.*s\n",
+ (int) cur->val.strval.len,
+ cur->val.strval.str);
break;
default:
__builtin_unreachable ();
@@ -359,7 +347,7 @@ __tunable_get_val (tunable_id_t id, void *valp, tunable_callback_t callback)
}
case TUNABLE_TYPE_STRING:
{
- *((const char **)valp) = cur->val.strval;
+ *((struct tunable_str_t **) valp) = &cur->val.strval;
break;
}
default:
diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
index 45c191e021..0e777d7d37 100644
--- a/elf/dl-tunables.h
+++ b/elf/dl-tunables.h
@@ -30,7 +30,11 @@ typedef intmax_t tunable_num_t;
typedef union
{
tunable_num_t numval;
- const char *strval;
+ struct tunable_str_t
+ {
+ const char *str;
+ size_t len;
+ } strval;
} tunable_val_t;
typedef void (*tunable_callback_t) (tunable_val_t *);
diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
index e1ad44f27c..188345b070 100644
--- a/elf/tst-tunables.c
+++ b/elf/tst-tunables.c
@@ -31,7 +31,8 @@ static int restart;
static const struct test_t
{
- const char *env;
+ const char *name;
+ const char *value;
int32_t expected_malloc_check;
size_t expected_mmap_threshold;
int32_t expected_perturb;
@@ -39,12 +40,14 @@ static const struct test_t
{
/* Expected tunable format. */
{
+ "GLIBC_TUNABLES",
"glibc.malloc.check=2",
2,
0,
0,
},
{
+ "GLIBC_TUNABLES",
"glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
2,
4096,
@@ -52,6 +55,7 @@ static const struct test_t
},
/* Empty tunable are ignored. */
{
+ "GLIBC_TUNABLES",
"glibc.malloc.check=2::glibc.malloc.mmap_threshold=4096",
2,
4096,
@@ -59,6 +63,7 @@ static const struct test_t
},
/* As well empty values. */
{
+ "GLIBC_TUNABLES",
"glibc.malloc.check=:glibc.malloc.mmap_threshold=4096",
0,
4096,
@@ -66,18 +71,21 @@ static const struct test_t
},
/* Tunable are processed from left to right, so last one is the one set. */
{
+ "GLIBC_TUNABLES",
"glibc.malloc.check=1:glibc.malloc.check=2",
2,
0,
0,
},
{
+ "GLIBC_TUNABLES",
"glibc.malloc.check=1:glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
2,
4096,
0,
},
{
+ "GLIBC_TUNABLES",
"glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096:glibc.malloc.check=1",
1,
4096,
@@ -85,12 +93,14 @@ static const struct test_t
},
/* 0x800 is larger than tunable maxval (0xff), so the tunable is unchanged. */
{
+ "GLIBC_TUNABLES",
"glibc.malloc.perturb=0x800",
0,
0,
0,
},
{
+ "GLIBC_TUNABLES",
"glibc.malloc.perturb=0x55",
0,
0,
@@ -98,6 +108,7 @@ static const struct test_t
},
/* Out of range values are just ignored. */
{
+ "GLIBC_TUNABLES",
"glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
0,
4096,
@@ -105,24 +116,28 @@ static const struct test_t
},
/* Invalid keys are ignored. */
{
+ "GLIBC_TUNABLES",
":glibc.malloc.garbage=2:glibc.malloc.check=1",
1,
0,
0,
},
{
+ "GLIBC_TUNABLES",
"glibc.malloc.perturb=0x800:not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",
0,
4096,
0,
},
{
+ "GLIBC_TUNABLES",
"glibc.not_valid.check=2:glibc.malloc.mmap_threshold=4096",
0,
4096,
0,
},
{
+ "GLIBC_TUNABLES",
"not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",
0,
4096,
@@ -130,24 +145,28 @@ static const struct test_t
},
/* Invalid subkeys are ignored. */
{
+ "GLIBC_TUNABLES",
"glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096:glibc.malloc.check=2",
2,
0,
0,
},
{
+ "GLIBC_TUNABLES",
"glibc.malloc.check=4:glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096",
0,
0,
0,
},
{
+ "GLIBC_TUNABLES",
"not_valid.malloc.check=2",
0,
0,
0,
},
{
+ "GLIBC_TUNABLES",
"glibc.not_valid.check=2",
0,
0,
@@ -156,6 +175,7 @@ static const struct test_t
/* An ill-formatted tunable in the for key=key=value will considere the
value as 'key=value' (which can not be parsed as an integer). */
{
+ "GLIBC_TUNABLES",
"glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096",
0,
0,
@@ -163,41 +183,77 @@ static const struct test_t
},
/* Ill-formatted tunables string is not parsed. */
{
+ "GLIBC_TUNABLES",
"glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096:glibc.malloc.check=2",
0,
0,
0,
},
{
+ "GLIBC_TUNABLES",
"glibc.malloc.check=2=2",
0,
0,
0,
},
{
+ "GLIBC_TUNABLES",
"glibc.malloc.check=2=2:glibc.malloc.mmap_threshold=4096",
0,
0,
0,
},
{
+ "GLIBC_TUNABLES",
"glibc.malloc.check=2=2:glibc.malloc.check=2",
0,
0,
0,
},
{
+ "GLIBC_TUNABLES",
"glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096=4096",
0,
0,
0,
},
{
+ "GLIBC_TUNABLES",
"glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096=4096",
0,
0,
0,
},
+ /* Also check some tunable aliases. */
+ {
+ "MALLOC_CHECK_",
+ "2",
+ 2,
+ 0,
+ 0,
+ },
+ {
+ "MALLOC_MMAP_THRESHOLD_",
+ "4096",
+ 0,
+ 4096,
+ 0,
+ },
+ {
+ "MALLOC_PERTURB_",
+ "0x55",
+ 0,
+ 0,
+ 0x55,
+ },
+ /* 0x800 is larger than tunable maxval (0xff), so the tunable is unchanged. */
+ {
+ "MALLOC_PERTURB_",
+ "0x800",
+ 0,
+ 0,
+ 0,
+ },
};
static int
@@ -245,13 +301,17 @@ do_test (int argc, char *argv[])
{
snprintf (nteststr, sizeof nteststr, "%d", i);
- printf ("[%d] Spawned test for %s\n", i, tests[i].env);
- setenv ("GLIBC_TUNABLES", tests[i].env, 1);
+ printf ("[%d] Spawned test for %s=%s\n",
+ i,
+ tests[i].name,
+ tests[i].value);
+ setenv (tests[i].name, tests[i].value, 1);
struct support_capture_subprocess result
= support_capture_subprogram (spargv[0], spargv);
support_capture_subprocess_check (&result, "tst-tunables", 0,
sc_allow_stderr);
support_capture_subprocess_free (&result);
+ unsetenv (tests[i].name);
}
return 0;
diff --git a/sysdeps/generic/dl-tunables-parse.h b/sysdeps/generic/dl-tunables-parse.h
new file mode 100644
index 0000000000..b37be0443b
--- /dev/null
+++ b/sysdeps/generic/dl-tunables-parse.h
@@ -0,0 +1,134 @@
+/* Helper functions to handle tunable strings.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _DL_TUNABLES_PARSE_H
+#define _DL_TUNABLES_PARSE_H 1
+
+#include <assert.h>
+#include <string.h>
+
+/* Compare the contents of STRVAL with STR of size LEN. The STR might not
+ be null-terminated. */
+static __always_inline bool
+tunable_strcmp (const struct tunable_str_t *strval, const char *str,
+ size_t len)
+{
+ return strval->len == len && memcmp (strval->str, str, len) == 0;
+}
+#define tunable_strcmp_cte(__tunable, __str) \
+ tunable_strcmp (&__tunable->strval, __str, sizeof (__str) - 1)
+
+/*
+ Helper functions to iterate over a tunable string composed by multiple
+ suboptions separated by commaxi; this is a common pattern for CPU. Each
+ suboptions is return in the form of { address, size } (no null terminated).
+ For instance:
+
+ struct tunable_str_comma_t ts;
+ tunable_str_comma_init (&ts, valp);
+
+ struct tunable_str_t t;
+ while (tunable_str_comma_next (&ts, &t))
+ {
+ _dl_printf ("[%s] %.*s (%d)\n",
+ __func__,
+ (int) tstr.len,
+ tstr.str,
+ (int) tstr.len);
+
+ if (tunable_str_comma_strcmp (&t, opt, opt1_len))
+ {
+ [...]
+ }
+ else if (tunable_str_comma_strcmp_cte (&t, "opt2"))
+ {
+ [...]
+ }
+ }
+
+ NB: These function are expected to be called from tunable callback
+ functions along with tunable_val_t with string types.
+*/
+
+struct tunable_str_comma_state_t
+{
+ const char *p;
+ size_t plen;
+ size_t maxplen;
+};
+
+struct tunable_str_comma_t
+{
+ const char *str;
+ size_t len;
+ bool disable;
+};
+
+static inline void
+tunable_str_comma_init (struct tunable_str_comma_state_t *state,
+ tunable_val_t *valp)
+{
+ assert (valp->strval.str != NULL);
+ state->p = valp->strval.str;
+ state->plen = 0;
+ state->maxplen = valp->strval.len;
+}
+
+static inline bool
+tunable_str_comma_next (struct tunable_str_comma_state_t *state,
+ struct tunable_str_comma_t *str)
+{
+ if (*state->p == '\0' || state->plen >= state->maxplen)
+ return false;
+
+ const char *c;
+ for (c = state->p; *c != ','; c++, state->plen++)
+ if (*c == '\0' || state->plen == state->maxplen)
+ break;
+
+ str->str = state->p;
+ str->len = c - state->p;
+
+ if (str->len > 0)
+ {
+ str->disable = *str->str == '-';
+ if (str->disable)
+ {
+ str->str = str->str + 1;
+ str->len = str->len - 1;
+ }
+ }
+
+ state->p = c + 1;
+ state->plen++;
+
+ return true;
+}
+
+/* Compare the contents of T with STR of size LEN. The STR might not be
+ null-terminated. */
+static __always_inline bool
+tunable_str_comma_strcmp (const struct tunable_str_comma_t *t, const char *str,
+ size_t len)
+{
+ return t->len == len && memcmp (t->str, str, len) == 0;
+}
+#define tunable_str_comma_strcmp_cte(__t, __str) \
+ tunable_str_comma_strcmp (__t, __str, sizeof (__str) - 1)
+
+#endif
diff --git a/sysdeps/s390/cpu-features.c b/sysdeps/s390/cpu-features.c
index 55449ba07f..06c1cab0fd 100644
--- a/sysdeps/s390/cpu-features.c
+++ b/sysdeps/s390/cpu-features.c
@@ -22,6 +22,7 @@
#include <ifunc-memcmp.h>
#include <string.h>
#include <dl-symbol-redir-ifunc.h>
+#include <dl-tunables-parse.h>
#define S390_COPY_CPU_FEATURES(SRC_PTR, DEST_PTR) \
(DEST_PTR)->hwcap = (SRC_PTR)->hwcap; \
@@ -51,33 +52,14 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
struct cpu_features cpu_features_curr;
S390_COPY_CPU_FEATURES (cpu_features, &cpu_features_curr);
- const char *token = valp->strval;
- do
+ struct tunable_str_comma_state_t ts;
+ tunable_str_comma_init (&ts, valp);
+
+ struct tunable_str_comma_t t;
+ while (tunable_str_comma_next (&ts, &t))
{
- const char *token_end, *feature;
- bool disable;
- size_t token_len;
- size_t feature_len;
-
- /* Find token separator or end of string. */
- for (token_end = token; *token_end != ','; token_end++)
- if (*token_end == '\0')
- break;
-
- /* Determine feature. */
- token_len = token_end - token;
- if (*token == '-')
- {
- disable = true;
- feature = token + 1;
- feature_len = token_len - 1;
- }
- else
- {
- disable = false;
- feature = token;
- feature_len = token_len;
- }
+ if (t.len == 0)
+ continue;
/* Handle only the features here which are really used in the
IFUNC-resolvers. All others are ignored as the values are only used
@@ -85,86 +67,64 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
bool reset_features = false;
unsigned long int hwcap_mask = 0UL;
unsigned long long stfle_bits0_mask = 0ULL;
+ bool disable = t.disable;
- if ((*feature == 'z' || *feature == 'a'))
+ if (tunable_str_comma_strcmp_cte (&t, "zEC12")
+ || tunable_str_comma_strcmp_cte (&t, "arch10"))
+ {
+ reset_features = true;
+ disable = true;
+ hwcap_mask = HWCAP_S390_VXRS | HWCAP_S390_VXRS_EXT
+ | HWCAP_S390_VXRS_EXT2;
+ stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
+ }
+ else if (tunable_str_comma_strcmp_cte (&t, "z13")
+ || tunable_str_comma_strcmp_cte (&t, "arch11"))
+ {
+ reset_features = true;
+ disable = true;
+ hwcap_mask = HWCAP_S390_VXRS_EXT | HWCAP_S390_VXRS_EXT2;
+ stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
+ }
+ else if (tunable_str_comma_strcmp_cte (&t, "z14")
+ || tunable_str_comma_strcmp_cte (&t, "arch12"))
+ {
+ reset_features = true;
+ disable = true;
+ hwcap_mask = HWCAP_S390_VXRS_EXT2;
+ stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
+ }
+ else if (tunable_str_comma_strcmp_cte (&t, "z15")
+ || tunable_str_comma_strcmp_cte (&t, "z16")
+ || tunable_str_comma_strcmp_cte (&t, "arch13")
+ || tunable_str_comma_strcmp_cte (&t, "arch14"))
{
- if ((feature_len == 5 && *feature == 'z'
- && memcmp (feature, "zEC12", 5) == 0)
- || (feature_len == 6 && *feature == 'a'
- && memcmp (feature, "arch10", 6) == 0))
- {
- reset_features = true;
- disable = true;
- hwcap_mask = HWCAP_S390_VXRS | HWCAP_S390_VXRS_EXT
- | HWCAP_S390_VXRS_EXT2;
- stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
- }
- else if ((feature_len == 3 && *feature == 'z'
- && memcmp (feature, "z13", 3) == 0)
- || (feature_len == 6 && *feature == 'a'
- && memcmp (feature, "arch11", 6) == 0))
- {
- reset_features = true;
- disable = true;
- hwcap_mask = HWCAP_S390_VXRS_EXT | HWCAP_S390_VXRS_EXT2;
- stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
- }
- else if ((feature_len == 3 && *feature == 'z'
- && memcmp (feature, "z14", 3) == 0)
- || (feature_len == 6 && *feature == 'a'
- && memcmp (feature, "arch12", 6) == 0))
- {
- reset_features = true;
- disable = true;
- hwcap_mask = HWCAP_S390_VXRS_EXT2;
- stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
- }
- else if ((feature_len == 3 && *feature == 'z'
- && (memcmp (feature, "z15", 3) == 0
- || memcmp (feature, "z16", 3) == 0))
- || (feature_len == 6
- && (memcmp (feature, "arch13", 6) == 0
- || memcmp (feature, "arch14", 6) == 0)))
- {
- /* For z15 or newer we don't have to disable something,
- but we have to reset to the original values. */
- reset_features = true;
- }
+ /* For z15 or newer we don't have to disable something, but we have
+ to reset to the original values. */
+ reset_features = true;
}
- else if (*feature == 'H')
+ else if (tunable_str_comma_strcmp_cte (&t, "HWCAP_S390_VXRS"))
{
- if (feature_len == 15
- && memcmp (feature, "HWCAP_S390_VXRS", 15) == 0)
- {
- hwcap_mask = HWCAP_S390_VXRS;
- if (disable)
- hwcap_mask |= HWCAP_S390_VXRS_EXT | HWCAP_S390_VXRS_EXT2;
- }
- else if (feature_len == 19
- && memcmp (feature, "HWCAP_S390_VXRS_EXT", 19) == 0)
- {
- hwcap_mask = HWCAP_S390_VXRS_EXT;
- if (disable)
- hwcap_mask |= HWCAP_S390_VXRS_EXT2;
- else
- hwcap_mask |= HWCAP_S390_VXRS;
- }
- else if (feature_len == 20
- && memcmp (feature, "HWCAP_S390_VXRS_EXT2", 20) == 0)
- {
- hwcap_mask = HWCAP_S390_VXRS_EXT2;
- if (!disable)
- hwcap_mask |= HWCAP_S390_VXRS | HWCAP_S390_VXRS_EXT;
- }
+ hwcap_mask = HWCAP_S390_VXRS;
+ if (t.disable)
+ hwcap_mask |= HWCAP_S390_VXRS_EXT | HWCAP_S390_VXRS_EXT2;
}
- else if (*feature == 'S')
+ else if (tunable_str_comma_strcmp_cte (&t, "HWCAP_S390_VXRS_EXT"))
{
- if (feature_len == 10
- && memcmp (feature, "STFLE_MIE3", 10) == 0)
- {
- stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
- }
+ hwcap_mask = HWCAP_S390_VXRS_EXT;
+ if (t.disable)
+ hwcap_mask |= HWCAP_S390_VXRS_EXT2;
+ else
+ hwcap_mask |= HWCAP_S390_VXRS;
+ }
+ else if (tunable_str_comma_strcmp_cte (&t, "HWCAP_S390_VXRS_EXT2"))
+ {
+ hwcap_mask = HWCAP_S390_VXRS_EXT2;
+ if (!t.disable)
+ hwcap_mask |= HWCAP_S390_VXRS | HWCAP_S390_VXRS_EXT;
}
+ else if (tunable_str_comma_strcmp_cte (&t, "STFLE_MIE3"))
+ stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
/* Perform the actions determined above. */
if (reset_features)
@@ -187,14 +147,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
else
cpu_features_curr.stfle_bits[0] |= stfle_bits0_mask;
}
-
- /* Jump over current token ... */
- token += token_len;
-
- /* ... and skip token separator for next round. */
- if (*token == ',') token++;
}
- while (*token != '\0');
/* Copy back the features after checking that no unsupported features were
enabled by user. */
diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
index a11a86efab..c57f154b48 100644
--- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
+++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
@@ -16,10 +16,12 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
+#include <array_length.h>
#include <cpu-features.h>
#include <sys/auxv.h>
#include <elf/dl-hwcaps.h>
#include <sys/prctl.h>
+#include <dl-tunables-parse.h>
#define DCZID_DZP_MASK (1 << 4)
#define DCZID_BS_MASK (0xf)
@@ -33,25 +35,28 @@
struct cpu_list
{
const char *name;
+ size_t len;
uint64_t midr;
};
-static struct cpu_list cpu_list[] = {
- {"thunderxt88", 0x430F0A10},
- {"thunderx2t99", 0x431F0AF0},
- {"thunderx2t99p1", 0x420F5160},
- {"ares", 0x411FD0C0},
- {"emag", 0x503F0001},
- {"kunpeng920", 0x481FD010},
- {"a64fx", 0x460F0010},
- {"generic", 0x0}
+static const struct cpu_list cpu_list[] =
+{
+#define CPU_LIST_ENTRY(__str, __num) { __str, sizeof (__str) - 1, __num }
+ CPU_LIST_ENTRY ("thunderxt88", 0x430F0A10),
+ CPU_LIST_ENTRY ("thunderx2t99", 0x431F0AF0),
+ CPU_LIST_ENTRY ("thunderx2t99p1", 0x420F5160),
+ CPU_LIST_ENTRY ("ares", 0x411FD0C0),
+ CPU_LIST_ENTRY ("emag", 0x503F0001),
+ CPU_LIST_ENTRY ("kunpeng920", 0x481FD010),
+ CPU_LIST_ENTRY ("a64fx", 0x460F0010),
+ CPU_LIST_ENTRY ("generic", 0x0),
};
static uint64_t
-get_midr_from_mcpu (const char *mcpu)
+get_midr_from_mcpu (const struct tunable_str_t *mcpu)
{
- for (int i = 0; i < sizeof (cpu_list) / sizeof (struct cpu_list); i++)
- if (strcmp (mcpu, cpu_list[i].name) == 0)
+ for (int i = 0; i < array_length (cpu_list); i++)
+ if (tunable_strcmp (mcpu, cpu_list[i].name, cpu_list[i].len))
return cpu_list[i].midr;
return UINT64_MAX;
@@ -63,7 +68,9 @@ init_cpu_features (struct cpu_features *cpu_features)
register uint64_t midr = UINT64_MAX;
/* Get the tunable override. */
- const char *mcpu = TUNABLE_GET (glibc, cpu, name, const char *, NULL);
+ const struct tunable_str_t *mcpu = TUNABLE_GET (glibc, cpu, name,
+ struct tunable_str_t *,
+ NULL);
if (mcpu != NULL)
midr = get_midr_from_mcpu (mcpu);
diff --git a/sysdeps/unix/sysv/linux/powerpc/cpu-features.c b/sysdeps/unix/sysv/linux/powerpc/cpu-features.c
index 7c6e20e702..390b3fd11a 100644
--- a/sysdeps/unix/sysv/linux/powerpc/cpu-features.c
+++ b/sysdeps/unix/sysv/linux/powerpc/cpu-features.c
@@ -20,6 +20,7 @@
#include <stdint.h>
#include <cpu-features.h>
#include <elf/dl-tunables.h>
+#include <dl-tunables-parse.h>
#include <unistd.h>
#include <string.h>
@@ -43,41 +44,26 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
struct cpu_features *cpu_features = &GLRO(dl_powerpc_cpu_features);
unsigned long int tcbv_hwcap = cpu_features->hwcap;
unsigned long int tcbv_hwcap2 = cpu_features->hwcap2;
- const char *token = valp->strval;
- do
+
+ struct tunable_str_comma_state_t ts;
+ tunable_str_comma_init (&ts, valp);
+
+ struct tunable_str_comma_t t;
+ while (tunable_str_comma_next (&ts, &t))
{
- const char *token_end, *feature;
- bool disable;
- size_t token_len, i, feature_len, offset = 0;
- /* Find token separator or end of string. */
- for (token_end = token; *token_end != ','; token_end++)
- if (*token_end == '\0')
- break;
+ if (t.len == 0)
+ continue;
- /* Determine feature. */
- token_len = token_end - token;
- if (*token == '-')
- {
- disable = true;
- feature = token + 1;
- feature_len = token_len - 1;
- }
- else
- {
- disable = false;
- feature = token;
- feature_len = token_len;
- }
- for (i = 0; i < array_length (hwcap_tunables); ++i)
+ size_t offset = 0;
+ for (int i = 0; i < array_length (hwcap_tunables); ++i)
{
const char *hwcap_name = hwcap_names + offset;
size_t hwcap_name_len = strlen (hwcap_name);
/* Check the tunable name on the supported list. */
- if (hwcap_name_len == feature_len
- && memcmp (feature, hwcap_name, feature_len) == 0)
+ if (tunable_str_comma_strcmp (&t, hwcap_name, hwcap_name_len))
{
/* Update the hwcap and hwcap2 bits. */
- if (disable)
+ if (t.disable)
{
/* Id is 1 for hwcap2 tunable. */
if (hwcap_tunables[i].id)
@@ -98,12 +84,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
}
offset += hwcap_name_len + 1;
}
- token += token_len;
- /* ... and skip token separator for next round. */
- if (*token == ',')
- token++;
}
- while (*token != '\0');
}
static inline void
diff --git a/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c b/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c
index 2631016a3a..049164f841 100644
--- a/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c
+++ b/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c
@@ -110,7 +110,11 @@ do_test (int argc, char *argv[])
run_test ("-arch_2_06", "__memcpy_power7");
if (hwcap & PPC_FEATURE_ARCH_2_05)
run_test ("-arch_2_06,-arch_2_05","__memcpy_power6");
- run_test ("-arch_2_06,-arch_2_05,-power5+,-power5,-power4", "__memcpy_power4");
+ run_test ("-arch_2_06,-arch_2_05,-power5+,-power5,-power4",
+ "__memcpy_power4");
+ /* Also run with valid, but empty settings. */
+ run_test (",-,-arch_2_06,-arch_2_05,-power5+,-power5,,-power4,-",
+ "__memcpy_power4");
}
else
{
diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index 917c26f116..a64e5f002a 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -12,7 +12,8 @@ CFLAGS-get-cpuid-feature-leaf.o += $(no-stack-protector)
tests += tst-get-cpu-features tst-get-cpu-features-static \
tst-cpu-features-cpuinfo tst-cpu-features-cpuinfo-static \
- tst-cpu-features-supports tst-cpu-features-supports-static
+ tst-cpu-features-supports tst-cpu-features-supports-static \
+ tst-hwcap-tunables
tests-static += tst-get-cpu-features-static \
tst-cpu-features-cpuinfo-static \
tst-cpu-features-supports-static
@@ -65,6 +66,7 @@ $(objpfx)tst-isa-level-1.out: $(objpfx)tst-isa-level-mod-1-baseline.so \
endif
tst-ifunc-isa-2-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SSE4_2,-AVX,-AVX2,-AVX512F
tst-ifunc-isa-2-static-ENV = $(tst-ifunc-isa-2-ENV)
+tst-hwcap-tunables-ARGS = -- $(host-test-program-cmd)
endif
ifeq ($(subdir),math)
diff --git a/sysdeps/x86/cpu-tunables.c b/sysdeps/x86/cpu-tunables.c
index 5697885226..ef96148d30 100644
--- a/sysdeps/x86/cpu-tunables.c
+++ b/sysdeps/x86/cpu-tunables.c
@@ -24,11 +24,12 @@
#include <string.h>
#include <cpu-features.h>
#include <ldsodefs.h>
+#include <dl-tunables-parse.h>
#include <dl-symbol-redir-ifunc.h>
#define CHECK_GLIBC_IFUNC_CPU_OFF(f, cpu_features, name, len) \
_Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
- if (memcmp (f, #name, len) == 0) \
+ if (tunable_str_comma_strcmp_cte (&f, #name)) \
{ \
CPU_FEATURE_UNSET (cpu_features, name) \
break; \
@@ -38,7 +39,7 @@
which isn't available. */
#define CHECK_GLIBC_IFUNC_PREFERRED_OFF(f, cpu_features, name, len) \
_Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
- if (memcmp (f, #name, len) == 0) \
+ if (tunable_str_comma_strcmp_cte (&f, #name) == 0) \
{ \
cpu_features->preferred[index_arch_##name] \
&= ~bit_arch_##name; \
@@ -46,12 +47,11 @@
}
/* Enable/disable a preferred feature NAME. */
-#define CHECK_GLIBC_IFUNC_PREFERRED_BOTH(f, cpu_features, name, \
- disable, len) \
+#define CHECK_GLIBC_IFUNC_PREFERRED_BOTH(f, cpu_features, name, len) \
_Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
- if (memcmp (f, #name, len) == 0) \
+ if (tunable_str_comma_strcmp_cte (&f, #name)) \
{ \
- if (disable) \
+ if (f.disable) \
cpu_features->preferred[index_arch_##name] &= ~bit_arch_##name; \
else \
cpu_features->preferred[index_arch_##name] |= bit_arch_##name; \
@@ -61,11 +61,11 @@
/* Enable/disable a preferred feature NAME. Enable a preferred feature
only if the feature NEED is usable. */
#define CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH(f, cpu_features, name, \
- need, disable, len) \
+ need, len) \
_Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
- if (memcmp (f, #name, len) == 0) \
+ if (tunable_str_comma_strcmp_cte (&f, #name)) \
{ \
- if (disable) \
+ if (f.disable) \
cpu_features->preferred[index_arch_##name] &= ~bit_arch_##name; \
else if (CPU_FEATURE_USABLE_P (cpu_features, need)) \
cpu_features->preferred[index_arch_##name] |= bit_arch_##name; \
@@ -93,38 +93,20 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
NOTE: the IFUNC selection may change over time. Please check all
multiarch implementations when experimenting. */
- const char *p = valp->strval, *c;
struct cpu_features *cpu_features = &GLRO(dl_x86_cpu_features);
- size_t len;
- do
- {
- const char *n;
- bool disable;
- size_t nl;
-
- for (c = p; *c != ','; c++)
- if (*c == '\0')
- break;
+ struct tunable_str_comma_state_t ts;
+ tunable_str_comma_init (&ts, valp);
- len = c - p;
- disable = *p == '-';
- if (disable)
- {
- n = p + 1;
- nl = len - 1;
- }
- else
- {
- n = p;
- nl = len;
- }
- switch (nl)
+ struct tunable_str_comma_t n;
+ while (tunable_str_comma_next (&ts, &n))
+ {
+ switch (n.len)
{
default:
break;
case 3:
- if (disable)
+ if (n.disable)
{
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX, 3);
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, CX8, 3);
@@ -135,7 +117,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
}
break;
case 4:
- if (disable)
+ if (n.disable)
{
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX2, 4);
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, BMI1, 4);
@@ -149,7 +131,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
}
break;
case 5:
- if (disable)
+ if (n.disable)
{
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, LZCNT, 5);
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, MOVBE, 5);
@@ -159,12 +141,12 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
}
break;
case 6:
- if (disable)
+ if (n.disable)
{
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, POPCNT, 6);
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, SSE4_1, 6);
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, SSE4_2, 6);
- if (memcmp (n, "XSAVEC", 6) == 0)
+ if (memcmp (n.str, "XSAVEC", 6) == 0)
{
/* Update xsave_state_size to XSAVE state size. */
cpu_features->xsave_state_size
@@ -174,14 +156,14 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
}
break;
case 7:
- if (disable)
+ if (n.disable)
{
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512F, 7);
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, OSXSAVE, 7);
}
break;
case 8:
- if (disable)
+ if (n.disable)
{
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512CD, 8);
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512BW, 8);
@@ -190,86 +172,72 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512PF, 8);
CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512VL, 8);
}
- CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features, Slow_BSF,
- disable, 8);
+ CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features, Slow_BSF, 8);
break;
case 11:
{
- CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
- Prefer_ERMS,
- disable, 11);
- CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
- Prefer_FSRM,
- disable, 11);
+ CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features, Prefer_ERMS,
+ 11);
+ CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features, Prefer_FSRM,
+ 11);
CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH (n, cpu_features,
Slow_SSE4_2,
SSE4_2,
- disable, 11);
+ 11);
}
break;
case 15:
{
CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
- Fast_Rep_String,
- disable, 15);
+ Fast_Rep_String, 15);
}
break;
case 16:
{
CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
- (n, cpu_features, Prefer_No_AVX512, AVX512F,
- disable, 16);
+ (n, cpu_features, Prefer_No_AVX512, AVX512F, 16);
}
break;
case 18:
{
CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
- Fast_Copy_Backward,
- disable, 18);
+ Fast_Copy_Backward, 18);
}
break;
case 19:
{
CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
- Fast_Unaligned_Load,
- disable, 19);
+ Fast_Unaligned_Load, 19);
CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
- Fast_Unaligned_Copy,
- disable, 19);
+ Fast_Unaligned_Copy, 19);
}
break;
case 20:
{
CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
- (n, cpu_features, Prefer_No_VZEROUPPER, AVX, disable,
- 20);
+ (n, cpu_features, Prefer_No_VZEROUPPER, AVX, 20);
}
break;
case 23:
{
CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
- (n, cpu_features, AVX_Fast_Unaligned_Load, AVX,
- disable, 23);
+ (n, cpu_features, AVX_Fast_Unaligned_Load, AVX, 23);
}
break;
case 24:
{
CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
- (n, cpu_features, MathVec_Prefer_No_AVX512, AVX512F,
- disable, 24);
+ (n, cpu_features, MathVec_Prefer_No_AVX512, AVX512F, 24);
}
break;
case 26:
{
CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
- (n, cpu_features, Prefer_PMINUB_for_stringop, SSE2,
- disable, 26);
+ (n, cpu_features, Prefer_PMINUB_for_stringop, SSE2, 26);
}
break;
}
- p += len + 1;
}
- while (*c != '\0');
}
#if CET_ENABLED
@@ -277,11 +245,11 @@ attribute_hidden
void
TUNABLE_CALLBACK (set_x86_ibt) (tunable_val_t *valp)
{
- if (memcmp (valp->strval, "on", sizeof ("on")) == 0)
+ if (tunable_strcmp_cte (valp, "on"))
GL(dl_x86_feature_control).ibt = cet_always_on;
- else if (memcmp (valp->strval, "off", sizeof ("off")) == 0)
+ else if (tunable_strcmp_cte (valp, "off"))
GL(dl_x86_feature_control).ibt = cet_always_off;
- else if (memcmp (valp->strval, "permissive", sizeof ("permissive")) == 0)
+ else if (tunable_strcmp_cte (valp, "permissive"))
GL(dl_x86_feature_control).ibt = cet_permissive;
}
@@ -289,11 +257,11 @@ attribute_hidden
void
TUNABLE_CALLBACK (set_x86_shstk) (tunable_val_t *valp)
{
- if (memcmp (valp->strval, "on", sizeof ("on")) == 0)
+ if (tunable_strcmp_cte (valp, "on"))
GL(dl_x86_feature_control).shstk = cet_always_on;
- else if (memcmp (valp->strval, "off", sizeof ("off")) == 0)
+ else if (tunable_strcmp_cte (valp, "off"))
GL(dl_x86_feature_control).shstk = cet_always_off;
- else if (memcmp (valp->strval, "permissive", sizeof ("permissive")) == 0)
+ else if (tunable_strcmp_cte (valp, "permissive"))
GL(dl_x86_feature_control).shstk = cet_permissive;
}
#endif
diff --git a/sysdeps/x86/tst-hwcap-tunables.c b/sysdeps/x86/tst-hwcap-tunables.c
new file mode 100644
index 0000000000..01a9377f7e
--- /dev/null
+++ b/sysdeps/x86/tst-hwcap-tunables.c
@@ -0,0 +1,148 @@
+/* Tests for x86 GLIBC_TUNABLES=glibc.cpu.hwcaps filter.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <array_length.h>
+#include <getopt.h>
+#include <ifunc-impl-list.h>
+#include <spawn.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <intprops.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/xunistd.h>
+#include <support/capture_subprocess.h>
+
+/* Nonzero if the program gets called via `exec'. */
+#define CMDLINE_OPTIONS \
+ { "restart", no_argument, &restart, 1 },
+static int restart;
+
+/* Disable everything. */
+static const char *test_1[] =
+{
+ "__memcpy_avx512_no_vzeroupper",
+ "__memcpy_avx512_unaligned",
+ "__memcpy_avx512_unaligned_erms",
+ "__memcpy_evex_unaligned",
+ "__memcpy_evex_unaligned_erms",
+ "__memcpy_avx_unaligned",
+ "__memcpy_avx_unaligned_erms",
+ "__memcpy_avx_unaligned_rtm",
+ "__memcpy_avx_unaligned_erms_rtm",
+ "__memcpy_ssse3",
+};
+
+static const struct test_t
+{
+ const char *env;
+ const char *const *funcs;
+ size_t nfuncs;
+} tests[] =
+{
+ {
+ /* Disable everything. */
+ "-Prefer_ERMS,-Prefer_FSRM,-AVX,-AVX2,-AVX_Usable,-AVX2_Usable,"
+ "-AVX512F_Usable,-SSE4_1,-SSE4_2,-SSSE3,-Fast_Unaligned_Load,-ERMS,"
+ "-AVX_Fast_Unaligned_Load",
+ test_1,
+ array_length (test_1)
+ },
+ {
+ /* Same as before, but with some empty suboptions. */
+ ",-,-Prefer_ERMS,-Prefer_FSRM,-AVX,-AVX2,-AVX_Usable,-AVX2_Usable,"
+ "-AVX512F_Usable,-SSE4_1,-SSE4_2,,-SSSE3,-Fast_Unaligned_Load,,-,-ERMS,"
+ "-AVX_Fast_Unaligned_Load,-,",
+ test_1,
+ array_length (test_1)
+ }
+};
+
+/* Called on process re-execution. */
+_Noreturn static void
+handle_restart (int ntest)
+{
+ struct libc_ifunc_impl impls[32];
+ int cnt = __libc_ifunc_impl_list ("memcpy", impls, array_length (impls));
+ if (cnt == 0)
+ _exit (EXIT_SUCCESS);
+ TEST_VERIFY_EXIT (cnt >= 1);
+ for (int i = 0; i < cnt; i++)
+ {
+ for (int f = 0; f < tests[ntest].nfuncs; f++)
+ {
+ if (strcmp (impls[i].name, tests[ntest].funcs[f]) == 0)
+ TEST_COMPARE (impls[i].usable, false);
+ }
+ }
+
+ _exit (EXIT_SUCCESS);
+}
+
+static int
+do_test (int argc, char *argv[])
+{
+ /* We must have either:
+ - One our fource parameters left if called initially:
+ + path to ld.so optional
+ + "--library-path" optional
+ + the library path optional
+ + the application name
+ + the test to check */
+
+ TEST_VERIFY_EXIT (argc == 2 || argc == 5);
+
+ if (restart)
+ handle_restart (atoi (argv[1]));
+
+ char nteststr[INT_BUFSIZE_BOUND (int)];
+
+ char *spargv[10];
+ {
+ int i = 0;
+ for (; i < argc - 1; i++)
+ spargv[i] = argv[i + 1];
+ spargv[i++] = (char *) "--direct";
+ spargv[i++] = (char *) "--restart";
+ spargv[i++] = nteststr;
+ spargv[i] = NULL;
+ }
+
+ for (int i = 0; i < array_length (tests); i++)
+ {
+ snprintf (nteststr, sizeof nteststr, "%d", i);
+
+ printf ("[%d] Spawned test for %s\n", i, tests[i].env);
+ char *tunable = xasprintf ("glibc.cpu.hwcaps=%s", tests[i].env);
+ setenv ("GLIBC_TUNABLES", tunable, 1);
+
+ struct support_capture_subprocess result
+ = support_capture_subprogram (spargv[0], spargv);
+ support_capture_subprocess_check (&result, "tst-tunables", 0,
+ sc_allow_stderr);
+ support_capture_subprocess_free (&result);
+
+ free (tunable);
+ }
+
+ return 0;
+}
+
+#define TEST_FUNCTION_ARGV do_test
+#include <support/test-driver.c>
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 2/5] elf: Do not set invalid tunables values
2023-11-22 20:43 [PATCH v5 0/5] Improve loader environment variable handling Adhemerval Zanella
2023-11-22 20:43 ` [PATCH v5 1/5] elf: Do not duplicate the GLIBC_TUNABLES string Adhemerval Zanella
@ 2023-11-22 20:43 ` Adhemerval Zanella
2023-12-01 15:32 ` Siddhesh Poyarekar
2023-11-22 20:43 ` [PATCH v5 3/5] elf: Ignore loader debug env vars for setuid Adhemerval Zanella
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Adhemerval Zanella @ 2023-11-22 20:43 UTC (permalink / raw)
To: libc-alpha, siddhesh
The loader now warns for invalid and out-of-range tunable values. The
patch also fixes the parsing of size_t maximum values, where
_dl_strtoul was failing for large values close to SIZE_MAX.
Checked on x86_64-linux-gnu.
---
elf/dl-misc.c | 5 ++++-
elf/dl-tunables.c | 35 ++++++++++++++++++++++++++++++-----
elf/tst-tunables.c | 30 ++++++++++++++++++++++++++++++
3 files changed, 64 insertions(+), 6 deletions(-)
diff --git a/elf/dl-misc.c b/elf/dl-misc.c
index 5b84adc2f4..037cbb3650 100644
--- a/elf/dl-misc.c
+++ b/elf/dl-misc.c
@@ -190,6 +190,9 @@ _dl_strtoul (const char *nptr, char **endptr)
}
}
+ const uint64_t cutoff = (UINT64_MAX * 2UL + 1UL) / 10;
+ const uint64_t cutlim = (UINT64_MAX * 2UL + 1UL) % 10;
+
while (1)
{
int digval;
@@ -207,7 +210,7 @@ _dl_strtoul (const char *nptr, char **endptr)
else
break;
- if (result >= (UINT64_MAX - digval) / base)
+ if (result > cutoff || (result == cutoff && digval > cutlim))
{
if (endptr != NULL)
*endptr = (char *) nptr;
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 26161c68e7..67a37ff704 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -77,16 +77,27 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
{
tunable_num_t val, min, max;
- if (cur->type.type_code == TUNABLE_TYPE_STRING)
+ switch (cur->type.type_code)
{
+ case TUNABLE_TYPE_STRING:
cur->val.strval = valp->strval;
cur->initialized = true;
return;
+ case TUNABLE_TYPE_INT_32:
+ val = (int32_t) valp->numval;
+ break;
+ case TUNABLE_TYPE_UINT_64:
+ val = (int64_t) valp->numval;
+ break;
+ case TUNABLE_TYPE_SIZE_T:
+ val = (size_t) valp->numval;
+ break;
+ default:
+ __builtin_unreachable ();
}
bool unsigned_cmp = unsigned_tunable_type (cur->type.type_code);
- val = valp->numval;
min = minp != NULL ? *minp : cur->type.min;
max = maxp != NULL ? *maxp : cur->type.max;
@@ -117,16 +128,24 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
/* Validate range of the input value and initialize the tunable CUR if it looks
good. */
-static void
+static bool
tunable_initialize (tunable_t *cur, const char *strval, size_t len)
{
tunable_val_t val = { 0 };
if (cur->type.type_code != TUNABLE_TYPE_STRING)
- val.numval = (tunable_num_t) _dl_strtoul (strval, NULL);
+ {
+ char *endptr = NULL;
+ uint64_t numval = _dl_strtoul (strval, &endptr);
+ if (endptr != strval + len)
+ return false;
+ val.numval = (tunable_num_t) numval;
+ }
else
val.strval = (struct tunable_str_t) { strval, len };
do_tunable_update_val (cur, &val, NULL, NULL);
+
+ return true;
}
void
@@ -226,7 +245,13 @@ parse_tunables (const char *valstring)
}
for (int i = 0; i < ntunables; i++)
- tunable_initialize (tunables[i].t, tunables[i].value, tunables[i].len);
+ 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 188345b070..d6a1e1b3ac 100644
--- a/elf/tst-tunables.c
+++ b/elf/tst-tunables.c
@@ -53,6 +53,13 @@ static const struct test_t
4096,
0,
},
+ {
+ "GLIBC_TUNABLES",
+ "glibc.malloc.mmap_threshold=-1",
+ 0,
+ SIZE_MAX,
+ 0,
+ },
/* Empty tunable are ignored. */
{
"GLIBC_TUNABLES",
@@ -224,6 +231,29 @@ static const struct test_t
0,
0,
},
+ /* Invalid numbers are ignored. */
+ {
+ "GLIBC_TUNABLES",
+ "glibc.malloc.check=abc:glibc.malloc.mmap_threshold=4096",
+ 0,
+ 4096,
+ 0,
+ },
+ {
+ "GLIBC_TUNABLES",
+ "glibc.malloc.check=2:glibc.malloc.mmap_threshold=abc",
+ 2,
+ 0,
+ 0,
+ },
+ {
+ "GLIBC_TUNABLES",
+ /* SIZE_MAX + 1 */
+ "glibc.malloc.mmap_threshold=18446744073709551616",
+ 0,
+ 0,
+ 0,
+ },
/* Also check some tunable aliases. */
{
"MALLOC_CHECK_",
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 3/5] elf: Ignore loader debug env vars for setuid
2023-11-22 20:43 [PATCH v5 0/5] Improve loader environment variable handling Adhemerval Zanella
2023-11-22 20:43 ` [PATCH v5 1/5] elf: Do not duplicate the GLIBC_TUNABLES string Adhemerval Zanella
2023-11-22 20:43 ` [PATCH v5 2/5] elf: Do not set invalid tunables values Adhemerval Zanella
@ 2023-11-22 20:43 ` Adhemerval Zanella
2023-12-01 15:34 ` Siddhesh Poyarekar
2023-11-22 20:43 ` [PATCH v5 4/5] elf: Ignore LD_BIND_NOW and LD_BIND_NOT for setuid binaries Adhemerval Zanella
2023-11-22 20:43 ` [PATCH v5 5/5] elf: Refactor process_envvars Adhemerval Zanella
4 siblings, 1 reply; 10+ messages in thread
From: Adhemerval Zanella @ 2023-11-22 20:43 UTC (permalink / raw)
To: libc-alpha, siddhesh
Loader already ignores LD_DEBUG, LD_DEBUG_OUTPUT, and
LD_TRACE_LOADED_OBJECTS. Both LD_WARN and LD_VERBOSE are similar to
LD_DEBUG, in the sense they enable additional checks and debug
information, so it makes sense to disable them.
Also add both LD_VERBOSE and LD_WARN on filtered environment variables
for setuid binaries.
Checked on x86_64-linux-gnu.
---
elf/rtld.c | 22 ++++++++++++++--------
elf/tst-env-setuid.c | 4 ++++
sysdeps/generic/unsecvars.h | 2 ++
3 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/elf/rtld.c b/elf/rtld.c
index 0553c05edb..d1017ba9e9 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -2550,13 +2550,15 @@ process_envvars (struct dl_main_state *state)
{
case 4:
/* Warning level, verbose or not. */
- if (memcmp (envline, "WARN", 4) == 0)
+ if (!__libc_enable_secure
+ && memcmp (envline, "WARN", 4) == 0)
GLRO(dl_verbose) = envline[5] != '\0';
break;
case 5:
/* Debugging of the dynamic linker? */
- if (memcmp (envline, "DEBUG", 5) == 0)
+ if (!__libc_enable_secure
+ && memcmp (envline, "DEBUG", 5) == 0)
{
process_dl_debug (state, &envline[6]);
break;
@@ -2571,7 +2573,8 @@ process_envvars (struct dl_main_state *state)
case 7:
/* Print information about versions. */
- if (memcmp (envline, "VERBOSE", 7) == 0)
+ if (!__libc_enable_secure
+ && memcmp (envline, "VERBOSE", 7) == 0)
{
state->version_info = envline[8] != '\0';
break;
@@ -2630,7 +2633,8 @@ process_envvars (struct dl_main_state *state)
}
/* Where to place the profiling data file. */
- if (memcmp (envline, "DEBUG_OUTPUT", 12) == 0)
+ if (!__libc_enable_secure
+ && memcmp (envline, "DEBUG_OUTPUT", 12) == 0)
{
debug_output = &envline[13];
break;
@@ -2651,7 +2655,8 @@ process_envvars (struct dl_main_state *state)
case 20:
/* The mode of the dynamic linker can be set. */
- if (memcmp (envline, "TRACE_LOADED_OBJECTS", 20) == 0)
+ if (!__libc_enable_secure
+ && memcmp (envline, "TRACE_LOADED_OBJECTS", 20) == 0)
{
state->mode = rtld_mode_trace;
state->mode_trace_program
@@ -2673,9 +2678,10 @@ process_envvars (struct dl_main_state *state)
}
while (*nextp != '\0');
- GLRO(dl_debug_mask) = 0;
-
- if (state->mode != rtld_mode_normal)
+ if (GLRO(dl_debug_mask) != 0
+ || GLRO(dl_verbose) != 0
+ || state->mode != rtld_mode_normal
+ || state->version_info)
_exit (5);
}
/* If we have to run the dynamic linker in debugging mode and the
diff --git a/elf/tst-env-setuid.c b/elf/tst-env-setuid.c
index 76b8e1fb45..b1d64ac085 100644
--- a/elf/tst-env-setuid.c
+++ b/elf/tst-env-setuid.c
@@ -59,6 +59,10 @@ static const struct envvar_t filtered_envvars[] =
{ "MALLOC_TRACE", FILTERED_VALUE },
{ "MALLOC_TRIM_THRESHOLD_", FILTERED_VALUE },
{ "RES_OPTIONS", FILTERED_VALUE },
+ { "LD_DEBUG", "all" },
+ { "LD_DEBUG_OUTPUT", "/tmp/some-file" },
+ { "LD_WARN", FILTERED_VALUE },
+ { "LD_VERBOSE", FILTERED_VALUE },
};
static const struct envvar_t unfiltered_envvars[] =
diff --git a/sysdeps/generic/unsecvars.h b/sysdeps/generic/unsecvars.h
index f7ebed60e5..8975df4a14 100644
--- a/sysdeps/generic/unsecvars.h
+++ b/sysdeps/generic/unsecvars.h
@@ -16,6 +16,8 @@
"LD_PRELOAD\0" \
"LD_PROFILE\0" \
"LD_SHOW_AUXV\0" \
+ "LD_VERBOSE\0" \
+ "LD_WARN\0" \
"LOCALDOMAIN\0" \
"LOCPATH\0" \
"MALLOC_ARENA_MAX\0" \
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 4/5] elf: Ignore LD_BIND_NOW and LD_BIND_NOT for setuid binaries
2023-11-22 20:43 [PATCH v5 0/5] Improve loader environment variable handling Adhemerval Zanella
` (2 preceding siblings ...)
2023-11-22 20:43 ` [PATCH v5 3/5] elf: Ignore loader debug env vars for setuid Adhemerval Zanella
@ 2023-11-22 20:43 ` Adhemerval Zanella
2023-11-22 20:43 ` [PATCH v5 5/5] elf: Refactor process_envvars Adhemerval Zanella
4 siblings, 0 replies; 10+ messages in thread
From: Adhemerval Zanella @ 2023-11-22 20:43 UTC (permalink / raw)
To: libc-alpha, siddhesh
To avoid any environment variable to change setuid binaries
semantics.
Checked on x86_64-linux-gnu.
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
elf/rtld.c | 8 ++++++--
elf/tst-env-setuid.c | 4 ++--
sysdeps/generic/unsecvars.h | 2 ++
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/elf/rtld.c b/elf/rtld.c
index d1017ba9e9..cfba30eba0 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -2598,12 +2598,14 @@ process_envvars (struct dl_main_state *state)
case 8:
/* Do we bind early? */
- if (memcmp (envline, "BIND_NOW", 8) == 0)
+ if (!__libc_enable_secure
+ && memcmp (envline, "BIND_NOW", 8) == 0)
{
GLRO(dl_lazy) = envline[9] == '\0';
break;
}
- if (memcmp (envline, "BIND_NOT", 8) == 0)
+ if (! __libc_enable_secure
+ && memcmp (envline, "BIND_NOT", 8) == 0)
GLRO(dl_bind_not) = envline[9] != '\0';
break;
@@ -2680,6 +2682,8 @@ process_envvars (struct dl_main_state *state)
if (GLRO(dl_debug_mask) != 0
|| GLRO(dl_verbose) != 0
+ || GLRO(dl_lazy) != 1
+ || GLRO(dl_bind_not) != 0
|| state->mode != rtld_mode_normal
|| state->version_info)
_exit (5);
diff --git a/elf/tst-env-setuid.c b/elf/tst-env-setuid.c
index b1d64ac085..9fa591a136 100644
--- a/elf/tst-env-setuid.c
+++ b/elf/tst-env-setuid.c
@@ -63,12 +63,12 @@ static const struct envvar_t filtered_envvars[] =
{ "LD_DEBUG_OUTPUT", "/tmp/some-file" },
{ "LD_WARN", FILTERED_VALUE },
{ "LD_VERBOSE", FILTERED_VALUE },
+ { "LD_BIND_NOW", "0" },
+ { "LD_BIND_NOT", "1" },
};
static const struct envvar_t unfiltered_envvars[] =
{
- { "LD_BIND_NOW", "0" },
- { "LD_BIND_NOT", "1" },
/* Non longer supported option. */
{ "LD_ASSUME_KERNEL", UNFILTERED_VALUE },
};
diff --git a/sysdeps/generic/unsecvars.h b/sysdeps/generic/unsecvars.h
index 8975df4a14..f1724efe0f 100644
--- a/sysdeps/generic/unsecvars.h
+++ b/sysdeps/generic/unsecvars.h
@@ -7,6 +7,8 @@
"GLIBC_TUNABLES\0" \
"HOSTALIASES\0" \
"LD_AUDIT\0" \
+ "LD_BIND_NOT\0" \
+ "LD_BIND_NOW\0" \
"LD_DEBUG\0" \
"LD_DEBUG_OUTPUT\0" \
"LD_DYNAMIC_WEAK\0" \
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 5/5] elf: Refactor process_envvars
2023-11-22 20:43 [PATCH v5 0/5] Improve loader environment variable handling Adhemerval Zanella
` (3 preceding siblings ...)
2023-11-22 20:43 ` [PATCH v5 4/5] elf: Ignore LD_BIND_NOW and LD_BIND_NOT for setuid binaries Adhemerval Zanella
@ 2023-11-22 20:43 ` Adhemerval Zanella
4 siblings, 0 replies; 10+ messages in thread
From: Adhemerval Zanella @ 2023-11-22 20:43 UTC (permalink / raw)
To: libc-alpha, siddhesh
It splits between process_envvars_secure and process_envvars_default,
with the former used to process arguments for __libc_enable_secure.
It does not have any semantic change, just simplify the code so there
is no need to handle __libc_enable_secure on each len switch.
Checked on x86_64-linux-gnu and aarch64-linux-gnu.
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
elf/rtld.c | 132 ++++++++++++++++++++++++++++++++++-------------------
1 file changed, 84 insertions(+), 48 deletions(-)
diff --git a/elf/rtld.c b/elf/rtld.c
index cfba30eba0..95dcd32185 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -2527,7 +2527,67 @@ a filename can be specified using the LD_DEBUG_OUTPUT environment variable.\n");
}
\f
static void
-process_envvars (struct dl_main_state *state)
+process_envvars_secure (struct dl_main_state *state)
+{
+ char **runp = _environ;
+ char *envline;
+
+ while ((envline = _dl_next_ld_env_entry (&runp)) != NULL)
+ {
+ size_t len = 0;
+
+ while (envline[len] != '\0' && envline[len] != '=')
+ ++len;
+
+ if (envline[len] != '=')
+ /* This is a "LD_" variable at the end of the string without
+ a '=' character. Ignore it since otherwise we will access
+ invalid memory below. */
+ continue;
+
+ switch (len)
+ {
+ case 5:
+ /* For __libc_enable_secure mode, audit pathnames containing slashes
+ are ignored. Also, shared audit objects are only loaded only from
+ the standard search directories and only if they have set-user-ID
+ mode bit enabled. */
+ if (memcmp (envline, "AUDIT", 5) == 0)
+ audit_list_add_string (&state->audit_list, &envline[6]);
+ break;
+
+ case 7:
+ /* For __libc_enable_secure mode, preload pathnames containing slashes
+ are ignored. Also, shared objects are only preloaded from the
+ standard search directories and only if they have set-user-ID mode
+ bit enabled. */
+ if (memcmp (envline, "PRELOAD", 7) == 0)
+ state->preloadlist = &envline[8];
+ break;
+ }
+ }
+
+ /* Extra security for SUID binaries. Remove all dangerous environment
+ variables. */
+ const char *nextp = UNSECURE_ENVVARS;
+ do
+ {
+ unsetenv (nextp);
+ nextp = strchr (nextp, '\0') + 1;
+ }
+ while (*nextp != '\0');
+
+ if (GLRO(dl_debug_mask) != 0
+ || GLRO(dl_verbose) != 0
+ || GLRO(dl_lazy) != 1
+ || GLRO(dl_bind_not) != 0
+ || state->mode != rtld_mode_normal
+ || state->version_info)
+ _exit (5);
+}
+
+static void
+process_envvars_default (struct dl_main_state *state)
{
char **runp = _environ;
char *envline;
@@ -2550,15 +2610,13 @@ process_envvars (struct dl_main_state *state)
{
case 4:
/* Warning level, verbose or not. */
- if (!__libc_enable_secure
- && memcmp (envline, "WARN", 4) == 0)
+ if (memcmp (envline, "WARN", 4) == 0)
GLRO(dl_verbose) = envline[5] != '\0';
break;
case 5:
/* Debugging of the dynamic linker? */
- if (!__libc_enable_secure
- && memcmp (envline, "DEBUG", 5) == 0)
+ if (memcmp (envline, "DEBUG", 5) == 0)
{
process_dl_debug (state, &envline[6]);
break;
@@ -2573,8 +2631,7 @@ process_envvars (struct dl_main_state *state)
case 7:
/* Print information about versions. */
- if (!__libc_enable_secure
- && memcmp (envline, "VERBOSE", 7) == 0)
+ if (memcmp (envline, "VERBOSE", 7) == 0)
{
state->version_info = envline[8] != '\0';
break;
@@ -2591,43 +2648,37 @@ process_envvars (struct dl_main_state *state)
}
/* Which shared object shall be profiled. */
- if (!__libc_enable_secure
- && memcmp (envline, "PROFILE", 7) == 0 && envline[8] != '\0')
+ if (memcmp (envline, "PROFILE", 7) == 0 && envline[8] != '\0')
GLRO(dl_profile) = &envline[8];
break;
case 8:
/* Do we bind early? */
- if (!__libc_enable_secure
- && memcmp (envline, "BIND_NOW", 8) == 0)
+ if (memcmp (envline, "BIND_NOW", 8) == 0)
{
GLRO(dl_lazy) = envline[9] == '\0';
break;
}
- if (! __libc_enable_secure
- && memcmp (envline, "BIND_NOT", 8) == 0)
+ if (memcmp (envline, "BIND_NOT", 8) == 0)
GLRO(dl_bind_not) = envline[9] != '\0';
break;
case 9:
/* Test whether we want to see the content of the auxiliary
array passed up from the kernel. */
- if (!__libc_enable_secure
- && memcmp (envline, "SHOW_AUXV", 9) == 0)
+ if (memcmp (envline, "SHOW_AUXV", 9) == 0)
_dl_show_auxv ();
break;
case 11:
/* Path where the binary is found. */
- if (!__libc_enable_secure
- && memcmp (envline, "ORIGIN_PATH", 11) == 0)
+ if (memcmp (envline, "ORIGIN_PATH", 11) == 0)
GLRO(dl_origin_path) = &envline[12];
break;
case 12:
/* The library search path. */
- if (!__libc_enable_secure
- && memcmp (envline, "LIBRARY_PATH", 12) == 0)
+ if (memcmp (envline, "LIBRARY_PATH", 12) == 0)
{
state->library_path = &envline[13];
state->library_path_source = "LD_LIBRARY_PATH";
@@ -2635,30 +2686,26 @@ process_envvars (struct dl_main_state *state)
}
/* Where to place the profiling data file. */
- if (!__libc_enable_secure
- && memcmp (envline, "DEBUG_OUTPUT", 12) == 0)
+ if (memcmp (envline, "DEBUG_OUTPUT", 12) == 0)
{
debug_output = &envline[13];
break;
}
- if (!__libc_enable_secure
- && memcmp (envline, "DYNAMIC_WEAK", 12) == 0)
+ if (memcmp (envline, "DYNAMIC_WEAK", 12) == 0)
GLRO(dl_dynamic_weak) = 1;
break;
case 14:
/* Where to place the profiling data file. */
- if (!__libc_enable_secure
- && memcmp (envline, "PROFILE_OUTPUT", 14) == 0
+ if (memcmp (envline, "PROFILE_OUTPUT", 14) == 0
&& envline[15] != '\0')
GLRO(dl_profile_output) = &envline[15];
break;
case 20:
/* The mode of the dynamic linker can be set. */
- if (!__libc_enable_secure
- && memcmp (envline, "TRACE_LOADED_OBJECTS", 20) == 0)
+ if (memcmp (envline, "TRACE_LOADED_OBJECTS", 20) == 0)
{
state->mode = rtld_mode_trace;
state->mode_trace_program
@@ -2668,30 +2715,10 @@ process_envvars (struct dl_main_state *state)
}
}
- /* Extra security for SUID binaries. Remove all dangerous environment
- variables. */
- if (__glibc_unlikely (__libc_enable_secure))
- {
- const char *nextp = UNSECURE_ENVVARS;
- do
- {
- unsetenv (nextp);
- nextp = strchr (nextp, '\0') + 1;
- }
- while (*nextp != '\0');
-
- if (GLRO(dl_debug_mask) != 0
- || GLRO(dl_verbose) != 0
- || GLRO(dl_lazy) != 1
- || GLRO(dl_bind_not) != 0
- || state->mode != rtld_mode_normal
- || state->version_info)
- _exit (5);
- }
/* If we have to run the dynamic linker in debugging mode and the
LD_DEBUG_OUTPUT environment variable is given, we write the debug
messages to this file. */
- else if (GLRO(dl_debug_mask) != 0 && debug_output != NULL)
+ if (GLRO(dl_debug_mask) != 0 && debug_output != NULL)
{
const int flags = O_WRONLY | O_APPEND | O_CREAT | O_NOFOLLOW;
size_t name_len = strlen (debug_output);
@@ -2710,6 +2737,15 @@ process_envvars (struct dl_main_state *state)
}
}
+static void
+process_envvars (struct dl_main_state *state)
+{
+ if (__glibc_unlikely (__libc_enable_secure))
+ process_envvars_secure (state);
+ else
+ process_envvars_default (state);
+}
+
#if HP_TIMING_INLINE
static void
print_statistics_item (const char *title, hp_timing_t time,
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 1/5] elf: Do not duplicate the GLIBC_TUNABLES string
2023-11-22 20:43 ` [PATCH v5 1/5] elf: Do not duplicate the GLIBC_TUNABLES string Adhemerval Zanella
@ 2023-12-01 15:20 ` Siddhesh Poyarekar
0 siblings, 0 replies; 10+ messages in thread
From: Siddhesh Poyarekar @ 2023-12-01 15:20 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
On 2023-11-22 15:43, Adhemerval Zanella wrote:
> The tunable parsing duplicates the tunable environment variable so it
> null-terminates each one since it simplifies the later parsing. It has
> the drawback of adding another point of failure (__minimal_malloc
> failing), and the memory copy requires tuning the compiler to avoid mem
> operations calls.
>
> The parsing now tracks the tunable start and its size. The
> dl-tunable-parse.h adds helper functions to help parsing, like a strcmp
> that also checks for size and an iterator for suboptions that are
> comma-separated (used on hwcap parsing by x86, powerpc, and s390x).
>
> Since the environment variable is allocated on the stack by the kernel,
> it is safe to keep the references to the suboptions for later parsing
> of string tunables (as done by set_hwcaps by multiple architectures).
>
> Checked on x86_64-linux-gnu, powerpc64le-linux-gnu, and
> aarch64-linux-gnu.
> ---
> elf/dl-tunables.c | 90 +++++-----
> elf/dl-tunables.h | 6 +-
> elf/tst-tunables.c | 66 ++++++-
> sysdeps/generic/dl-tunables-parse.h | 134 ++++++++++++++
> sysdeps/s390/cpu-features.c | 165 +++++++-----------
> .../unix/sysv/linux/aarch64/cpu-features.c | 33 ++--
> .../unix/sysv/linux/powerpc/cpu-features.c | 45 ++---
> .../sysv/linux/powerpc/tst-hwcap-tunables.c | 6 +-
> sysdeps/x86/Makefile | 4 +-
> sysdeps/x86/cpu-tunables.c | 118 +++++--------
> sysdeps/x86/tst-hwcap-tunables.c | 148 ++++++++++++++++
> 11 files changed, 532 insertions(+), 283 deletions(-)
> create mode 100644 sysdeps/generic/dl-tunables-parse.h
> create mode 100644 sysdeps/x86/tst-hwcap-tunables.c
>
> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
> index 83265bc00b..26161c68e7 100644
> --- a/elf/dl-tunables.c
> +++ b/elf/dl-tunables.c
> @@ -36,48 +36,32 @@
> #define TUNABLES_INTERNAL 1
> #include "dl-tunables.h"
>
> -#include <not-errno.h>
> -
> -static char *
> -tunables_strdup (const char *in)
> -{
> - size_t i = 0;
> -
> - while (in[i++] != '\0');
> - char *out = __minimal_malloc (i + 1);
> -
> - /* For most of the tunables code, we ignore user errors. However,
> - this is a system error - and running out of memory at program
> - startup should be reported, so we do. */
> - if (out == NULL)
> - _dl_fatal_printf ("failed to allocate memory to process tunables\n");
> -
> - while (i-- > 0)
> - out[i] = in[i];
> -
> - return out;
> -}
> -
> static char **
> -get_next_env (char **envp, char **name, size_t *namelen, char **val,
> +get_next_env (char **envp, char **name, char **val, size_t *vallen,
> char ***prev_envp)
> {
> while (envp != NULL && *envp != NULL)
> {
> char **prev = envp;
> char *envline = *envp++;
> - int len = 0;
> + char *penv = envline;
> + size_t len;
>
> - while (envline[len] != '\0' && envline[len] != '=')
> - len++;
> + for (; *penv != '\0' && *penv != '='; penv++);
>
> /* Just the name and no value, go to the next one. */
> - if (envline[len] == '\0')
> + if (*penv == '\0')
> continue;
>
> *name = envline;
> - *namelen = len;
> - *val = &envline[len + 1];
> + /* Skip '='. */
> + *val = ++penv;
> +
> + len = 0;
> + while (*penv++ != '\0')
> + len++;
> + *vallen = len;
Isn't this just strlen(val) ? You could delay that until it's actually
needed in __tunables_init. Then this part becomes an obvious cleanup to
drop the unused namelen.
> +
> *prev_envp = prev;
>
> return envp;
> @@ -134,14 +118,14 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
> /* Validate range of the input value and initialize the tunable CUR if it looks
> good. */
> static void
> -tunable_initialize (tunable_t *cur, const char *strval)
> +tunable_initialize (tunable_t *cur, const char *strval, size_t len)
> {
> - tunable_val_t val;
> + tunable_val_t val = { 0 };
>
> if (cur->type.type_code != TUNABLE_TYPE_STRING)
> val.numval = (tunable_num_t) _dl_strtoul (strval, NULL);
> else
> - val.strval = strval;
> + val.strval = (struct tunable_str_t) { strval, len };
> do_tunable_update_val (cur, &val, NULL, NULL);
> }
>
> @@ -158,29 +142,29 @@ struct tunable_toset_t
> {
> tunable_t *t;
> const char *value;
> + size_t len;
> };
>
> enum { tunables_list_size = array_length (tunable_list) };
>
> /* Parse the tunable string VALSTRING and set TUNABLES with the found tunables
> - and their respective strings. VALSTRING is a duplicated values, where
> - delimiters ':' are replaced with '\0', so string tunables are null
> - terminated.
> + and their respectibles values. The VALSTRING is parsed in place, with the
s/respectibles/respective/
> + tunable start and size recorded in TUNABLES.
> Return the number of tunables found (including 0 if the string is empty)
> or -1 if for an ill-formatted definition. */
> static int
> -parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
> +parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables)
> {
> if (valstring == NULL || *valstring == '\0')
> return 0;
>
> - char *p = valstring;
> + const char *p = valstring;
> bool done = false;
> int ntunables = 0;
>
> while (!done)
> {
> - char *name = p;
> + const char *name = p;
>
> /* First, find where the name ends. */
> while (*p != '=' && *p != ':' && *p != '\0')
> @@ -202,7 +186,7 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
> /* Skip the '='. */
> p++;
>
> - char *value = p;
> + const char *value = p;
>
> while (*p != '=' && *p != ':' && *p != '\0')
> p++;
> @@ -211,8 +195,6 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
> return -1;
> else if (*p == '\0')
> done = true;
> - else
> - *p++ = '\0';
>
> /* Add the tunable if it exists. */
> for (size_t i = 0; i < tunables_list_size; i++)
> @@ -221,7 +203,8 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
>
> if (tunable_is_name (cur->name, name))
> {
> - tunables[ntunables++] = (struct tunable_toset_t) { cur, value };
> + tunables[ntunables++] =
> + (struct tunable_toset_t) { cur, value, p - value };
> break;
> }
> }
> @@ -231,7 +214,7 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
> }
>
> static void
> -parse_tunables (char *valstring)
> +parse_tunables (const char *valstring)
> {
> struct tunable_toset_t tunables[tunables_list_size];
> int ntunables = parse_tunables_string (valstring, tunables);
> @@ -243,7 +226,7 @@ parse_tunables (char *valstring)
> }
>
> for (int i = 0; i < ntunables; i++)
> - tunable_initialize (tunables[i].t, tunables[i].value);
> + tunable_initialize (tunables[i].t, tunables[i].value, tunables[i].len);
> }
>
> /* Initialize the tunables list from the environment. For now we only use the
> @@ -254,19 +237,22 @@ __tunables_init (char **envp)
> {
> char *envname = NULL;
> char *envval = NULL;
> - size_t len = 0;
> + size_t envvallen = 0;
> char **prev_envp = envp;
>
> /* Ignore tunables for AT_SECURE programs. */
> if (__libc_enable_secure)
> return;
>
> - while ((envp = get_next_env (envp, &envname, &len, &envval,
> + while ((envp = get_next_env (envp, &envname, &envval, &envvallen,
> &prev_envp)) != NULL)
> {
> + /* The environment variable is allocated on the stack by the kernel, so
> + it is safe to keep the references to the suboptions for later parsing
> + of string tunables. */
> if (tunable_is_name ("GLIBC_TUNABLES", envname))
> {
> - parse_tunables (tunables_strdup (envval));
> + parse_tunables (envval);
> continue;
> }
>
> @@ -284,7 +270,7 @@ __tunables_init (char **envp)
> /* We have a match. Initialize and move on to the next line. */
> if (tunable_is_name (name, envname))
> {
> - tunable_initialize (cur, envval);
> + tunable_initialize (cur, envval, envvallen);
Couldn't this just be:
size_t len = 0;
const char *p = envval;
/* Envvar lines are always NULL terminated. */
for (const char *p = envval; *p != '\0'; p++, len++);
tunable_initialize (cur, envval, len);
That eliminates length computation (and passing) in get_next_env for
every envvar, to only the ones that matter.
> break;
> }
> }
> @@ -298,7 +284,7 @@ __tunables_print (void)
> {
> const tunable_t *cur = &tunable_list[i];
> if (cur->type.type_code == TUNABLE_TYPE_STRING
> - && cur->val.strval == NULL)
> + && cur->val.strval.str == NULL)
> _dl_printf ("%s:\n", cur->name);
> else
> {
> @@ -324,7 +310,9 @@ __tunables_print (void)
> (size_t) cur->type.max);
> break;
> case TUNABLE_TYPE_STRING:
> - _dl_printf ("%s\n", cur->val.strval);
> + _dl_printf ("%.*s\n",
> + (int) cur->val.strval.len,
> + cur->val.strval.str);
> break;
> default:
> __builtin_unreachable ();
> @@ -359,7 +347,7 @@ __tunable_get_val (tunable_id_t id, void *valp, tunable_callback_t callback)
> }
> case TUNABLE_TYPE_STRING:
> {
> - *((const char **)valp) = cur->val.strval;
> + *((struct tunable_str_t **) valp) = &cur->val.strval;
> break;
> }
> default:
> diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
> index 45c191e021..0e777d7d37 100644
> --- a/elf/dl-tunables.h
> +++ b/elf/dl-tunables.h
> @@ -30,7 +30,11 @@ typedef intmax_t tunable_num_t;
> typedef union
> {
> tunable_num_t numval;
> - const char *strval;
> + struct tunable_str_t
> + {
> + const char *str;
> + size_t len;
> + } strval;
> } tunable_val_t;
>
> typedef void (*tunable_callback_t) (tunable_val_t *);
> diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
> index e1ad44f27c..188345b070 100644
> --- a/elf/tst-tunables.c
> +++ b/elf/tst-tunables.c
> @@ -31,7 +31,8 @@ static int restart;
>
> static const struct test_t
> {
> - const char *env;
> + const char *name;
> + const char *value;
> int32_t expected_malloc_check;
> size_t expected_mmap_threshold;
> int32_t expected_perturb;
> @@ -39,12 +40,14 @@ static const struct test_t
> {
> /* Expected tunable format. */
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.check=2",
> 2,
> 0,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
> 2,
> 4096,
> @@ -52,6 +55,7 @@ static const struct test_t
> },
> /* Empty tunable are ignored. */
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.check=2::glibc.malloc.mmap_threshold=4096",
> 2,
> 4096,
> @@ -59,6 +63,7 @@ static const struct test_t
> },
> /* As well empty values. */
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.check=:glibc.malloc.mmap_threshold=4096",
> 0,
> 4096,
> @@ -66,18 +71,21 @@ static const struct test_t
> },
> /* Tunable are processed from left to right, so last one is the one set. */
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.check=1:glibc.malloc.check=2",
> 2,
> 0,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.check=1:glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
> 2,
> 4096,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096:glibc.malloc.check=1",
> 1,
> 4096,
> @@ -85,12 +93,14 @@ static const struct test_t
> },
> /* 0x800 is larger than tunable maxval (0xff), so the tunable is unchanged. */
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.perturb=0x800",
> 0,
> 0,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.perturb=0x55",
> 0,
> 0,
> @@ -98,6 +108,7 @@ static const struct test_t
> },
> /* Out of range values are just ignored. */
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
> 0,
> 4096,
> @@ -105,24 +116,28 @@ static const struct test_t
> },
> /* Invalid keys are ignored. */
> {
> + "GLIBC_TUNABLES",
> ":glibc.malloc.garbage=2:glibc.malloc.check=1",
> 1,
> 0,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.perturb=0x800:not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",
> 0,
> 4096,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "glibc.not_valid.check=2:glibc.malloc.mmap_threshold=4096",
> 0,
> 4096,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",
> 0,
> 4096,
> @@ -130,24 +145,28 @@ static const struct test_t
> },
> /* Invalid subkeys are ignored. */
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096:glibc.malloc.check=2",
> 2,
> 0,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.check=4:glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096",
> 0,
> 0,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "not_valid.malloc.check=2",
> 0,
> 0,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "glibc.not_valid.check=2",
> 0,
> 0,
> @@ -156,6 +175,7 @@ static const struct test_t
> /* An ill-formatted tunable in the for key=key=value will considere the
> value as 'key=value' (which can not be parsed as an integer). */
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096",
> 0,
> 0,
> @@ -163,41 +183,77 @@ static const struct test_t
> },
> /* Ill-formatted tunables string is not parsed. */
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096:glibc.malloc.check=2",
> 0,
> 0,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.check=2=2",
> 0,
> 0,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.check=2=2:glibc.malloc.mmap_threshold=4096",
> 0,
> 0,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.check=2=2:glibc.malloc.check=2",
> 0,
> 0,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096=4096",
> 0,
> 0,
> 0,
> },
> {
> + "GLIBC_TUNABLES",
> "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096=4096",
> 0,
> 0,
> 0,
> },
> + /* Also check some tunable aliases. */
> + {
> + "MALLOC_CHECK_",
> + "2",
> + 2,
> + 0,
> + 0,
> + },
> + {
> + "MALLOC_MMAP_THRESHOLD_",
> + "4096",
> + 0,
> + 4096,
> + 0,
> + },
> + {
> + "MALLOC_PERTURB_",
> + "0x55",
> + 0,
> + 0,
> + 0x55,
> + },
> + /* 0x800 is larger than tunable maxval (0xff), so the tunable is unchanged. */
> + {
> + "MALLOC_PERTURB_",
> + "0x800",
> + 0,
> + 0,
> + 0,
> + },
> };
>
> static int
> @@ -245,13 +301,17 @@ do_test (int argc, char *argv[])
> {
> snprintf (nteststr, sizeof nteststr, "%d", i);
>
> - printf ("[%d] Spawned test for %s\n", i, tests[i].env);
> - setenv ("GLIBC_TUNABLES", tests[i].env, 1);
> + printf ("[%d] Spawned test for %s=%s\n",
> + i,
> + tests[i].name,
> + tests[i].value);
> + setenv (tests[i].name, tests[i].value, 1);
> struct support_capture_subprocess result
> = support_capture_subprogram (spargv[0], spargv);
> support_capture_subprocess_check (&result, "tst-tunables", 0,
> sc_allow_stderr);
> support_capture_subprocess_free (&result);
> + unsetenv (tests[i].name);
> }
>
> return 0;
> diff --git a/sysdeps/generic/dl-tunables-parse.h b/sysdeps/generic/dl-tunables-parse.h
> new file mode 100644
> index 0000000000..b37be0443b
> --- /dev/null
> +++ b/sysdeps/generic/dl-tunables-parse.h
> @@ -0,0 +1,134 @@
> +/* Helper functions to handle tunable strings.
> + Copyright (C) 2023 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#ifndef _DL_TUNABLES_PARSE_H
> +#define _DL_TUNABLES_PARSE_H 1
> +
> +#include <assert.h>
> +#include <string.h>
> +
> +/* Compare the contents of STRVAL with STR of size LEN. The STR might not
> + be null-terminated. */
> +static __always_inline bool
> +tunable_strcmp (const struct tunable_str_t *strval, const char *str,
> + size_t len)
> +{
> + return strval->len == len && memcmp (strval->str, str, len) == 0;
> +}
> +#define tunable_strcmp_cte(__tunable, __str) \
> + tunable_strcmp (&__tunable->strval, __str, sizeof (__str) - 1)
> +
> +/*
> + Helper functions to iterate over a tunable string composed by multiple
> + suboptions separated by commaxi; this is a common pattern for CPU. Each
> + suboptions is return in the form of { address, size } (no null terminated).
> + For instance:
> +
> + struct tunable_str_comma_t ts;
> + tunable_str_comma_init (&ts, valp);
> +
> + struct tunable_str_t t;
> + while (tunable_str_comma_next (&ts, &t))
> + {
> + _dl_printf ("[%s] %.*s (%d)\n",
> + __func__,
> + (int) tstr.len,
> + tstr.str,
> + (int) tstr.len);
> +
> + if (tunable_str_comma_strcmp (&t, opt, opt1_len))
> + {
> + [...]
> + }
> + else if (tunable_str_comma_strcmp_cte (&t, "opt2"))
> + {
> + [...]
> + }
> + }
> +
> + NB: These function are expected to be called from tunable callback
> + functions along with tunable_val_t with string types.
> +*/
> +
> +struct tunable_str_comma_state_t
> +{
> + const char *p;
> + size_t plen;
> + size_t maxplen;
> +};
> +
> +struct tunable_str_comma_t
> +{
> + const char *str;
> + size_t len;
> + bool disable;
> +};
> +
> +static inline void
> +tunable_str_comma_init (struct tunable_str_comma_state_t *state,
> + tunable_val_t *valp)
> +{
> + assert (valp->strval.str != NULL);
> + state->p = valp->strval.str;
> + state->plen = 0;
> + state->maxplen = valp->strval.len;
> +}
> +
> +static inline bool
> +tunable_str_comma_next (struct tunable_str_comma_state_t *state,
> + struct tunable_str_comma_t *str)
> +{
> + if (*state->p == '\0' || state->plen >= state->maxplen)
> + return false;
> +
> + const char *c;
> + for (c = state->p; *c != ','; c++, state->plen++)
> + if (*c == '\0' || state->plen == state->maxplen)
> + break;
> +
> + str->str = state->p;
> + str->len = c - state->p;
> +
> + if (str->len > 0)
> + {
> + str->disable = *str->str == '-';
> + if (str->disable)
> + {
> + str->str = str->str + 1;
> + str->len = str->len - 1;
> + }
> + }
> +
> + state->p = c + 1;
> + state->plen++;
> +
> + return true;
> +}
> +
> +/* Compare the contents of T with STR of size LEN. The STR might not be
> + null-terminated. */
> +static __always_inline bool
> +tunable_str_comma_strcmp (const struct tunable_str_comma_t *t, const char *str,
> + size_t len)
> +{
> + return t->len == len && memcmp (t->str, str, len) == 0;
> +}
> +#define tunable_str_comma_strcmp_cte(__t, __str) \
> + tunable_str_comma_strcmp (__t, __str, sizeof (__str) - 1)
> +
> +#endif
> diff --git a/sysdeps/s390/cpu-features.c b/sysdeps/s390/cpu-features.c
> index 55449ba07f..06c1cab0fd 100644
> --- a/sysdeps/s390/cpu-features.c
> +++ b/sysdeps/s390/cpu-features.c
> @@ -22,6 +22,7 @@
> #include <ifunc-memcmp.h>
> #include <string.h>
> #include <dl-symbol-redir-ifunc.h>
> +#include <dl-tunables-parse.h>
>
> #define S390_COPY_CPU_FEATURES(SRC_PTR, DEST_PTR) \
> (DEST_PTR)->hwcap = (SRC_PTR)->hwcap; \
> @@ -51,33 +52,14 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> struct cpu_features cpu_features_curr;
> S390_COPY_CPU_FEATURES (cpu_features, &cpu_features_curr);
>
> - const char *token = valp->strval;
> - do
> + struct tunable_str_comma_state_t ts;
> + tunable_str_comma_init (&ts, valp);
> +
> + struct tunable_str_comma_t t;
> + while (tunable_str_comma_next (&ts, &t))
> {
> - const char *token_end, *feature;
> - bool disable;
> - size_t token_len;
> - size_t feature_len;
> -
> - /* Find token separator or end of string. */
> - for (token_end = token; *token_end != ','; token_end++)
> - if (*token_end == '\0')
> - break;
> -
> - /* Determine feature. */
> - token_len = token_end - token;
> - if (*token == '-')
> - {
> - disable = true;
> - feature = token + 1;
> - feature_len = token_len - 1;
> - }
> - else
> - {
> - disable = false;
> - feature = token;
> - feature_len = token_len;
> - }
> + if (t.len == 0)
> + continue;
>
> /* Handle only the features here which are really used in the
> IFUNC-resolvers. All others are ignored as the values are only used
> @@ -85,86 +67,64 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> bool reset_features = false;
> unsigned long int hwcap_mask = 0UL;
> unsigned long long stfle_bits0_mask = 0ULL;
> + bool disable = t.disable;
>
> - if ((*feature == 'z' || *feature == 'a'))
> + if (tunable_str_comma_strcmp_cte (&t, "zEC12")
> + || tunable_str_comma_strcmp_cte (&t, "arch10"))
> + {
> + reset_features = true;
> + disable = true;
> + hwcap_mask = HWCAP_S390_VXRS | HWCAP_S390_VXRS_EXT
> + | HWCAP_S390_VXRS_EXT2;
> + stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> + }
> + else if (tunable_str_comma_strcmp_cte (&t, "z13")
> + || tunable_str_comma_strcmp_cte (&t, "arch11"))
> + {
> + reset_features = true;
> + disable = true;
> + hwcap_mask = HWCAP_S390_VXRS_EXT | HWCAP_S390_VXRS_EXT2;
> + stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> + }
> + else if (tunable_str_comma_strcmp_cte (&t, "z14")
> + || tunable_str_comma_strcmp_cte (&t, "arch12"))
> + {
> + reset_features = true;
> + disable = true;
> + hwcap_mask = HWCAP_S390_VXRS_EXT2;
> + stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> + }
> + else if (tunable_str_comma_strcmp_cte (&t, "z15")
> + || tunable_str_comma_strcmp_cte (&t, "z16")
> + || tunable_str_comma_strcmp_cte (&t, "arch13")
> + || tunable_str_comma_strcmp_cte (&t, "arch14"))
> {
> - if ((feature_len == 5 && *feature == 'z'
> - && memcmp (feature, "zEC12", 5) == 0)
> - || (feature_len == 6 && *feature == 'a'
> - && memcmp (feature, "arch10", 6) == 0))
> - {
> - reset_features = true;
> - disable = true;
> - hwcap_mask = HWCAP_S390_VXRS | HWCAP_S390_VXRS_EXT
> - | HWCAP_S390_VXRS_EXT2;
> - stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> - }
> - else if ((feature_len == 3 && *feature == 'z'
> - && memcmp (feature, "z13", 3) == 0)
> - || (feature_len == 6 && *feature == 'a'
> - && memcmp (feature, "arch11", 6) == 0))
> - {
> - reset_features = true;
> - disable = true;
> - hwcap_mask = HWCAP_S390_VXRS_EXT | HWCAP_S390_VXRS_EXT2;
> - stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> - }
> - else if ((feature_len == 3 && *feature == 'z'
> - && memcmp (feature, "z14", 3) == 0)
> - || (feature_len == 6 && *feature == 'a'
> - && memcmp (feature, "arch12", 6) == 0))
> - {
> - reset_features = true;
> - disable = true;
> - hwcap_mask = HWCAP_S390_VXRS_EXT2;
> - stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> - }
> - else if ((feature_len == 3 && *feature == 'z'
> - && (memcmp (feature, "z15", 3) == 0
> - || memcmp (feature, "z16", 3) == 0))
> - || (feature_len == 6
> - && (memcmp (feature, "arch13", 6) == 0
> - || memcmp (feature, "arch14", 6) == 0)))
> - {
> - /* For z15 or newer we don't have to disable something,
> - but we have to reset to the original values. */
> - reset_features = true;
> - }
> + /* For z15 or newer we don't have to disable something, but we have
> + to reset to the original values. */
> + reset_features = true;
> }
> - else if (*feature == 'H')
> + else if (tunable_str_comma_strcmp_cte (&t, "HWCAP_S390_VXRS"))
> {
> - if (feature_len == 15
> - && memcmp (feature, "HWCAP_S390_VXRS", 15) == 0)
> - {
> - hwcap_mask = HWCAP_S390_VXRS;
> - if (disable)
> - hwcap_mask |= HWCAP_S390_VXRS_EXT | HWCAP_S390_VXRS_EXT2;
> - }
> - else if (feature_len == 19
> - && memcmp (feature, "HWCAP_S390_VXRS_EXT", 19) == 0)
> - {
> - hwcap_mask = HWCAP_S390_VXRS_EXT;
> - if (disable)
> - hwcap_mask |= HWCAP_S390_VXRS_EXT2;
> - else
> - hwcap_mask |= HWCAP_S390_VXRS;
> - }
> - else if (feature_len == 20
> - && memcmp (feature, "HWCAP_S390_VXRS_EXT2", 20) == 0)
> - {
> - hwcap_mask = HWCAP_S390_VXRS_EXT2;
> - if (!disable)
> - hwcap_mask |= HWCAP_S390_VXRS | HWCAP_S390_VXRS_EXT;
> - }
> + hwcap_mask = HWCAP_S390_VXRS;
> + if (t.disable)
> + hwcap_mask |= HWCAP_S390_VXRS_EXT | HWCAP_S390_VXRS_EXT2;
> }
> - else if (*feature == 'S')
> + else if (tunable_str_comma_strcmp_cte (&t, "HWCAP_S390_VXRS_EXT"))
> {
> - if (feature_len == 10
> - && memcmp (feature, "STFLE_MIE3", 10) == 0)
> - {
> - stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> - }
> + hwcap_mask = HWCAP_S390_VXRS_EXT;
> + if (t.disable)
> + hwcap_mask |= HWCAP_S390_VXRS_EXT2;
> + else
> + hwcap_mask |= HWCAP_S390_VXRS;
> + }
> + else if (tunable_str_comma_strcmp_cte (&t, "HWCAP_S390_VXRS_EXT2"))
> + {
> + hwcap_mask = HWCAP_S390_VXRS_EXT2;
> + if (!t.disable)
> + hwcap_mask |= HWCAP_S390_VXRS | HWCAP_S390_VXRS_EXT;
> }
> + else if (tunable_str_comma_strcmp_cte (&t, "STFLE_MIE3"))
> + stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
>
> /* Perform the actions determined above. */
> if (reset_features)
> @@ -187,14 +147,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> else
> cpu_features_curr.stfle_bits[0] |= stfle_bits0_mask;
> }
> -
> - /* Jump over current token ... */
> - token += token_len;
> -
> - /* ... and skip token separator for next round. */
> - if (*token == ',') token++;
> }
> - while (*token != '\0');
>
> /* Copy back the features after checking that no unsupported features were
> enabled by user. */
> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> index a11a86efab..c57f154b48 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> @@ -16,10 +16,12 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> +#include <array_length.h>
> #include <cpu-features.h>
> #include <sys/auxv.h>
> #include <elf/dl-hwcaps.h>
> #include <sys/prctl.h>
> +#include <dl-tunables-parse.h>
>
> #define DCZID_DZP_MASK (1 << 4)
> #define DCZID_BS_MASK (0xf)
> @@ -33,25 +35,28 @@
> struct cpu_list
> {
> const char *name;
> + size_t len;
> uint64_t midr;
> };
>
> -static struct cpu_list cpu_list[] = {
> - {"thunderxt88", 0x430F0A10},
> - {"thunderx2t99", 0x431F0AF0},
> - {"thunderx2t99p1", 0x420F5160},
> - {"ares", 0x411FD0C0},
> - {"emag", 0x503F0001},
> - {"kunpeng920", 0x481FD010},
> - {"a64fx", 0x460F0010},
> - {"generic", 0x0}
> +static const struct cpu_list cpu_list[] =
> +{
> +#define CPU_LIST_ENTRY(__str, __num) { __str, sizeof (__str) - 1, __num }
> + CPU_LIST_ENTRY ("thunderxt88", 0x430F0A10),
> + CPU_LIST_ENTRY ("thunderx2t99", 0x431F0AF0),
> + CPU_LIST_ENTRY ("thunderx2t99p1", 0x420F5160),
> + CPU_LIST_ENTRY ("ares", 0x411FD0C0),
> + CPU_LIST_ENTRY ("emag", 0x503F0001),
> + CPU_LIST_ENTRY ("kunpeng920", 0x481FD010),
> + CPU_LIST_ENTRY ("a64fx", 0x460F0010),
> + CPU_LIST_ENTRY ("generic", 0x0),
> };
>
> static uint64_t
> -get_midr_from_mcpu (const char *mcpu)
> +get_midr_from_mcpu (const struct tunable_str_t *mcpu)
> {
> - for (int i = 0; i < sizeof (cpu_list) / sizeof (struct cpu_list); i++)
> - if (strcmp (mcpu, cpu_list[i].name) == 0)
> + for (int i = 0; i < array_length (cpu_list); i++)
> + if (tunable_strcmp (mcpu, cpu_list[i].name, cpu_list[i].len))
> return cpu_list[i].midr;
>
> return UINT64_MAX;
> @@ -63,7 +68,9 @@ init_cpu_features (struct cpu_features *cpu_features)
> register uint64_t midr = UINT64_MAX;
>
> /* Get the tunable override. */
> - const char *mcpu = TUNABLE_GET (glibc, cpu, name, const char *, NULL);
> + const struct tunable_str_t *mcpu = TUNABLE_GET (glibc, cpu, name,
> + struct tunable_str_t *,
> + NULL);
> if (mcpu != NULL)
> midr = get_midr_from_mcpu (mcpu);
>
> diff --git a/sysdeps/unix/sysv/linux/powerpc/cpu-features.c b/sysdeps/unix/sysv/linux/powerpc/cpu-features.c
> index 7c6e20e702..390b3fd11a 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/cpu-features.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/cpu-features.c
> @@ -20,6 +20,7 @@
> #include <stdint.h>
> #include <cpu-features.h>
> #include <elf/dl-tunables.h>
> +#include <dl-tunables-parse.h>
> #include <unistd.h>
> #include <string.h>
>
> @@ -43,41 +44,26 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> struct cpu_features *cpu_features = &GLRO(dl_powerpc_cpu_features);
> unsigned long int tcbv_hwcap = cpu_features->hwcap;
> unsigned long int tcbv_hwcap2 = cpu_features->hwcap2;
> - const char *token = valp->strval;
> - do
> +
> + struct tunable_str_comma_state_t ts;
> + tunable_str_comma_init (&ts, valp);
> +
> + struct tunable_str_comma_t t;
> + while (tunable_str_comma_next (&ts, &t))
> {
> - const char *token_end, *feature;
> - bool disable;
> - size_t token_len, i, feature_len, offset = 0;
> - /* Find token separator or end of string. */
> - for (token_end = token; *token_end != ','; token_end++)
> - if (*token_end == '\0')
> - break;
> + if (t.len == 0)
> + continue;
>
> - /* Determine feature. */
> - token_len = token_end - token;
> - if (*token == '-')
> - {
> - disable = true;
> - feature = token + 1;
> - feature_len = token_len - 1;
> - }
> - else
> - {
> - disable = false;
> - feature = token;
> - feature_len = token_len;
> - }
> - for (i = 0; i < array_length (hwcap_tunables); ++i)
> + size_t offset = 0;
> + for (int i = 0; i < array_length (hwcap_tunables); ++i)
> {
> const char *hwcap_name = hwcap_names + offset;
> size_t hwcap_name_len = strlen (hwcap_name);
> /* Check the tunable name on the supported list. */
> - if (hwcap_name_len == feature_len
> - && memcmp (feature, hwcap_name, feature_len) == 0)
> + if (tunable_str_comma_strcmp (&t, hwcap_name, hwcap_name_len))
> {
> /* Update the hwcap and hwcap2 bits. */
> - if (disable)
> + if (t.disable)
> {
> /* Id is 1 for hwcap2 tunable. */
> if (hwcap_tunables[i].id)
> @@ -98,12 +84,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> }
> offset += hwcap_name_len + 1;
> }
> - token += token_len;
> - /* ... and skip token separator for next round. */
> - if (*token == ',')
> - token++;
> }
> - while (*token != '\0');
> }
>
> static inline void
> diff --git a/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c b/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c
> index 2631016a3a..049164f841 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c
> @@ -110,7 +110,11 @@ do_test (int argc, char *argv[])
> run_test ("-arch_2_06", "__memcpy_power7");
> if (hwcap & PPC_FEATURE_ARCH_2_05)
> run_test ("-arch_2_06,-arch_2_05","__memcpy_power6");
> - run_test ("-arch_2_06,-arch_2_05,-power5+,-power5,-power4", "__memcpy_power4");
> + run_test ("-arch_2_06,-arch_2_05,-power5+,-power5,-power4",
> + "__memcpy_power4");
> + /* Also run with valid, but empty settings. */
> + run_test (",-,-arch_2_06,-arch_2_05,-power5+,-power5,,-power4,-",
> + "__memcpy_power4");
> }
> else
> {
> diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
> index 917c26f116..a64e5f002a 100644
> --- a/sysdeps/x86/Makefile
> +++ b/sysdeps/x86/Makefile
> @@ -12,7 +12,8 @@ CFLAGS-get-cpuid-feature-leaf.o += $(no-stack-protector)
>
> tests += tst-get-cpu-features tst-get-cpu-features-static \
> tst-cpu-features-cpuinfo tst-cpu-features-cpuinfo-static \
> - tst-cpu-features-supports tst-cpu-features-supports-static
> + tst-cpu-features-supports tst-cpu-features-supports-static \
> + tst-hwcap-tunables
> tests-static += tst-get-cpu-features-static \
> tst-cpu-features-cpuinfo-static \
> tst-cpu-features-supports-static
> @@ -65,6 +66,7 @@ $(objpfx)tst-isa-level-1.out: $(objpfx)tst-isa-level-mod-1-baseline.so \
> endif
> tst-ifunc-isa-2-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SSE4_2,-AVX,-AVX2,-AVX512F
> tst-ifunc-isa-2-static-ENV = $(tst-ifunc-isa-2-ENV)
> +tst-hwcap-tunables-ARGS = -- $(host-test-program-cmd)
> endif
>
> ifeq ($(subdir),math)
> diff --git a/sysdeps/x86/cpu-tunables.c b/sysdeps/x86/cpu-tunables.c
> index 5697885226..ef96148d30 100644
> --- a/sysdeps/x86/cpu-tunables.c
> +++ b/sysdeps/x86/cpu-tunables.c
> @@ -24,11 +24,12 @@
> #include <string.h>
> #include <cpu-features.h>
> #include <ldsodefs.h>
> +#include <dl-tunables-parse.h>
> #include <dl-symbol-redir-ifunc.h>
>
> #define CHECK_GLIBC_IFUNC_CPU_OFF(f, cpu_features, name, len) \
> _Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
> - if (memcmp (f, #name, len) == 0) \
> + if (tunable_str_comma_strcmp_cte (&f, #name)) \
> { \
> CPU_FEATURE_UNSET (cpu_features, name) \
> break; \
> @@ -38,7 +39,7 @@
> which isn't available. */
> #define CHECK_GLIBC_IFUNC_PREFERRED_OFF(f, cpu_features, name, len) \
> _Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
> - if (memcmp (f, #name, len) == 0) \
> + if (tunable_str_comma_strcmp_cte (&f, #name) == 0) \
> { \
> cpu_features->preferred[index_arch_##name] \
> &= ~bit_arch_##name; \
> @@ -46,12 +47,11 @@
> }
>
> /* Enable/disable a preferred feature NAME. */
> -#define CHECK_GLIBC_IFUNC_PREFERRED_BOTH(f, cpu_features, name, \
> - disable, len) \
> +#define CHECK_GLIBC_IFUNC_PREFERRED_BOTH(f, cpu_features, name, len) \
> _Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
> - if (memcmp (f, #name, len) == 0) \
> + if (tunable_str_comma_strcmp_cte (&f, #name)) \
> { \
> - if (disable) \
> + if (f.disable) \
> cpu_features->preferred[index_arch_##name] &= ~bit_arch_##name; \
> else \
> cpu_features->preferred[index_arch_##name] |= bit_arch_##name; \
> @@ -61,11 +61,11 @@
> /* Enable/disable a preferred feature NAME. Enable a preferred feature
> only if the feature NEED is usable. */
> #define CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH(f, cpu_features, name, \
> - need, disable, len) \
> + need, len) \
> _Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
> - if (memcmp (f, #name, len) == 0) \
> + if (tunable_str_comma_strcmp_cte (&f, #name)) \
> { \
> - if (disable) \
> + if (f.disable) \
> cpu_features->preferred[index_arch_##name] &= ~bit_arch_##name; \
> else if (CPU_FEATURE_USABLE_P (cpu_features, need)) \
> cpu_features->preferred[index_arch_##name] |= bit_arch_##name; \
> @@ -93,38 +93,20 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> NOTE: the IFUNC selection may change over time. Please check all
> multiarch implementations when experimenting. */
>
> - const char *p = valp->strval, *c;
> struct cpu_features *cpu_features = &GLRO(dl_x86_cpu_features);
> - size_t len;
>
> - do
> - {
> - const char *n;
> - bool disable;
> - size_t nl;
> -
> - for (c = p; *c != ','; c++)
> - if (*c == '\0')
> - break;
> + struct tunable_str_comma_state_t ts;
> + tunable_str_comma_init (&ts, valp);
>
> - len = c - p;
> - disable = *p == '-';
> - if (disable)
> - {
> - n = p + 1;
> - nl = len - 1;
> - }
> - else
> - {
> - n = p;
> - nl = len;
> - }
> - switch (nl)
> + struct tunable_str_comma_t n;
> + while (tunable_str_comma_next (&ts, &n))
> + {
> + switch (n.len)
> {
> default:
> break;
> case 3:
> - if (disable)
> + if (n.disable)
> {
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX, 3);
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, CX8, 3);
> @@ -135,7 +117,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> }
> break;
> case 4:
> - if (disable)
> + if (n.disable)
> {
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX2, 4);
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, BMI1, 4);
> @@ -149,7 +131,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> }
> break;
> case 5:
> - if (disable)
> + if (n.disable)
> {
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, LZCNT, 5);
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, MOVBE, 5);
> @@ -159,12 +141,12 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> }
> break;
> case 6:
> - if (disable)
> + if (n.disable)
> {
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, POPCNT, 6);
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, SSE4_1, 6);
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, SSE4_2, 6);
> - if (memcmp (n, "XSAVEC", 6) == 0)
> + if (memcmp (n.str, "XSAVEC", 6) == 0)
> {
> /* Update xsave_state_size to XSAVE state size. */
> cpu_features->xsave_state_size
> @@ -174,14 +156,14 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> }
> break;
> case 7:
> - if (disable)
> + if (n.disable)
> {
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512F, 7);
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, OSXSAVE, 7);
> }
> break;
> case 8:
> - if (disable)
> + if (n.disable)
> {
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512CD, 8);
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512BW, 8);
> @@ -190,86 +172,72 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512PF, 8);
> CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512VL, 8);
> }
> - CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features, Slow_BSF,
> - disable, 8);
> + CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features, Slow_BSF, 8);
> break;
> case 11:
> {
> - CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
> - Prefer_ERMS,
> - disable, 11);
> - CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
> - Prefer_FSRM,
> - disable, 11);
> + CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features, Prefer_ERMS,
> + 11);
> + CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features, Prefer_FSRM,
> + 11);
> CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH (n, cpu_features,
> Slow_SSE4_2,
> SSE4_2,
> - disable, 11);
> + 11);
> }
> break;
> case 15:
> {
> CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
> - Fast_Rep_String,
> - disable, 15);
> + Fast_Rep_String, 15);
> }
> break;
> case 16:
> {
> CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
> - (n, cpu_features, Prefer_No_AVX512, AVX512F,
> - disable, 16);
> + (n, cpu_features, Prefer_No_AVX512, AVX512F, 16);
> }
> break;
> case 18:
> {
> CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
> - Fast_Copy_Backward,
> - disable, 18);
> + Fast_Copy_Backward, 18);
> }
> break;
> case 19:
> {
> CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
> - Fast_Unaligned_Load,
> - disable, 19);
> + Fast_Unaligned_Load, 19);
> CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
> - Fast_Unaligned_Copy,
> - disable, 19);
> + Fast_Unaligned_Copy, 19);
> }
> break;
> case 20:
> {
> CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
> - (n, cpu_features, Prefer_No_VZEROUPPER, AVX, disable,
> - 20);
> + (n, cpu_features, Prefer_No_VZEROUPPER, AVX, 20);
> }
> break;
> case 23:
> {
> CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
> - (n, cpu_features, AVX_Fast_Unaligned_Load, AVX,
> - disable, 23);
> + (n, cpu_features, AVX_Fast_Unaligned_Load, AVX, 23);
> }
> break;
> case 24:
> {
> CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
> - (n, cpu_features, MathVec_Prefer_No_AVX512, AVX512F,
> - disable, 24);
> + (n, cpu_features, MathVec_Prefer_No_AVX512, AVX512F, 24);
> }
> break;
> case 26:
> {
> CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
> - (n, cpu_features, Prefer_PMINUB_for_stringop, SSE2,
> - disable, 26);
> + (n, cpu_features, Prefer_PMINUB_for_stringop, SSE2, 26);
> }
> break;
> }
> - p += len + 1;
> }
> - while (*c != '\0');
> }
>
> #if CET_ENABLED
> @@ -277,11 +245,11 @@ attribute_hidden
> void
> TUNABLE_CALLBACK (set_x86_ibt) (tunable_val_t *valp)
> {
> - if (memcmp (valp->strval, "on", sizeof ("on")) == 0)
> + if (tunable_strcmp_cte (valp, "on"))
> GL(dl_x86_feature_control).ibt = cet_always_on;
> - else if (memcmp (valp->strval, "off", sizeof ("off")) == 0)
> + else if (tunable_strcmp_cte (valp, "off"))
> GL(dl_x86_feature_control).ibt = cet_always_off;
> - else if (memcmp (valp->strval, "permissive", sizeof ("permissive")) == 0)
> + else if (tunable_strcmp_cte (valp, "permissive"))
> GL(dl_x86_feature_control).ibt = cet_permissive;
> }
>
> @@ -289,11 +257,11 @@ attribute_hidden
> void
> TUNABLE_CALLBACK (set_x86_shstk) (tunable_val_t *valp)
> {
> - if (memcmp (valp->strval, "on", sizeof ("on")) == 0)
> + if (tunable_strcmp_cte (valp, "on"))
> GL(dl_x86_feature_control).shstk = cet_always_on;
> - else if (memcmp (valp->strval, "off", sizeof ("off")) == 0)
> + else if (tunable_strcmp_cte (valp, "off"))
> GL(dl_x86_feature_control).shstk = cet_always_off;
> - else if (memcmp (valp->strval, "permissive", sizeof ("permissive")) == 0)
> + else if (tunable_strcmp_cte (valp, "permissive"))
> GL(dl_x86_feature_control).shstk = cet_permissive;
> }
> #endif
> diff --git a/sysdeps/x86/tst-hwcap-tunables.c b/sysdeps/x86/tst-hwcap-tunables.c
> new file mode 100644
> index 0000000000..01a9377f7e
> --- /dev/null
> +++ b/sysdeps/x86/tst-hwcap-tunables.c
> @@ -0,0 +1,148 @@
> +/* Tests for x86 GLIBC_TUNABLES=glibc.cpu.hwcaps filter.
> + Copyright (C) 2023 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <array_length.h>
> +#include <getopt.h>
> +#include <ifunc-impl-list.h>
> +#include <spawn.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <intprops.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <support/xunistd.h>
> +#include <support/capture_subprocess.h>
> +
> +/* Nonzero if the program gets called via `exec'. */
> +#define CMDLINE_OPTIONS \
> + { "restart", no_argument, &restart, 1 },
> +static int restart;
> +
> +/* Disable everything. */
> +static const char *test_1[] =
> +{
> + "__memcpy_avx512_no_vzeroupper",
> + "__memcpy_avx512_unaligned",
> + "__memcpy_avx512_unaligned_erms",
> + "__memcpy_evex_unaligned",
> + "__memcpy_evex_unaligned_erms",
> + "__memcpy_avx_unaligned",
> + "__memcpy_avx_unaligned_erms",
> + "__memcpy_avx_unaligned_rtm",
> + "__memcpy_avx_unaligned_erms_rtm",
> + "__memcpy_ssse3",
> +};
> +
> +static const struct test_t
> +{
> + const char *env;
> + const char *const *funcs;
> + size_t nfuncs;
> +} tests[] =
> +{
> + {
> + /* Disable everything. */
> + "-Prefer_ERMS,-Prefer_FSRM,-AVX,-AVX2,-AVX_Usable,-AVX2_Usable,"
> + "-AVX512F_Usable,-SSE4_1,-SSE4_2,-SSSE3,-Fast_Unaligned_Load,-ERMS,"
> + "-AVX_Fast_Unaligned_Load",
> + test_1,
> + array_length (test_1)
> + },
> + {
> + /* Same as before, but with some empty suboptions. */
> + ",-,-Prefer_ERMS,-Prefer_FSRM,-AVX,-AVX2,-AVX_Usable,-AVX2_Usable,"
> + "-AVX512F_Usable,-SSE4_1,-SSE4_2,,-SSSE3,-Fast_Unaligned_Load,,-,-ERMS,"
> + "-AVX_Fast_Unaligned_Load,-,",
> + test_1,
> + array_length (test_1)
> + }
> +};
> +
> +/* Called on process re-execution. */
> +_Noreturn static void
> +handle_restart (int ntest)
> +{
> + struct libc_ifunc_impl impls[32];
> + int cnt = __libc_ifunc_impl_list ("memcpy", impls, array_length (impls));
> + if (cnt == 0)
> + _exit (EXIT_SUCCESS);
> + TEST_VERIFY_EXIT (cnt >= 1);
> + for (int i = 0; i < cnt; i++)
> + {
> + for (int f = 0; f < tests[ntest].nfuncs; f++)
> + {
> + if (strcmp (impls[i].name, tests[ntest].funcs[f]) == 0)
> + TEST_COMPARE (impls[i].usable, false);
> + }
> + }
> +
> + _exit (EXIT_SUCCESS);
> +}
> +
> +static int
> +do_test (int argc, char *argv[])
> +{
> + /* We must have either:
> + - One our fource parameters left if called initially:
> + + path to ld.so optional
> + + "--library-path" optional
> + + the library path optional
> + + the application name
> + + the test to check */
> +
> + TEST_VERIFY_EXIT (argc == 2 || argc == 5);
> +
> + if (restart)
> + handle_restart (atoi (argv[1]));
> +
> + char nteststr[INT_BUFSIZE_BOUND (int)];
> +
> + char *spargv[10];
> + {
> + int i = 0;
> + for (; i < argc - 1; i++)
> + spargv[i] = argv[i + 1];
> + spargv[i++] = (char *) "--direct";
> + spargv[i++] = (char *) "--restart";
> + spargv[i++] = nteststr;
> + spargv[i] = NULL;
> + }
> +
> + for (int i = 0; i < array_length (tests); i++)
> + {
> + snprintf (nteststr, sizeof nteststr, "%d", i);
> +
> + printf ("[%d] Spawned test for %s\n", i, tests[i].env);
> + char *tunable = xasprintf ("glibc.cpu.hwcaps=%s", tests[i].env);
> + setenv ("GLIBC_TUNABLES", tunable, 1);
> +
> + struct support_capture_subprocess result
> + = support_capture_subprogram (spargv[0], spargv);
> + support_capture_subprocess_check (&result, "tst-tunables", 0,
> + sc_allow_stderr);
> + support_capture_subprocess_free (&result);
> +
> + free (tunable);
> + }
> +
> + return 0;
> +}
> +
> +#define TEST_FUNCTION_ARGV do_test
> +#include <support/test-driver.c>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 2/5] elf: Do not set invalid tunables values
2023-11-22 20:43 ` [PATCH v5 2/5] elf: Do not set invalid tunables values Adhemerval Zanella
@ 2023-12-01 15:32 ` Siddhesh Poyarekar
2023-12-06 13:06 ` Adhemerval Zanella Netto
0 siblings, 1 reply; 10+ messages in thread
From: Siddhesh Poyarekar @ 2023-12-01 15:32 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
On 2023-11-22 15:43, Adhemerval Zanella wrote:
> The loader now warns for invalid and out-of-range tunable values. The
> patch also fixes the parsing of size_t maximum values, where
> _dl_strtoul was failing for large values close to SIZE_MAX.
>
> Checked on x86_64-linux-gnu.
> ---
> elf/dl-misc.c | 5 ++++-
> elf/dl-tunables.c | 35 ++++++++++++++++++++++++++++++-----
> elf/tst-tunables.c | 30 ++++++++++++++++++++++++++++++
> 3 files changed, 64 insertions(+), 6 deletions(-)
>
> diff --git a/elf/dl-misc.c b/elf/dl-misc.c
> index 5b84adc2f4..037cbb3650 100644
> --- a/elf/dl-misc.c
> +++ b/elf/dl-misc.c
> @@ -190,6 +190,9 @@ _dl_strtoul (const char *nptr, char **endptr)
> }
> }
>
> + const uint64_t cutoff = (UINT64_MAX * 2UL + 1UL) / 10;
> + const uint64_t cutlim = (UINT64_MAX * 2UL + 1UL) % 10;
> +
> while (1)
> {
> int digval;
> @@ -207,7 +210,7 @@ _dl_strtoul (const char *nptr, char **endptr)
> else
> break;
>
> - if (result >= (UINT64_MAX - digval) / base)
> + if (result > cutoff || (result == cutoff && digval > cutlim))
I don't understand this change; how does this work with octal or
hexadecimal inputs?
> {
> if (endptr != NULL)
> *endptr = (char *) nptr;
> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
> index 26161c68e7..67a37ff704 100644
> --- a/elf/dl-tunables.c
> +++ b/elf/dl-tunables.c
> @@ -77,16 +77,27 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
> {
> tunable_num_t val, min, max;
>
> - if (cur->type.type_code == TUNABLE_TYPE_STRING)
> + switch (cur->type.type_code)
> {
> + case TUNABLE_TYPE_STRING:
> cur->val.strval = valp->strval;
> cur->initialized = true;
> return;
> + case TUNABLE_TYPE_INT_32:
> + val = (int32_t) valp->numval;
> + break;
> + case TUNABLE_TYPE_UINT_64:
> + val = (int64_t) valp->numval;
> + break;
> + case TUNABLE_TYPE_SIZE_T:
> + val = (size_t) valp->numval;
> + break;
> + default:
> + __builtin_unreachable ();
> }
>
> bool unsigned_cmp = unsigned_tunable_type (cur->type.type_code);
>
> - val = valp->numval;
> min = minp != NULL ? *minp : cur->type.min;
> max = maxp != NULL ? *maxp : cur->type.max;
>
> @@ -117,16 +128,24 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
>
> /* Validate range of the input value and initialize the tunable CUR if it looks
> good. */
> -static void
> +static bool
> tunable_initialize (tunable_t *cur, const char *strval, size_t len)
> {
> tunable_val_t val = { 0 };
>
> if (cur->type.type_code != TUNABLE_TYPE_STRING)
> - val.numval = (tunable_num_t) _dl_strtoul (strval, NULL);
> + {
> + char *endptr = NULL;
> + uint64_t numval = _dl_strtoul (strval, &endptr);
> + if (endptr != strval + len)
> + return false;
> + val.numval = (tunable_num_t) numval;
> + }
> else
> val.strval = (struct tunable_str_t) { strval, len };
> do_tunable_update_val (cur, &val, NULL, NULL);
> +
> + return true;
> }
>
> void
> @@ -226,7 +245,13 @@ parse_tunables (const char *valstring)
> }
>
> for (int i = 0; i < ntunables; i++)
> - tunable_initialize (tunables[i].t, tunables[i].value, tunables[i].len);
> + 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 188345b070..d6a1e1b3ac 100644
> --- a/elf/tst-tunables.c
> +++ b/elf/tst-tunables.c
> @@ -53,6 +53,13 @@ static const struct test_t
> 4096,
> 0,
> },
> + {
> + "GLIBC_TUNABLES",
> + "glibc.malloc.mmap_threshold=-1",
> + 0,
> + SIZE_MAX,
> + 0,
> + },
> /* Empty tunable are ignored. */
> {
> "GLIBC_TUNABLES",
> @@ -224,6 +231,29 @@ static const struct test_t
> 0,
> 0,
> },
> + /* Invalid numbers are ignored. */
> + {
> + "GLIBC_TUNABLES",
> + "glibc.malloc.check=abc:glibc.malloc.mmap_threshold=4096",
> + 0,
> + 4096,
> + 0,
> + },
> + {
> + "GLIBC_TUNABLES",
> + "glibc.malloc.check=2:glibc.malloc.mmap_threshold=abc",
> + 2,
> + 0,
> + 0,
> + },
> + {
> + "GLIBC_TUNABLES",
> + /* SIZE_MAX + 1 */
> + "glibc.malloc.mmap_threshold=18446744073709551616",
> + 0,
> + 0,
> + 0,
> + },
> /* Also check some tunable aliases. */
> {
> "MALLOC_CHECK_",
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 3/5] elf: Ignore loader debug env vars for setuid
2023-11-22 20:43 ` [PATCH v5 3/5] elf: Ignore loader debug env vars for setuid Adhemerval Zanella
@ 2023-12-01 15:34 ` Siddhesh Poyarekar
0 siblings, 0 replies; 10+ messages in thread
From: Siddhesh Poyarekar @ 2023-12-01 15:34 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
On 2023-11-22 15:43, Adhemerval Zanella wrote:
> Loader already ignores LD_DEBUG, LD_DEBUG_OUTPUT, and
> LD_TRACE_LOADED_OBJECTS. Both LD_WARN and LD_VERBOSE are similar to
> LD_DEBUG, in the sense they enable additional checks and debug
> information, so it makes sense to disable them.
>
> Also add both LD_VERBOSE and LD_WARN on filtered environment variables
> for setuid binaries.
LGTM.
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
I think patches 3-5 are independent of the remaining two patches, so you
could push them if you want.
Thanks,
Sid
>
> Checked on x86_64-linux-gnu.
> ---
> elf/rtld.c | 22 ++++++++++++++--------
> elf/tst-env-setuid.c | 4 ++++
> sysdeps/generic/unsecvars.h | 2 ++
> 3 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 0553c05edb..d1017ba9e9 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -2550,13 +2550,15 @@ process_envvars (struct dl_main_state *state)
> {
> case 4:
> /* Warning level, verbose or not. */
> - if (memcmp (envline, "WARN", 4) == 0)
> + if (!__libc_enable_secure
> + && memcmp (envline, "WARN", 4) == 0)
> GLRO(dl_verbose) = envline[5] != '\0';
> break;
>
> case 5:
> /* Debugging of the dynamic linker? */
> - if (memcmp (envline, "DEBUG", 5) == 0)
> + if (!__libc_enable_secure
> + && memcmp (envline, "DEBUG", 5) == 0)
> {
> process_dl_debug (state, &envline[6]);
> break;
> @@ -2571,7 +2573,8 @@ process_envvars (struct dl_main_state *state)
>
> case 7:
> /* Print information about versions. */
> - if (memcmp (envline, "VERBOSE", 7) == 0)
> + if (!__libc_enable_secure
> + && memcmp (envline, "VERBOSE", 7) == 0)
> {
> state->version_info = envline[8] != '\0';
> break;
> @@ -2630,7 +2633,8 @@ process_envvars (struct dl_main_state *state)
> }
>
> /* Where to place the profiling data file. */
> - if (memcmp (envline, "DEBUG_OUTPUT", 12) == 0)
> + if (!__libc_enable_secure
> + && memcmp (envline, "DEBUG_OUTPUT", 12) == 0)
> {
> debug_output = &envline[13];
> break;
> @@ -2651,7 +2655,8 @@ process_envvars (struct dl_main_state *state)
>
> case 20:
> /* The mode of the dynamic linker can be set. */
> - if (memcmp (envline, "TRACE_LOADED_OBJECTS", 20) == 0)
> + if (!__libc_enable_secure
> + && memcmp (envline, "TRACE_LOADED_OBJECTS", 20) == 0)
> {
> state->mode = rtld_mode_trace;
> state->mode_trace_program
> @@ -2673,9 +2678,10 @@ process_envvars (struct dl_main_state *state)
> }
> while (*nextp != '\0');
>
> - GLRO(dl_debug_mask) = 0;
> -
> - if (state->mode != rtld_mode_normal)
> + if (GLRO(dl_debug_mask) != 0
> + || GLRO(dl_verbose) != 0
> + || state->mode != rtld_mode_normal
> + || state->version_info)
> _exit (5);
> }
> /* If we have to run the dynamic linker in debugging mode and the
> diff --git a/elf/tst-env-setuid.c b/elf/tst-env-setuid.c
> index 76b8e1fb45..b1d64ac085 100644
> --- a/elf/tst-env-setuid.c
> +++ b/elf/tst-env-setuid.c
> @@ -59,6 +59,10 @@ static const struct envvar_t filtered_envvars[] =
> { "MALLOC_TRACE", FILTERED_VALUE },
> { "MALLOC_TRIM_THRESHOLD_", FILTERED_VALUE },
> { "RES_OPTIONS", FILTERED_VALUE },
> + { "LD_DEBUG", "all" },
> + { "LD_DEBUG_OUTPUT", "/tmp/some-file" },
> + { "LD_WARN", FILTERED_VALUE },
> + { "LD_VERBOSE", FILTERED_VALUE },
> };
>
> static const struct envvar_t unfiltered_envvars[] =
> diff --git a/sysdeps/generic/unsecvars.h b/sysdeps/generic/unsecvars.h
> index f7ebed60e5..8975df4a14 100644
> --- a/sysdeps/generic/unsecvars.h
> +++ b/sysdeps/generic/unsecvars.h
> @@ -16,6 +16,8 @@
> "LD_PRELOAD\0" \
> "LD_PROFILE\0" \
> "LD_SHOW_AUXV\0" \
> + "LD_VERBOSE\0" \
> + "LD_WARN\0" \
> "LOCALDOMAIN\0" \
> "LOCPATH\0" \
> "MALLOC_ARENA_MAX\0" \
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 2/5] elf: Do not set invalid tunables values
2023-12-01 15:32 ` Siddhesh Poyarekar
@ 2023-12-06 13:06 ` Adhemerval Zanella Netto
0 siblings, 0 replies; 10+ messages in thread
From: Adhemerval Zanella Netto @ 2023-12-06 13:06 UTC (permalink / raw)
To: Siddhesh Poyarekar, libc-alpha
On 01/12/23 12:32, Siddhesh Poyarekar wrote:
>
>
> On 2023-11-22 15:43, Adhemerval Zanella wrote:
>> The loader now warns for invalid and out-of-range tunable values. The
>> patch also fixes the parsing of size_t maximum values, where
>> _dl_strtoul was failing for large values close to SIZE_MAX.
>>
>> Checked on x86_64-linux-gnu.
>> ---
>> elf/dl-misc.c | 5 ++++-
>> elf/dl-tunables.c | 35 ++++++++++++++++++++++++++++++-----
>> elf/tst-tunables.c | 30 ++++++++++++++++++++++++++++++
>> 3 files changed, 64 insertions(+), 6 deletions(-)
>>
>> diff --git a/elf/dl-misc.c b/elf/dl-misc.c
>> index 5b84adc2f4..037cbb3650 100644
>> --- a/elf/dl-misc.c
>> +++ b/elf/dl-misc.c
>> @@ -190,6 +190,9 @@ _dl_strtoul (const char *nptr, char **endptr)
>> }
>> }
>> + const uint64_t cutoff = (UINT64_MAX * 2UL + 1UL) / 10;
>> + const uint64_t cutlim = (UINT64_MAX * 2UL + 1UL) % 10;
>> +
>> while (1)
>> {
>> int digval;
>> @@ -207,7 +210,7 @@ _dl_strtoul (const char *nptr, char **endptr)
>> else
>> break;
>> - if (result >= (UINT64_MAX - digval) / base)
>> + if (result > cutoff || (result == cutoff && digval > cutlim))
>
> I don't understand this change; how does this work with octal or hexadecimal inputs?
In fact the cutoff/cutlim should be adjusted when a different base is used,
I will fix it. The logic here is similar to the stdlib/strtol_l.c:486.
>
>> {
>> if (endptr != NULL)
>> *endptr = (char *) nptr;
>> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
>> index 26161c68e7..67a37ff704 100644
>> --- a/elf/dl-tunables.c
>> +++ b/elf/dl-tunables.c
>> @@ -77,16 +77,27 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
>> {
>> tunable_num_t val, min, max;
>> - if (cur->type.type_code == TUNABLE_TYPE_STRING)
>> + switch (cur->type.type_code)
>> {
>> + case TUNABLE_TYPE_STRING:
>> cur->val.strval = valp->strval;
>> cur->initialized = true;
>> return;
>> + case TUNABLE_TYPE_INT_32:
>> + val = (int32_t) valp->numval;
>> + break;
>> + case TUNABLE_TYPE_UINT_64:
>> + val = (int64_t) valp->numval;
>> + break;
>> + case TUNABLE_TYPE_SIZE_T:
>> + val = (size_t) valp->numval;
>> + break;
>> + default:
>> + __builtin_unreachable ();
>> }
>> bool unsigned_cmp = unsigned_tunable_type (cur->type.type_code);
>> - val = valp->numval;
>> min = minp != NULL ? *minp : cur->type.min;
>> max = maxp != NULL ? *maxp : cur->type.max;
>> @@ -117,16 +128,24 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
>> /* Validate range of the input value and initialize the tunable CUR if it looks
>> good. */
>> -static void
>> +static bool
>> tunable_initialize (tunable_t *cur, const char *strval, size_t len)
>> {
>> tunable_val_t val = { 0 };
>> if (cur->type.type_code != TUNABLE_TYPE_STRING)
>> - val.numval = (tunable_num_t) _dl_strtoul (strval, NULL);
>> + {
>> + char *endptr = NULL;
>> + uint64_t numval = _dl_strtoul (strval, &endptr);
>> + if (endptr != strval + len)
>> + return false;
>> + val.numval = (tunable_num_t) numval;
>> + }
>> else
>> val.strval = (struct tunable_str_t) { strval, len };
>> do_tunable_update_val (cur, &val, NULL, NULL);
>> +
>> + return true;
>> }
>> void
>> @@ -226,7 +245,13 @@ parse_tunables (const char *valstring)
>> }
>> for (int i = 0; i < ntunables; i++)
>> - tunable_initialize (tunables[i].t, tunables[i].value, tunables[i].len);
>> + 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 188345b070..d6a1e1b3ac 100644
>> --- a/elf/tst-tunables.c
>> +++ b/elf/tst-tunables.c
>> @@ -53,6 +53,13 @@ static const struct test_t
>> 4096,
>> 0,
>> },
>> + {
>> + "GLIBC_TUNABLES",
>> + "glibc.malloc.mmap_threshold=-1",
>> + 0,
>> + SIZE_MAX,
>> + 0,
>> + },
>> /* Empty tunable are ignored. */
>> {
>> "GLIBC_TUNABLES",
>> @@ -224,6 +231,29 @@ static const struct test_t
>> 0,
>> 0,
>> },
>> + /* Invalid numbers are ignored. */
>> + {
>> + "GLIBC_TUNABLES",
>> + "glibc.malloc.check=abc:glibc.malloc.mmap_threshold=4096",
>> + 0,
>> + 4096,
>> + 0,
>> + },
>> + {
>> + "GLIBC_TUNABLES",
>> + "glibc.malloc.check=2:glibc.malloc.mmap_threshold=abc",
>> + 2,
>> + 0,
>> + 0,
>> + },
>> + {
>> + "GLIBC_TUNABLES",
>> + /* SIZE_MAX + 1 */
>> + "glibc.malloc.mmap_threshold=18446744073709551616",
>> + 0,
>> + 0,
>> + 0,
>> + },
>> /* Also check some tunable aliases. */
>> {
>> "MALLOC_CHECK_",
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-12-06 13:06 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-22 20:43 [PATCH v5 0/5] Improve loader environment variable handling Adhemerval Zanella
2023-11-22 20:43 ` [PATCH v5 1/5] elf: Do not duplicate the GLIBC_TUNABLES string Adhemerval Zanella
2023-12-01 15:20 ` Siddhesh Poyarekar
2023-11-22 20:43 ` [PATCH v5 2/5] elf: Do not set invalid tunables values Adhemerval Zanella
2023-12-01 15:32 ` Siddhesh Poyarekar
2023-12-06 13:06 ` Adhemerval Zanella Netto
2023-11-22 20:43 ` [PATCH v5 3/5] elf: Ignore loader debug env vars for setuid Adhemerval Zanella
2023-12-01 15:34 ` Siddhesh Poyarekar
2023-11-22 20:43 ` [PATCH v5 4/5] elf: Ignore LD_BIND_NOW and LD_BIND_NOT for setuid binaries Adhemerval Zanella
2023-11-22 20:43 ` [PATCH v5 5/5] elf: Refactor process_envvars Adhemerval Zanella
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).