* [PATCH 00/11] Improve tunable handling
@ 2023-10-10 18:01 Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 01/11] elf: Remove /etc/suid-debug support Adhemerval Zanella
` (10 more replies)
0 siblings, 11 replies; 27+ messages in thread
From: Adhemerval Zanella @ 2023-10-10 18:01 UTC (permalink / raw)
To: libc-alpha, Siddhesh Poyarekar
The recent CVE-2023-4911 fix [1] and tunable change to SXID_ERASE
discussion [2] brought some issues with the current tunable handling by
the loader. Besides the bugs in tuning parsing, some other questions
are:
* What should be the security boundaries for tunable and other tuning
* environment variables?
* Should tunables be filtered out or be disabled altogether in setuid
* binaries [3]?
* How should ld.so handle security-sensitive tunable (like malloc
* options)?
* How to handle ill-formatted tunable definition [4]?
* Is tunable copy/parsing (through tunable_strdup) required [5]?
On this patchset, I followed the idea laid out in the discussion on
whether to apply SXID_ERASE to all tunables [6]:
Ignore any tunable on AT_SECURE binaries (as some Linux distributions
are already [7]);
Add malloc tunables along with GLIBC_TUNABLES to unsecvars;
Do not parse ill-formatted GLIBC_TUNABLES strings;
Remove the requirement of duplicating the GLIBC_TUNABLES string for
parsing.
Patch #1 removes '/etc/suid-debug', which has not been working since
malloc debugging supported moved to libc_malloc_debug.so. It is one
thing less that might change AT_SECURE binaries' behavior
due to environment configurations.
Patch #2 removed tunables parsing and applying for setuid/setgid
binaries (similar to Alt Linux patch).
Patch #3 and #4 add all malloc tunable and GLIBC_TUNABLES to unsecvars
and improve tst-env-setuid.c to test all possible environment variables.
Patch #5 and #6 improved the GLIBC_TUNABLES handling to avoid handling
ill-formatted inputs.
Patch #7 makes _dl_debug_vdprintf usable before self-relocation so patch
#8 can add a loader warning that ill-formatted GLIBC_TUNABLES inputs are
ignored (it also fixes the issue where the GLIBC_TUNABLE allocation
failure will trigger a SEGFAULT on some architecture for PIE).
Patch #9, #10, and #11 remove the tunable_strdup and make the
GLIBC_TUNABLE parsing in place (no more possible allocation failure).
The parsing now tracks the tunable 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).
[1] https://sourceware.org/pipermail/libc-alpha/2023-October/151921.html
[2] https://sourceware.org/pipermail/libc-alpha/2023-October/151936.html
[3] https://www.openwall.com/lists/oss-security/2023/10/03/3
[4] https://sourceware.org/pipermail/libc-alpha/2023-October/151927.html
[5] https://sourceware.org/pipermail/libc-alpha/2023-October/151959.html
[6] https://sourceware.org/pipermail/libc-alpha/2023-October/152011.html
[7] https://git.altlinux.org/gears/g/glibc.git?p=glibc.git;a=commitdiff;h=5d1686416ab766f3dd0780ab730650c4c0f76ca9
Adhemerval Zanella (11):
elf: Remove /etc/suid-debug support
elf: Ignore GLIBC_TUNABLES for setuid/setgid binaries
elf: Add all malloc tunable to unsecvars
elf: Add GLIBC_TUNABLES to unsecvars
elf: Do not process invalid tunable format
elf: Do not parse ill-formatted strings
elf: Fix _dl_debug_vdprintf to work before self-relocation
elf: Emit warning if tunable is ill-formatted
x86: Use dl-symbol-redir-ifunc.h on cpu-tunables
s390: Use dl-symbol-redir-ifunc.h on cpu-tunables
elf: Do not duplicate the GLIBC_TUNABLES string
elf/Makefile | 5 +-
elf/dl-printf.c | 16 +-
elf/dl-tunable-types.h | 10 -
elf/dl-tunables.c | 219 +++++----------
elf/dl-tunables.h | 6 +-
elf/dl-tunables.list | 9 -
elf/rtld.c | 3 -
elf/tst-env-setuid-tunables.c | 58 ++--
elf/tst-env-setuid.c | 87 ++----
elf/tst-tunables.c | 260 ++++++++++++++++++
manual/README.tunables | 9 -
manual/memory.texi | 4 +-
manual/tunables.texi | 4 +-
scripts/gen-tunables.awk | 18 +-
stdio-common/Makefile | 5 +
stdio-common/_itoa.c | 5 +
sysdeps/generic/dl-tunables-parse.h | 128 +++++++++
sysdeps/generic/unsecvars.h | 8 +
.../i686/multiarch/dl-symbol-redir-ifunc.h | 5 +
sysdeps/s390/cpu-features.c | 169 +++++-------
.../s390/multiarch/dl-symbol-redir-ifunc.h | 2 +
.../unix/sysv/linux/aarch64/cpu-features.c | 38 ++-
.../sysv/linux/i386/dl-writev.h} | 18 +-
.../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 | 135 +++------
sysdeps/x86/tst-hwcap-tunables.c | 151 ++++++++++
sysdeps/x86_64/64/dl-tunables.list | 1 -
.../x86_64/multiarch/dl-symbol-redir-ifunc.h | 15 +
30 files changed, 888 insertions(+), 555 deletions(-)
create mode 100644 elf/tst-tunables.c
create mode 100644 sysdeps/generic/dl-tunables-parse.h
rename sysdeps/{x86_64/memcmp-isa-default-impl.h => unix/sysv/linux/i386/dl-writev.h} (62%)
create mode 100644 sysdeps/x86/tst-hwcap-tunables.c
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 01/11] elf: Remove /etc/suid-debug support
2023-10-10 18:01 [PATCH 00/11] Improve tunable handling Adhemerval Zanella
@ 2023-10-10 18:01 ` Adhemerval Zanella
2023-10-12 8:44 ` Florian Weimer
2023-10-10 18:01 ` [PATCH 02/11] elf: Ignore GLIBC_TUNABLES for setuid/setgid binaries Adhemerval Zanella
` (9 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2023-10-10 18:01 UTC (permalink / raw)
To: libc-alpha, Siddhesh Poyarekar
Since malloc debug support moved to a different library
(libc_malloc_debug.so), the glibc.malloc.check requires preloading the
debug library to enable it. It means that suid-debug support has not
been working since 2.34.
To restore its support, it would require to add additional information
and parsing to where to find libc_malloc_debug.so.
It is one thing less that might change AT_SECURE binaries' behavior
due to environment configurations.
Checked on x86_64-linux-gnu.
---
elf/dl-tunables.c | 16 ----------------
elf/rtld.c | 3 ---
manual/memory.texi | 4 +---
manual/tunables.texi | 4 +---
4 files changed, 2 insertions(+), 25 deletions(-)
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index cae67efa0a..24252af22c 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -252,20 +252,6 @@ parse_tunables (char *tunestr, char *valstring)
tunestr[off] = '\0';
}
-/* Enable the glibc.malloc.check tunable in SETUID/SETGID programs only when
- the system administrator has created the /etc/suid-debug file. This is a
- special case where we want to conditionally enable/disable a tunable even
- for setuid binaries. We use the special version of access() to avoid
- setting ERRNO, which is a TLS variable since TLS has not yet been set
- up. */
-static __always_inline void
-maybe_enable_malloc_check (void)
-{
- tunable_id_t id = TUNABLE_ENUM_NAME (glibc, malloc, check);
- if (__libc_enable_secure && __access_noerrno ("/etc/suid-debug", F_OK) == 0)
- tunable_list[id].security_level = TUNABLE_SECLEVEL_NONE;
-}
-
/* Initialize the tunables list from the environment. For now we only use the
ENV_ALIAS to find values. Later we will also use the tunable names to find
values. */
@@ -277,8 +263,6 @@ __tunables_init (char **envp)
size_t len = 0;
char **prev_envp = envp;
- maybe_enable_malloc_check ();
-
while ((envp = get_next_env (envp, &envname, &len, &envval,
&prev_envp)) != NULL)
{
diff --git a/elf/rtld.c b/elf/rtld.c
index 5107d16fe3..318a3661f0 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -2670,9 +2670,6 @@ process_envvars (struct dl_main_state *state)
}
while (*nextp != '\0');
- if (__access ("/etc/suid-debug", F_OK) != 0)
- GLRO(dl_debug_mask) = 0;
-
if (state->mode != rtld_mode_normal)
_exit (5);
}
diff --git a/manual/memory.texi b/manual/memory.texi
index 5781a64f35..258fdbd3a0 100644
--- a/manual/memory.texi
+++ b/manual/memory.texi
@@ -1379,9 +1379,7 @@ There is one problem with @code{MALLOC_CHECK_}: in SUID or SGID binaries
it could possibly be exploited since diverging from the normal programs
behavior it now writes something to the standard error descriptor.
Therefore the use of @code{MALLOC_CHECK_} is disabled by default for
-SUID and SGID binaries. It can be enabled again by the system
-administrator by adding a file @file{/etc/suid-debug} (the content is
-not important it could be empty).
+SUID and SGID binaries.
So, what's the difference between using @code{MALLOC_CHECK_} and linking
with @samp{-lmcheck}? @code{MALLOC_CHECK_} is orthogonal with respect to
diff --git a/manual/tunables.texi b/manual/tunables.texi
index 776fd93fd9..347b5698b5 100644
--- a/manual/tunables.texi
+++ b/manual/tunables.texi
@@ -136,9 +136,7 @@ termination of the process.
Like @env{MALLOC_CHECK_}, @code{glibc.malloc.check} has a problem in that it
diverges from normal program behavior by writing to @code{stderr}, which could
by exploited in SUID and SGID binaries. Therefore, @code{glibc.malloc.check}
-is disabled by default for SUID and SGID binaries. This can be enabled again
-by the system administrator by adding a file @file{/etc/suid-debug}; the
-content of the file could be anything or even empty.
+is disabled by default for SUID and SGID binaries.
@end deftp
@deftp Tunable glibc.malloc.top_pad
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 02/11] elf: Ignore GLIBC_TUNABLES for setuid/setgid binaries
2023-10-10 18:01 [PATCH 00/11] Improve tunable handling Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 01/11] elf: Remove /etc/suid-debug support Adhemerval Zanella
@ 2023-10-10 18:01 ` Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 03/11] elf: Add all malloc tunable to unsecvars Adhemerval Zanella
` (8 subsequent siblings)
10 siblings, 0 replies; 27+ messages in thread
From: Adhemerval Zanella @ 2023-10-10 18:01 UTC (permalink / raw)
To: libc-alpha, Siddhesh Poyarekar
The tunable privilege levels were a retrofit to try and keep the malloc
tunable environment variables' behavior unchanged across security
boundaries. However, CVE-2023-4911 shows how tricky can be
tunable parsing in a security-sensitive environment.
Not only parsing, but the malloc tunable essentially changes some
semantics on setuid/setgid processes. Although it is not a direct
security issue, allowing users to change setuid/setgid semantics is not
a good security practice, and requires extra code and analysis to check
if each tunable is safe to use on all security boundaries.
It also means that security opt-in features, like aarch64 MTE, would
need to be explicit enabled by an administrator with a wrapper script
or with a possible future system-wide tunable setting.
Co-authored-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
elf/Makefile | 5 +-
elf/dl-tunable-types.h | 10 --
elf/dl-tunables.c | 127 ++++-----------
elf/dl-tunables.list | 9 --
elf/tst-env-setuid-tunables.c | 65 ++++----
elf/tst-tunables.c | 243 +++++++++++++++++++++++++++++
manual/README.tunables | 9 --
scripts/gen-tunables.awk | 18 +--
sysdeps/x86_64/64/dl-tunables.list | 1 -
9 files changed, 306 insertions(+), 181 deletions(-)
create mode 100644 elf/tst-tunables.c
diff --git a/elf/Makefile b/elf/Makefile
index 9176cbf1e3..c824f7dab7 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -263,7 +263,6 @@ tests-static-normal := \
tst-dl-iter-static \
tst-dst-static \
tst-env-setuid \
- tst-env-setuid-tunables \
tst-getauxval-static \
tst-linkall-static \
tst-single_threaded-pthread-static \
@@ -276,10 +275,12 @@ tests-static-normal := \
tests-static-internal := \
tst-dl-printf-static \
tst-dl_find_object-static \
+ tst-env-setuid-tunables \
tst-ptrguard1-static \
tst-stackguard1-static \
tst-tls1-static \
tst-tls1-static-non-pie \
+ tst-tunables \
# tests-static-internal
CRT-tst-tls1-static-non-pie := $(csu-objpfx)crt1.o
@@ -2696,6 +2697,8 @@ $(objpfx)tst-glibc-hwcaps-mask.out: \
# tst-glibc-hwcaps-cache.
$(objpfx)tst-glibc-hwcaps-cache.out: $(objpfx)tst-glibc-hwcaps
+tst-tunables-ARGS = -- $(host-test-program-cmd)
+
$(objpfx)list-tunables.out: tst-rtld-list-tunables.sh $(objpfx)ld.so
$(SHELL) $< $(objpfx)ld.so '$(test-wrapper-env)' \
'$(run_program_env)' > $(objpfx)/tst-rtld-list-tunables.out
diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h
index c88332657e..62d6d9e629 100644
--- a/elf/dl-tunable-types.h
+++ b/elf/dl-tunable-types.h
@@ -64,16 +64,6 @@ struct _tunable
tunable_val_t val; /* The value. */
bool initialized; /* Flag to indicate that the tunable is
initialized. */
- tunable_seclevel_t security_level; /* Specify the security level for the
- tunable with respect to AT_SECURE
- programs. See description of
- tunable_seclevel_t to see a
- description of the values.
-
- Note that even if the tunable is
- read, it may not get used by the
- target module if the value is
- considered unsafe. */
/* Compatibility elements. */
const char env_alias[TUNABLE_ALIAS_MAX]; /* The compatibility environment
variable name. */
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 24252af22c..a83bd2b8bc 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -154,50 +154,51 @@ __tunable_set_val (tunable_id_t id, tunable_val_t *valp, tunable_num_t *minp,
do_tunable_update_val (cur, valp, minp, maxp);
}
-/* Parse the tunable string TUNESTR and adjust it to drop any tunables that may
- be unsafe for AT_SECURE processes so that it can be used as the new
- environment variable value for GLIBC_TUNABLES. VALSTRING is the original
- environment variable string which we use to make NULL terminated values so
- that we don't have to allocate memory again for it. */
+/* Parse the tunable string VALSTRING. VALSTRING is a duplicated values,
+ where delimiters ':' are replaced with '\0', so string tunables are null
+ terminated. */
static void
-parse_tunables (char *tunestr, char *valstring)
+parse_tunables (char *valstring)
{
- if (tunestr == NULL || *tunestr == '\0')
+ if (valstring == NULL || *valstring == '\0')
return;
- char *p = tunestr;
- size_t off = 0;
+ char *p = valstring;
+ bool done = false;
- while (true)
+ while (!done)
{
char *name = p;
- size_t len = 0;
/* First, find where the name ends. */
- while (p[len] != '=' && p[len] != ':' && p[len] != '\0')
- len++;
+ while (*p != '=' && *p != ':' && *p != '\0')
+ p++;
/* If we reach the end of the string before getting a valid name-value
pair, bail out. */
- if (p[len] == '\0')
+ if (*p == '\0')
break;
/* We did not find a valid name-value pair before encountering the
colon. */
- if (p[len]== ':')
+ if (*p == ':')
{
- p += len + 1;
+ p++;
continue;
}
- p += len + 1;
+ /* Skip the ':' or '='. */
+ p++;
- /* Take the value from the valstring since we need to NULL terminate it. */
- char *value = &valstring[p - tunestr];
- len = 0;
+ const char *value = p;
- while (p[len] != ':' && p[len] != '\0')
- len++;
+ while (*p != ':' && *p != '\0')
+ p++;
+
+ if (*p == '\0')
+ done = true;
+ else
+ *p++ = '\0';
/* Add the tunable if it exists. */
for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
@@ -206,50 +207,11 @@ parse_tunables (char *tunestr, char *valstring)
if (tunable_is_name (cur->name, name))
{
- /* If we are in a secure context (AT_SECURE) then ignore the
- tunable unless it is explicitly marked as secure. Tunable
- values take precedence over their envvar aliases. We write
- the tunables that are not SXID_ERASE back to TUNESTR, thus
- dropping all SXID_ERASE tunables and any invalid or
- unrecognized tunables. */
- if (__libc_enable_secure)
- {
- if (cur->security_level != TUNABLE_SECLEVEL_SXID_ERASE)
- {
- if (off > 0)
- tunestr[off++] = ':';
-
- const char *n = cur->name;
-
- while (*n != '\0')
- tunestr[off++] = *n++;
-
- tunestr[off++] = '=';
-
- for (size_t j = 0; j < len; j++)
- tunestr[off++] = value[j];
- }
-
- if (cur->security_level != TUNABLE_SECLEVEL_NONE)
- break;
- }
-
- value[len] = '\0';
tunable_initialize (cur, value);
break;
}
}
-
- /* We reached the end while processing the tunable string. */
- if (p[len] == '\0')
- break;
-
- p += len + 1;
}
-
- /* Terminate tunestr before we leave. */
- if (__libc_enable_secure)
- tunestr[off] = '\0';
}
/* Initialize the tunables list from the environment. For now we only use the
@@ -263,16 +225,16 @@ __tunables_init (char **envp)
size_t len = 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,
&prev_envp)) != NULL)
{
if (tunable_is_name ("GLIBC_TUNABLES", envname))
{
- char *new_env = tunables_strdup (envname);
- if (new_env != NULL)
- parse_tunables (new_env + len + 1, envval);
- /* Put in the updated envval. */
- *prev_envp = new_env;
+ parse_tunables (tunables_strdup (envval));
continue;
}
@@ -290,39 +252,6 @@ __tunables_init (char **envp)
/* We have a match. Initialize and move on to the next line. */
if (tunable_is_name (name, envname))
{
- /* For AT_SECURE binaries, we need to check the security settings of
- the tunable and decide whether we read the value and also whether
- we erase the value so that child processes don't inherit them in
- the environment. */
- if (__libc_enable_secure)
- {
- if (cur->security_level == TUNABLE_SECLEVEL_SXID_ERASE)
- {
- /* Erase the environment variable. */
- char **ep = prev_envp;
-
- while (*ep != NULL)
- {
- if (tunable_is_name (name, *ep))
- {
- char **dp = ep;
-
- do
- dp[0] = dp[1];
- while (*dp++);
- }
- else
- ++ep;
- }
- /* Reset the iterator so that we read the environment again
- from the point we erased. */
- envp = prev_envp;
- }
-
- if (cur->security_level != TUNABLE_SECLEVEL_NONE)
- continue;
- }
-
tunable_initialize (cur, envval);
break;
}
diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
index 695ba7192e..471895af7b 100644
--- a/elf/dl-tunables.list
+++ b/elf/dl-tunables.list
@@ -21,14 +21,6 @@
# minval: Optional minimum acceptable value
# maxval: Optional maximum acceptable value
# env_alias: An alias environment variable
-# security_level: Specify security level of the tunable for AT_SECURE binaries.
-# Valid values are:
-#
-# SXID_ERASE: (default) Do not read and do not pass on to
-# child processes.
-# SXID_IGNORE: Do not read, but retain for non-AT_SECURE
-# subprocesses.
-# NONE: Read all the time.
glibc {
malloc {
@@ -158,7 +150,6 @@ glibc {
type: INT_32
minval: 0
maxval: 255
- security_level: SXID_IGNORE
}
}
diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c
index f0b92c97e7..3232f6b4c1 100644
--- a/elf/tst-env-setuid-tunables.c
+++ b/elf/tst-env-setuid-tunables.c
@@ -15,14 +15,10 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-/* Verify that tunables correctly filter out unsafe tunables like
- glibc.malloc.check and glibc.malloc.mmap_threshold but also retain
- glibc.malloc.mmap_threshold in an unprivileged child. */
-
-#define _LIBC 1
-#include "config.h"
-#undef _LIBC
+/* Verify that GLIBC_TUNABLES is kept unchanged but no tunable is actually
+ enabled for AT_SECURE processes. */
+#include <dl-tunables.h>
#include <errno.h>
#include <fcntl.h>
#include <stdlib.h>
@@ -40,7 +36,7 @@
#include <support/test-driver.h>
#include <support/capture_subprocess.h>
-const char *teststrings[] =
+static const char *teststrings[] =
{
"glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
"glibc.malloc.check=2:glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
@@ -60,45 +56,42 @@ const char *teststrings[] =
"glibc.not_valid.check=2",
};
-const char *resultstrings[] =
-{
- "glibc.malloc.mmap_threshold=4096",
- "glibc.malloc.mmap_threshold=4096",
- "glibc.malloc.mmap_threshold=4096",
- "glibc.malloc.perturb=0x800",
- "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
- "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
- "glibc.malloc.mmap_threshold=4096",
- "glibc.malloc.mmap_threshold=4096",
- "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096",
- "",
- "",
- "",
- "",
- "",
- "",
- "",
-};
-
static int
test_child (int off)
{
const char *val = getenv ("GLIBC_TUNABLES");
+ int ret = 1;
printf (" [%d] GLIBC_TUNABLES is %s\n", off, val);
fflush (stdout);
- if (val != NULL && strcmp (val, resultstrings[off]) == 0)
- return 0;
-
- if (val != NULL)
- printf (" [%d] Unexpected GLIBC_TUNABLES VALUE %s, expected %s\n",
- off, val, resultstrings[off]);
- else
+ if (val == NULL)
printf (" [%d] GLIBC_TUNABLES environment variable absent\n", off);
+ else
+ {
+ if (strcmp (val, teststrings[off]) != 0)
+ printf (" [%d] Unexpected GLIBC_TUNABLES VALUE %s\n", off, val);
+ else
+ ret = 0;
+ }
+ fflush (stdout);
+ int32_t check = TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL);
+ size_t mmap_threshold = TUNABLE_GET_FULL (glibc, malloc, mmap_threshold,
+ size_t, NULL);
+ int32_t perturb = TUNABLE_GET_FULL (glibc, malloc, perturb, int32_t, NULL);
+
+ printf (" [%d] glibc.malloc.check=%d\n", off, check);
+ fflush (stdout);
+ printf (" [%d] glibc.malloc.mmap_threshold=%zu\n", off, mmap_threshold);
fflush (stdout);
+ printf (" [%d] glibc.malloc.perturb=%d\n", off, perturb);
+ fflush (stdout);
+
+ ret |= check != 0;
+ ret |= mmap_threshold != 0;
+ ret |= perturb != 0;
- return 1;
+ return ret;
}
static int
diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
new file mode 100644
index 0000000000..d6d3bea2f7
--- /dev/null
+++ b/elf/tst-tunables.c
@@ -0,0 +1,243 @@
+/* Check GLIBC_TUNABLES parsing.
+ 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/>. */
+
+#include <array_length.h>
+#include <getopt.h>
+#include <intprops.h>
+#include <stdint.h>
+#include <dl-tunables.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+
+static int restart;
+#define CMDLINE_OPTIONS \
+ { "restart", no_argument, &restart, 1 },
+
+static const struct test_t
+{
+ const char *env;
+ int32_t expected_malloc_check;
+ size_t expected_mmap_threshold;
+ int32_t expected_perturb;
+} tests[] =
+{
+ /* Expected tunable format. */
+ {
+ "glibc.malloc.check=2",
+ 2,
+ 0,
+ 0,
+ },
+ {
+ "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
+ 2,
+ 4096,
+ 0,
+ },
+ /* Empty tunable are ignored. */
+ {
+ "glibc.malloc.check=2::glibc.malloc.mmap_threshold=4096",
+ 2,
+ 4096,
+ 0,
+ },
+ /* As well empty values. */
+ {
+ "glibc.malloc.check=:glibc.malloc.mmap_threshold=4096",
+ 0,
+ 4096,
+ 0,
+ },
+ /* Tunable are processed from left to right, so last one is the one set. */
+ {
+ "glibc.malloc.check=1:glibc.malloc.check=2",
+ 2,
+ 0,
+ 0,
+ },
+ {
+ "glibc.malloc.check=2:glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
+ 2,
+ 4096,
+ 0,
+ },
+ {
+ "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096:glibc.malloc.check=2",
+ 2,
+ 4096,
+ 0,
+ },
+ /* 0x800 is larger than tunable maxval (0xff), so the tunable is unchanged. */
+ {
+ "glibc.malloc.perturb=0x800",
+ 0,
+ 0,
+ 0,
+ },
+ {
+ "glibc.malloc.perturb=0x55",
+ 0,
+ 0,
+ 0x55,
+ },
+ /* Out of range values are just ignored. */
+ {
+ "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
+ 0,
+ 4096,
+ 0,
+ },
+ /* Invalid keys are ignored. */
+ {
+ ":glibc.malloc.garbage=2:glibc.malloc.check=1",
+ 1,
+ 0,
+ 0,
+ },
+ {
+ "glibc.malloc.perturb=0x800:not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",
+ 0,
+ 4096,
+ 0,
+ },
+ {
+ "glibc.not_valid.check=2:glibc.malloc.mmap_threshold=4096",
+ 0,
+ 4096,
+ 0,
+ },
+ {
+ "not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",
+ 0,
+ 4096,
+ 0,
+ },
+ /* Invalid subkeys are ignored. */
+ {
+ "glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096:glibc.malloc.check=2",
+ 2,
+ 0,
+ 0,
+ },
+ {
+ "glibc.malloc.check=4:glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096",
+ 0,
+ 0,
+ 0,
+ },
+ {
+ "not_valid.malloc.check=2",
+ 0,
+ 0,
+ 0,
+ },
+ {
+ "glibc.not_valid.check=2",
+ 0,
+ 0,
+ 0,
+ },
+ /* 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.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096",
+ 0,
+ 0,
+ 0,
+ },
+ /* The ill-formatted tunable is also skipped. */
+ {
+ "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096:glibc.malloc.check=2",
+ 2,
+ 0,
+ 0,
+ },
+ /* For an integer tunable, parse will stop on non number character. */
+ {
+ "glibc.malloc.check=2=2",
+ 2,
+ 0,
+ 0,
+ },
+ {
+ "glibc.malloc.check=2=2:glibc.malloc.mmap_threshold=4096",
+ 2,
+ 4096,
+ 0,
+ }
+};
+
+static int
+handle_restart (int i)
+{
+ TEST_COMPARE (tests[i].expected_malloc_check,
+ TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL));
+ TEST_COMPARE (tests[i].expected_mmap_threshold,
+ TUNABLE_GET_FULL (glibc, malloc, mmap_threshold, size_t, NULL));
+ TEST_COMPARE (tests[i].expected_perturb,
+ TUNABLE_GET_FULL (glibc, malloc, perturb, int32_t, NULL));
+ return 0;
+}
+
+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)
+ return 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);
+ setenv ("GLIBC_TUNABLES", tests[i].env, 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);
+ }
+
+ return 0;
+}
+
+#define TEST_FUNCTION_ARGV do_test
+#include <support/test-driver.c>
diff --git a/manual/README.tunables b/manual/README.tunables
index 605ddd78cd..72ae00dc02 100644
--- a/manual/README.tunables
+++ b/manual/README.tunables
@@ -59,15 +59,6 @@ The list of allowed attributes are:
- env_alias: An alias environment variable
-- security_level: Specify security level of the tunable for AT_SECURE
- binaries. Valid values are:
-
- SXID_ERASE: (default) Do not read and do not pass on to
- child processes.
- SXID_IGNORE: Do not read, but retain for non-AT_SECURE
- child processes.
- NONE: Read all the time.
-
2. Use TUNABLE_GET/TUNABLE_SET/TUNABLE_SET_WITH_BOUNDS to get and set tunables.
3. OPTIONAL: If tunables in a namespace are being used multiple times within a
diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
index d6de100df0..1e9d6b534e 100644
--- a/scripts/gen-tunables.awk
+++ b/scripts/gen-tunables.awk
@@ -61,9 +61,6 @@ $1 == "}" {
if (!env_alias[top_ns,ns,tunable]) {
env_alias[top_ns,ns,tunable] = "{0}"
}
- if (!security_level[top_ns,ns,tunable]) {
- security_level[top_ns,ns,tunable] = "SXID_ERASE"
- }
len = length(top_ns"."ns"."tunable)
if (len > max_name_len)
max_name_len = len
@@ -118,17 +115,6 @@ $1 == "}" {
if (len > max_alias_len)
max_alias_len = len
}
- else if (attr == "security_level") {
- if (val == "SXID_ERASE" || val == "SXID_IGNORE" || val == "NONE") {
- security_level[top_ns,ns,tunable] = val
- }
- else {
- printf("Line %d: Invalid value (%s) for security_level: %s, ", NR, val,
- $0)
- print("Allowed values are 'SXID_ERASE', 'SXID_IGNORE', or 'NONE'")
- exit 1
- }
- }
else if (attr == "default") {
if (types[top_ns,ns,tunable] == "STRING") {
default_val[top_ns,ns,tunable] = sprintf(".strval = \"%s\"", val);
@@ -177,9 +163,9 @@ END {
n = indices[2];
m = indices[3];
printf (" {TUNABLE_NAME_S(%s, %s, %s)", t, n, m)
- printf (", {TUNABLE_TYPE_%s, %s, %s}, {%s}, false, TUNABLE_SECLEVEL_%s, %s},\n",
+ printf (", {TUNABLE_TYPE_%s, %s, %s}, {%s}, false, %s},\n",
types[t,n,m], minvals[t,n,m], maxvals[t,n,m],
- default_val[t,n,m], security_level[t,n,m], env_alias[t,n,m]);
+ default_val[t,n,m], env_alias[t,n,m]);
}
print "};"
print "#endif"
diff --git a/sysdeps/x86_64/64/dl-tunables.list b/sysdeps/x86_64/64/dl-tunables.list
index 0aab52e662..54a216a461 100644
--- a/sysdeps/x86_64/64/dl-tunables.list
+++ b/sysdeps/x86_64/64/dl-tunables.list
@@ -23,7 +23,6 @@ glibc {
minval: 0
maxval: 1
env_alias: LD_PREFER_MAP_32BIT_EXEC
- security_level: SXID_IGNORE
}
}
}
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 03/11] elf: Add all malloc tunable to unsecvars
2023-10-10 18:01 [PATCH 00/11] Improve tunable handling Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 01/11] elf: Remove /etc/suid-debug support Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 02/11] elf: Ignore GLIBC_TUNABLES for setuid/setgid binaries Adhemerval Zanella
@ 2023-10-10 18:01 ` Adhemerval Zanella
2023-10-12 8:47 ` Florian Weimer
2023-10-10 18:01 ` [PATCH 04/11] elf: Add GLIBC_TUNABLES " Adhemerval Zanella
` (7 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2023-10-10 18:01 UTC (permalink / raw)
To: libc-alpha, Siddhesh Poyarekar
Some environment variables allow alteration of allocator behavior
across setuid boundaries, where a setuid program may ignore the
tunable, but its non-setuid child can read it and adjust the memory
allocator behavior accordingly.
Most library behavior tunings is limited to the current process and does
not bleed in scope; so it is unclear how pratical this misfeature is.
If behavior change across privilege boundaries is desirable, it would be
better done with a wrapper program around the non-setuid child that sets
these envvars, instead of using the setuid process as the messenger.
The patch as fixes tst-env-setuid, where it fail if any unsecvars is
set.
Co-authored-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Checked on x86_64-linux-gnu.
---
elf/tst-env-setuid.c | 87 +++++++++++++------------------------
sysdeps/generic/unsecvars.h | 7 +++
2 files changed, 36 insertions(+), 58 deletions(-)
diff --git a/elf/tst-env-setuid.c b/elf/tst-env-setuid.c
index 032ab44be2..b9f4b3244d 100644
--- a/elf/tst-env-setuid.c
+++ b/elf/tst-env-setuid.c
@@ -15,19 +15,14 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-/* Verify that tunables correctly filter out unsafe environment variables like
- MALLOC_CHECK_ and MALLOC_MMAP_THRESHOLD_ but also retain
- MALLOC_MMAP_THRESHOLD_ in an unprivileged child. */
+/* Verify that correctly filter out unsafe environment variables defined
+ by unsecvars.h. */
-#include <errno.h>
-#include <fcntl.h>
-#include <stdlib.h>
-#include <stdint.h>
#include <stdio.h>
+#include <stdlib.h>
#include <string.h>
-#include <sys/stat.h>
-#include <sys/wait.h>
#include <unistd.h>
+#include <unsecvars.h>
#include <support/check.h>
#include <support/support.h>
@@ -36,57 +31,22 @@
static char SETGID_CHILD[] = "setgid-child";
-#ifndef test_child
static int
test_child (void)
{
- if (getenv ("MALLOC_CHECK_") != NULL)
- {
- printf ("MALLOC_CHECK_ is still set\n");
- return 1;
- }
-
- if (getenv ("MALLOC_MMAP_THRESHOLD_") == NULL)
- {
- printf ("MALLOC_MMAP_THRESHOLD_ lost\n");
- return 1;
- }
+ int ret = 0;
- if (getenv ("LD_HWCAP_MASK") != NULL)
+ const char *nextp = UNSECURE_ENVVARS;
+ do
{
- printf ("LD_HWCAP_MASK still set\n");
- return 1;
+ const char *env = getenv (nextp);
+ ret |= env != NULL;
+ nextp = strchr (nextp, '\0') + 1;
}
+ while (*nextp != '\0');
return 0;
}
-#endif
-
-#ifndef test_parent
-static int
-test_parent (void)
-{
- if (getenv ("MALLOC_CHECK_") == NULL)
- {
- printf ("MALLOC_CHECK_ lost\n");
- return 1;
- }
-
- if (getenv ("MALLOC_MMAP_THRESHOLD_") == NULL)
- {
- printf ("MALLOC_MMAP_THRESHOLD_ lost\n");
- return 1;
- }
-
- if (getenv ("LD_HWCAP_MASK") == NULL)
- {
- printf ("LD_HWCAP_MASK lost\n");
- return 1;
- }
-
- return 0;
-}
-#endif
static int
do_test (int argc, char **argv)
@@ -104,20 +64,31 @@ do_test (int argc, char **argv)
if (ret != 0)
exit (1);
- exit (EXIT_SUCCESS);
+ /* Special return code to make sure that the child executed all the way
+ through. */
+ exit (42);
}
else
{
- if (test_parent () != 0)
- exit (1);
+ const char *nextp = UNSECURE_ENVVARS;
+ do
+ {
+ setenv (nextp, "some-value", 1);
+ nextp = strchr (nextp, '\0') + 1;
+ }
+ while (*nextp != '\0');
int status = support_capture_subprogram_self_sgid (SETGID_CHILD);
if (WEXITSTATUS (status) == EXIT_UNSUPPORTED)
- return EXIT_UNSUPPORTED;
-
- if (!WIFEXITED (status))
- FAIL_EXIT1 ("Unexpected exit status %d from child process\n", status);
+ exit (EXIT_UNSUPPORTED);
+
+ if (WEXITSTATUS (status) != 42)
+ {
+ printf (" child failed with status %d\n",
+ WEXITSTATUS (status));
+ support_record_failure ();
+ }
return 0;
}
diff --git a/sysdeps/generic/unsecvars.h b/sysdeps/generic/unsecvars.h
index 8278c50a84..ca70e2e989 100644
--- a/sysdeps/generic/unsecvars.h
+++ b/sysdeps/generic/unsecvars.h
@@ -17,7 +17,14 @@
"LD_SHOW_AUXV\0" \
"LOCALDOMAIN\0" \
"LOCPATH\0" \
+ "MALLOC_ARENA_MAX\0" \
+ "MALLOC_ARENA_TEST\0" \
+ "MALLOC_MMAP_MAX_\0" \
+ "MALLOC_MMAP_THRESHOLD_\0" \
+ "MALLOC_PERTURB_\0" \
+ "MALLOC_TOP_PAD_\0" \
"MALLOC_TRACE\0" \
+ "MALLOC_TRIM_THRESHOLD_\0" \
"NIS_PATH\0" \
"NLSPATH\0" \
"RESOLV_HOST_CONF\0" \
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 04/11] elf: Add GLIBC_TUNABLES to unsecvars
2023-10-10 18:01 [PATCH 00/11] Improve tunable handling Adhemerval Zanella
` (2 preceding siblings ...)
2023-10-10 18:01 ` [PATCH 03/11] elf: Add all malloc tunable to unsecvars Adhemerval Zanella
@ 2023-10-10 18:01 ` Adhemerval Zanella
2023-10-12 8:46 ` Florian Weimer
2023-10-10 18:01 ` [PATCH 05/11] elf: Do not process invalid tunable format Adhemerval Zanella
` (6 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2023-10-10 18:01 UTC (permalink / raw)
To: libc-alpha, Siddhesh Poyarekar
setuid/setgid process now ignores any glibc tunables, and filters out
all environment variables that might changes its behavior. This patch
also adds GLIBC_TUNABLES, so any spawned process by setuid/setgid
processes should set tunable explicitly.
Checked on x86_64-linux-gnu.
---
elf/tst-env-setuid-tunables.c | 11 +++--------
sysdeps/generic/unsecvars.h | 1 +
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c
index 3232f6b4c1..39b3648aa6 100644
--- a/elf/tst-env-setuid-tunables.c
+++ b/elf/tst-env-setuid-tunables.c
@@ -64,15 +64,10 @@ test_child (int off)
printf (" [%d] GLIBC_TUNABLES is %s\n", off, val);
fflush (stdout);
- if (val == NULL)
- printf (" [%d] GLIBC_TUNABLES environment variable absent\n", off);
+ if (val != NULL)
+ printf (" [%d] Unexpected GLIBC_TUNABLES VALUE %s\n", off, val);
else
- {
- if (strcmp (val, teststrings[off]) != 0)
- printf (" [%d] Unexpected GLIBC_TUNABLES VALUE %s\n", off, val);
- else
- ret = 0;
- }
+ ret = 0;
fflush (stdout);
int32_t check = TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL);
diff --git a/sysdeps/generic/unsecvars.h b/sysdeps/generic/unsecvars.h
index ca70e2e989..f7ebed60e5 100644
--- a/sysdeps/generic/unsecvars.h
+++ b/sysdeps/generic/unsecvars.h
@@ -4,6 +4,7 @@
#define UNSECURE_ENVVARS \
"GCONV_PATH\0" \
"GETCONF_DIR\0" \
+ "GLIBC_TUNABLES\0" \
"HOSTALIASES\0" \
"LD_AUDIT\0" \
"LD_DEBUG\0" \
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 05/11] elf: Do not process invalid tunable format
2023-10-10 18:01 [PATCH 00/11] Improve tunable handling Adhemerval Zanella
` (3 preceding siblings ...)
2023-10-10 18:01 ` [PATCH 04/11] elf: Add GLIBC_TUNABLES " Adhemerval Zanella
@ 2023-10-10 18:01 ` Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 06/11] elf: Do not parse ill-formatted strings Adhemerval Zanella
` (5 subsequent siblings)
10 siblings, 0 replies; 27+ messages in thread
From: Adhemerval Zanella @ 2023-10-10 18:01 UTC (permalink / raw)
To: libc-alpha, Siddhesh Poyarekar
Tunable definitions with more than one '=' on are parsed and enabled,
and any subsequent '=' are ignored. It means that tunables in the form
'tunable=tunable=value' or 'tunable=value=value' are handled as
'tunable=value'. These inputs are likely user input errors, which
should not be accepted.
Checked on x86_64-linux-gnu.
---
elf/dl-tunables.c | 6 ++++--
elf/tst-tunables.c | 22 +++++++++++++++++-----
2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index a83bd2b8bc..59bee61124 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -192,10 +192,12 @@ parse_tunables (char *valstring)
const char *value = p;
- while (*p != ':' && *p != '\0')
+ while (*p != '=' && *p != ':' && *p != '\0')
p++;
- if (*p == '\0')
+ if (*p == '=')
+ break;
+ else if (*p == '\0')
done = true;
else
*p++ = '\0';
diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
index d6d3bea2f7..c8546b75c7 100644
--- a/elf/tst-tunables.c
+++ b/elf/tst-tunables.c
@@ -160,24 +160,36 @@ static const struct test_t
0,
0,
},
- /* The ill-formatted tunable is also skipped. */
+ /* If there is a ill-formatted key=value, everything after is also ignored. */
{
"glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096:glibc.malloc.check=2",
- 2,
+ 0,
0,
0,
},
- /* For an integer tunable, parse will stop on non number character. */
{
"glibc.malloc.check=2=2",
- 2,
+ 0,
0,
0,
},
{
"glibc.malloc.check=2=2:glibc.malloc.mmap_threshold=4096",
+ 0,
+ 0,
+ 0,
+ },
+ {
+ "glibc.malloc.check=2=2:glibc.malloc.check=2",
+ 0,
+ 0,
+ 0,
+ },
+ /* Valid tunables set before ill-formatted ones are set. */
+ {
+ "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096=4096",
2,
- 4096,
+ 0,
0,
}
};
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 06/11] elf: Do not parse ill-formatted strings
2023-10-10 18:01 [PATCH 00/11] Improve tunable handling Adhemerval Zanella
` (4 preceding siblings ...)
2023-10-10 18:01 ` [PATCH 05/11] elf: Do not process invalid tunable format Adhemerval Zanella
@ 2023-10-10 18:01 ` Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 07/11] elf: Fix _dl_debug_vdprintf to work before self-relocation Adhemerval Zanella
` (4 subsequent siblings)
10 siblings, 0 replies; 27+ messages in thread
From: Adhemerval Zanella @ 2023-10-10 18:01 UTC (permalink / raw)
To: libc-alpha, Siddhesh Poyarekar
Instead of ignoring ill-formatted tunable strings, first, check all the
tunable definitions are correct and then set each tunable value. It
means that partially invalid strings, like "key1=value1:key2=key2=value'
or 'key1=value':key2=value2=value2' do not enable 'key1=value1'. It
avoids possible user-defined errors in tunable definitions.
Checked on x86_64-linux-gnu.
---
elf/dl-tunables.c | 50 +++++++++++++++++++++++++++++++++++-----------
elf/tst-tunables.c | 13 ++++++++----
2 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 59bee61124..5d4b8c5bc0 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -154,17 +154,29 @@ __tunable_set_val (tunable_id_t id, tunable_val_t *valp, tunable_num_t *minp,
do_tunable_update_val (cur, valp, minp, maxp);
}
-/* Parse the tunable string VALSTRING. VALSTRING is a duplicated values,
- where delimiters ':' are replaced with '\0', so string tunables are null
- terminated. */
-static void
-parse_tunables (char *valstring)
+struct tunable_toset_t
+{
+ tunable_t *t;
+ const char *value;
+};
+
+enum { tunables_list_size = array_length (tunable_list) };
+
+/* Parse the tunable string VALSTRING and set TUNABLES with the found tunables
+ and their respectibles values. VALSTRING is a duplicated values, where
+ delimiters ':' are replaced with '\0', so string tunables are null
+ terminated.
+ Return the number of tunables found (including 0 if the string is empty)
+ or -1 if for a ill-formatted definition. */
+static int
+parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
{
if (valstring == NULL || *valstring == '\0')
- return;
+ return 0;
char *p = valstring;
bool done = false;
+ int ntunables = 0;
while (!done)
{
@@ -177,7 +189,7 @@ parse_tunables (char *valstring)
/* If we reach the end of the string before getting a valid name-value
pair, bail out. */
if (*p == '\0')
- break;
+ return -1;
/* We did not find a valid name-value pair before encountering the
colon. */
@@ -190,30 +202,44 @@ parse_tunables (char *valstring)
/* Skip the ':' or '='. */
p++;
- const char *value = p;
+ char *value = p;
while (*p != '=' && *p != ':' && *p != '\0')
p++;
if (*p == '=')
- break;
+ return -1;
else if (*p == '\0')
done = true;
else
*p++ = '\0';
/* Add the tunable if it exists. */
- for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
+ for (size_t i = 0; i < tunables_list_size; i++)
{
tunable_t *cur = &tunable_list[i];
if (tunable_is_name (cur->name, name))
{
- tunable_initialize (cur, value);
+ tunables[ntunables++] = (struct tunable_toset_t) { cur, value };
break;
}
}
}
+
+ return ntunables;
+}
+
+static void
+parse_tunables (char *valstring)
+{
+ struct tunable_toset_t tunables[tunables_list_size];
+ int ntunables = parse_tunables_string (valstring, tunables);
+ if (ntunables == -1)
+ return;
+
+ for (int i = 0; i < ntunables; i++)
+ tunable_initialize (tunables[i].t, tunables[i].value);
}
/* Initialize the tunables list from the environment. For now we only use the
@@ -240,7 +266,7 @@ __tunables_init (char **envp)
continue;
}
- for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++)
+ for (int i = 0; i < tunables_list_size; i++)
{
tunable_t *cur = &tunable_list[i];
diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
index c8546b75c7..5168331d15 100644
--- a/elf/tst-tunables.c
+++ b/elf/tst-tunables.c
@@ -160,7 +160,7 @@ static const struct test_t
0,
0,
},
- /* If there is a ill-formatted key=value, everything after is also ignored. */
+ /* Ill-formatted tunables string is not parsed. */
{
"glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096:glibc.malloc.check=2",
0,
@@ -185,13 +185,18 @@ static const struct test_t
0,
0,
},
- /* Valid tunables set before ill-formatted ones are set. */
{
"glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096=4096",
- 2,
0,
0,
- }
+ 0,
+ },
+ {
+ "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096=4096",
+ 0,
+ 0,
+ 0,
+ },
};
static int
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 07/11] elf: Fix _dl_debug_vdprintf to work before self-relocation
2023-10-10 18:01 [PATCH 00/11] Improve tunable handling Adhemerval Zanella
` (5 preceding siblings ...)
2023-10-10 18:01 ` [PATCH 06/11] elf: Do not parse ill-formatted strings Adhemerval Zanella
@ 2023-10-10 18:01 ` Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 08/11] elf: Emit warning if tunable is ill-formatted Adhemerval Zanella
` (3 subsequent siblings)
10 siblings, 0 replies; 27+ messages in thread
From: Adhemerval Zanella @ 2023-10-10 18:01 UTC (permalink / raw)
To: libc-alpha, Siddhesh Poyarekar
The strlen might trigger and invalid GOT entry if it used before
the process is self-relocated (for instance on dl-tunables if any
error occurs).
Checked on x86_64-linux-gnu.
---
elf/dl-printf.c | 16 ++++++++++++++--
stdio-common/Makefile | 5 +++++
stdio-common/_itoa.c | 5 +++++
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/elf/dl-printf.c b/elf/dl-printf.c
index 6efb4c019a..5e93208535 100644
--- a/elf/dl-printf.c
+++ b/elf/dl-printf.c
@@ -17,6 +17,10 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
+#include <string.h>
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
#include <_itoa.h>
#include <assert.h>
#include <dl-writev.h>
@@ -25,11 +29,19 @@
#include <stdarg.h>
#include <stdint.h>
#include <stdlib.h>
-#include <string.h>
#include <sys/uio.h>
#include <unistd.h>
#include <intprops.h>
+/* The function might be called before the process is self-relocated. */
+static size_t
+_dl_debug_strlen (const char *s)
+{
+ const char *p = s;
+ for (; *s != '\0'; s++);
+ return s - p;
+}
+
/* Bare-bones printf implementation. This function only knows about
the formats and flags needed and can handle only up to 64 stripes in
the output. */
@@ -193,7 +205,7 @@ _dl_debug_vdprintf (int fd, int tag_p, const char *fmt, va_list arg)
case 's':
/* Get the string argument. */
iov[niov].iov_base = va_arg (arg, char *);
- iov[niov].iov_len = strlen (iov[niov].iov_base);
+ iov[niov].iov_len = _dl_debug_strlen (iov[niov].iov_base);
if (prec != -1)
iov[niov].iov_len = MIN ((size_t) prec, iov[niov].iov_len);
++niov;
diff --git a/stdio-common/Makefile b/stdio-common/Makefile
index bacb795fed..e88a9cea29 100644
--- a/stdio-common/Makefile
+++ b/stdio-common/Makefile
@@ -460,6 +460,11 @@ CFLAGS-isoc23_scanf.c += -fexceptions
CFLAGS-dprintf.c += $(config-cflags-wno-ignored-attributes)
+# Called during static library initialization, so turn stack-protection
+# off for non-shared builds.
+CFLAGS-_itoa.o = $(no-stack-protector)
+CFLAGS-_itoa.op = $(no-stack-protector)
+
# scanf18.c and scanf19.c test a deprecated extension which is no
# longer visible under most conformance levels; see the source files
# for more detail.
diff --git a/stdio-common/_itoa.c b/stdio-common/_itoa.c
index 3037b0f529..48f2903ecb 100644
--- a/stdio-common/_itoa.c
+++ b/stdio-common/_itoa.c
@@ -16,6 +16,11 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
+/* Mark symbols hidden in static PIE for early self relocation to work.
+ Note: string.h may have ifuncs which cannot be hidden on i686. */
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
#include <gmp-mparam.h>
#include <gmp.h>
#include <limits.h>
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 08/11] elf: Emit warning if tunable is ill-formatted
2023-10-10 18:01 [PATCH 00/11] Improve tunable handling Adhemerval Zanella
` (6 preceding siblings ...)
2023-10-10 18:01 ` [PATCH 07/11] elf: Fix _dl_debug_vdprintf to work before self-relocation Adhemerval Zanella
@ 2023-10-10 18:01 ` Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 09/11] x86: Use dl-symbol-redir-ifunc.h on cpu-tunables Adhemerval Zanella
` (2 subsequent siblings)
10 siblings, 0 replies; 27+ messages in thread
From: Adhemerval Zanella @ 2023-10-10 18:01 UTC (permalink / raw)
To: libc-alpha, Siddhesh Poyarekar
So caller knows that the tunable will be ignored.
Checked on x86_64-linux-gnu.
---
elf/dl-tunables.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 5d4b8c5bc0..b39c14694c 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -236,7 +236,11 @@ parse_tunables (char *valstring)
struct tunable_toset_t tunables[tunables_list_size];
int ntunables = parse_tunables_string (valstring, tunables);
if (ntunables == -1)
- return;
+ {
+ _dl_error_printf (
+ "ERROR: ld.so: invalid GLIBC_TUNABLES `%s': ignored.\n", valstring);
+ return;
+ }
for (int i = 0; i < ntunables; i++)
tunable_initialize (tunables[i].t, tunables[i].value);
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 09/11] x86: Use dl-symbol-redir-ifunc.h on cpu-tunables
2023-10-10 18:01 [PATCH 00/11] Improve tunable handling Adhemerval Zanella
` (7 preceding siblings ...)
2023-10-10 18:01 ` [PATCH 08/11] elf: Emit warning if tunable is ill-formatted Adhemerval Zanella
@ 2023-10-10 18:01 ` Adhemerval Zanella
2023-10-12 18:11 ` Noah Goldstein
2023-10-10 18:01 ` [PATCH 10/11] s390: " Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 11/11] elf: Do not duplicate the GLIBC_TUNABLES string Adhemerval Zanella
10 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2023-10-10 18:01 UTC (permalink / raw)
To: libc-alpha, Siddhesh Poyarekar
The dl-symbol-redir-ifunc.h redirects compiler-generated libcalls to
arch-specific memory implementations to avoid ifun calls where it is not
yet possible. The memcmp-isa-default-impl.h aims to fix the same issue
by calling the specific memset implementation directly.
Using the memcmp symbol directly allows the compile to inline the memset
calls (especially because _dl_tunable_set_hwcaps uses constants values),
generating better code.
For i386, _dl_writev with PIE requires to use the old 'int $0x80'
syscall mode because the calling the TLS register (gs) is not yet
initialized.
Checked on x86_64-linux-gnu.
---
.../i686/multiarch/dl-symbol-redir-ifunc.h | 5 +++
.../sysv/linux/i386/dl-writev.h} | 18 ++++-----
sysdeps/x86/cpu-tunables.c | 39 ++++++-------------
.../x86_64/multiarch/dl-symbol-redir-ifunc.h | 15 +++++++
4 files changed, 39 insertions(+), 38 deletions(-)
rename sysdeps/{x86_64/memcmp-isa-default-impl.h => unix/sysv/linux/i386/dl-writev.h} (62%)
diff --git a/sysdeps/i386/i686/multiarch/dl-symbol-redir-ifunc.h b/sysdeps/i386/i686/multiarch/dl-symbol-redir-ifunc.h
index dee69d19db..220c586bd2 100644
--- a/sysdeps/i386/i686/multiarch/dl-symbol-redir-ifunc.h
+++ b/sysdeps/i386/i686/multiarch/dl-symbol-redir-ifunc.h
@@ -19,6 +19,11 @@
#ifndef _DL_IFUNC_GENERIC_H
#define _DL_IFUNC_GENERIC_H
+#ifndef SHARED
+
asm ("memset = __memset_ia32");
+asm ("memcmp = __memcmp_ia32");
+
+#endif /* SHARED */
#endif
diff --git a/sysdeps/x86_64/memcmp-isa-default-impl.h b/sysdeps/unix/sysv/linux/i386/dl-writev.h
similarity index 62%
rename from sysdeps/x86_64/memcmp-isa-default-impl.h
rename to sysdeps/unix/sysv/linux/i386/dl-writev.h
index 0962e83c3d..624d0e46b0 100644
--- a/sysdeps/x86_64/memcmp-isa-default-impl.h
+++ b/sysdeps/unix/sysv/linux/i386/dl-writev.h
@@ -1,5 +1,5 @@
-/* Set default memcmp impl based on ISA level.
- Copyright (C) 2022-2023 Free Software Foundation, Inc.
+/* Message-writing for the dynamic linker. Linux/i386 version.
+ Copyright (C) 2013-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
@@ -16,13 +16,9 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-#include <isa-level.h>
-#if MINIMUM_X86_ISA_LEVEL == 1 || MINIMUM_X86_ISA_LEVEL == 2
-# define DEFAULT_MEMCMP __memcmp_sse2
-#elif MINIMUM_X86_ISA_LEVEL == 3
-# define DEFAULT_MEMCMP __memcmp_avx2_movbe
-#elif MINIMUM_X86_ISA_LEVEL == 4
-# define DEFAULT_MEMCMP __memcmp_evex_movbe
-#else
-# error "Unknown default memcmp implementation"
+#if BUILD_PIE_DEFAULT
+/* Can't use "call *%gs:SYSINFO_OFFSET" during startup in static PIE. */
+# define I386_USE_SYSENTER 0
#endif
+
+#include <sysdeps/unix/sysv/linux/dl-writev.h>
diff --git a/sysdeps/x86/cpu-tunables.c b/sysdeps/x86/cpu-tunables.c
index 0d4f328585..5697885226 100644
--- a/sysdeps/x86/cpu-tunables.c
+++ b/sysdeps/x86/cpu-tunables.c
@@ -24,24 +24,11 @@
#include <string.h>
#include <cpu-features.h>
#include <ldsodefs.h>
-
-/* We can't use IFUNC memcmp nor strlen in init_cpu_features from libc.a
- since IFUNC must be set up by init_cpu_features. */
-#if defined USE_MULTIARCH && !defined SHARED
-# ifdef __x86_64__
-/* DEFAULT_MEMCMP by sysdeps/x86_64/memcmp-isa-default-impl.h. */
-# include <sysdeps/x86_64/memcmp-isa-default-impl.h>
-# else
-# define DEFAULT_MEMCMP __memcmp_ia32
-# endif
-extern __typeof (memcmp) DEFAULT_MEMCMP;
-#else
-# define DEFAULT_MEMCMP memcmp
-#endif
+#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 (!DEFAULT_MEMCMP (f, #name, len)) \
+ if (memcmp (f, #name, len) == 0) \
{ \
CPU_FEATURE_UNSET (cpu_features, name) \
break; \
@@ -51,7 +38,7 @@ extern __typeof (memcmp) DEFAULT_MEMCMP;
which isn't available. */
#define CHECK_GLIBC_IFUNC_PREFERRED_OFF(f, cpu_features, name, len) \
_Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
- if (!DEFAULT_MEMCMP (f, #name, len)) \
+ if (memcmp (f, #name, len) == 0) \
{ \
cpu_features->preferred[index_arch_##name] \
&= ~bit_arch_##name; \
@@ -62,7 +49,7 @@ extern __typeof (memcmp) DEFAULT_MEMCMP;
#define CHECK_GLIBC_IFUNC_PREFERRED_BOTH(f, cpu_features, name, \
disable, len) \
_Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
- if (!DEFAULT_MEMCMP (f, #name, len)) \
+ if (memcmp (f, #name, len) == 0) \
{ \
if (disable) \
cpu_features->preferred[index_arch_##name] &= ~bit_arch_##name; \
@@ -76,7 +63,7 @@ extern __typeof (memcmp) DEFAULT_MEMCMP;
#define CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH(f, cpu_features, name, \
need, disable, len) \
_Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
- if (!DEFAULT_MEMCMP (f, #name, len)) \
+ if (memcmp (f, #name, len) == 0) \
{ \
if (disable) \
cpu_features->preferred[index_arch_##name] &= ~bit_arch_##name; \
@@ -177,7 +164,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
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 (!DEFAULT_MEMCMP (n, "XSAVEC", 6))
+ if (memcmp (n, "XSAVEC", 6) == 0)
{
/* Update xsave_state_size to XSAVE state size. */
cpu_features->xsave_state_size
@@ -290,12 +277,11 @@ attribute_hidden
void
TUNABLE_CALLBACK (set_x86_ibt) (tunable_val_t *valp)
{
- if (DEFAULT_MEMCMP (valp->strval, "on", sizeof ("on")) == 0)
+ if (memcmp (valp->strval, "on", sizeof ("on")) == 0)
GL(dl_x86_feature_control).ibt = cet_always_on;
- else if (DEFAULT_MEMCMP (valp->strval, "off", sizeof ("off")) == 0)
+ else if (memcmp (valp->strval, "off", sizeof ("off")) == 0)
GL(dl_x86_feature_control).ibt = cet_always_off;
- else if (DEFAULT_MEMCMP (valp->strval, "permissive",
- sizeof ("permissive")) == 0)
+ else if (memcmp (valp->strval, "permissive", sizeof ("permissive")) == 0)
GL(dl_x86_feature_control).ibt = cet_permissive;
}
@@ -303,12 +289,11 @@ attribute_hidden
void
TUNABLE_CALLBACK (set_x86_shstk) (tunable_val_t *valp)
{
- if (DEFAULT_MEMCMP (valp->strval, "on", sizeof ("on")) == 0)
+ if (memcmp (valp->strval, "on", sizeof ("on")) == 0)
GL(dl_x86_feature_control).shstk = cet_always_on;
- else if (DEFAULT_MEMCMP (valp->strval, "off", sizeof ("off")) == 0)
+ else if (memcmp (valp->strval, "off", sizeof ("off")) == 0)
GL(dl_x86_feature_control).shstk = cet_always_off;
- else if (DEFAULT_MEMCMP (valp->strval, "permissive",
- sizeof ("permissive")) == 0)
+ else if (memcmp (valp->strval, "permissive", sizeof ("permissive")) == 0)
GL(dl_x86_feature_control).shstk = cet_permissive;
}
#endif
diff --git a/sysdeps/x86_64/multiarch/dl-symbol-redir-ifunc.h b/sysdeps/x86_64/multiarch/dl-symbol-redir-ifunc.h
index 3fe73ca1c3..c7d8961bb6 100644
--- a/sysdeps/x86_64/multiarch/dl-symbol-redir-ifunc.h
+++ b/sysdeps/x86_64/multiarch/dl-symbol-redir-ifunc.h
@@ -19,6 +19,8 @@
#ifndef _DL_IFUNC_GENERIC_H
#define _DL_IFUNC_GENERIC_H
+#ifndef SHARED
+
#include <isa-level.h>
#if MINIMUM_X86_ISA_LEVEL >= 4
@@ -31,4 +33,17 @@
asm ("memset = " HAVE_MEMSET_IFUNC_GENERIC);
+
+#if MINIMUM_X86_ISA_LEVEL >= 4
+# define HAVE_MEMCMP_IFUNC_GENERIC "__memcmp_evex_movbe"
+#elif MINIMUM_X86_ISA_LEVEL == 3
+# define HAVE_MEMCMP_IFUNC_GENERIC "__memcmp_avx2_movbe"
+#else
+# define HAVE_MEMCMP_IFUNC_GENERIC "__memcmp_sse2"
+#endif
+
+asm ("memcmp = " HAVE_MEMCMP_IFUNC_GENERIC);
+
+#endif /* SHARED */
+
#endif
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 10/11] s390: Use dl-symbol-redir-ifunc.h on cpu-tunables
2023-10-10 18:01 [PATCH 00/11] Improve tunable handling Adhemerval Zanella
` (8 preceding siblings ...)
2023-10-10 18:01 ` [PATCH 09/11] x86: Use dl-symbol-redir-ifunc.h on cpu-tunables Adhemerval Zanella
@ 2023-10-10 18:01 ` Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 11/11] elf: Do not duplicate the GLIBC_TUNABLES string Adhemerval Zanella
10 siblings, 0 replies; 27+ messages in thread
From: Adhemerval Zanella @ 2023-10-10 18:01 UTC (permalink / raw)
To: libc-alpha, Siddhesh Poyarekar
Using the memcmp symbol directly allows the compile to inline the
memcmp calls (especially because _dl_tunable_set_hwcaps uses constants
values), generating better code.
Checked with tst-tunables on s390x-linux-gnu (qemu system).
---
sysdeps/s390/cpu-features.c | 30 +++++++++----------
.../s390/multiarch/dl-symbol-redir-ifunc.h | 2 ++
2 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/sysdeps/s390/cpu-features.c b/sysdeps/s390/cpu-features.c
index 39f8c23a60..55449ba07f 100644
--- a/sysdeps/s390/cpu-features.c
+++ b/sysdeps/s390/cpu-features.c
@@ -21,7 +21,7 @@
#include <elf/dl-tunables.h>
#include <ifunc-memcmp.h>
#include <string.h>
-extern __typeof (memcmp) MEMCMP_DEFAULT;
+#include <dl-symbol-redir-ifunc.h>
#define S390_COPY_CPU_FEATURES(SRC_PTR, DEST_PTR) \
(DEST_PTR)->hwcap = (SRC_PTR)->hwcap; \
@@ -89,9 +89,9 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
if ((*feature == 'z' || *feature == 'a'))
{
if ((feature_len == 5 && *feature == 'z'
- && MEMCMP_DEFAULT (feature, "zEC12", 5) == 0)
+ && memcmp (feature, "zEC12", 5) == 0)
|| (feature_len == 6 && *feature == 'a'
- && MEMCMP_DEFAULT (feature, "arch10", 6) == 0))
+ && memcmp (feature, "arch10", 6) == 0))
{
reset_features = true;
disable = true;
@@ -100,9 +100,9 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
}
else if ((feature_len == 3 && *feature == 'z'
- && MEMCMP_DEFAULT (feature, "z13", 3) == 0)
+ && memcmp (feature, "z13", 3) == 0)
|| (feature_len == 6 && *feature == 'a'
- && MEMCMP_DEFAULT (feature, "arch11", 6) == 0))
+ && memcmp (feature, "arch11", 6) == 0))
{
reset_features = true;
disable = true;
@@ -110,9 +110,9 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
}
else if ((feature_len == 3 && *feature == 'z'
- && MEMCMP_DEFAULT (feature, "z14", 3) == 0)
+ && memcmp (feature, "z14", 3) == 0)
|| (feature_len == 6 && *feature == 'a'
- && MEMCMP_DEFAULT (feature, "arch12", 6) == 0))
+ && memcmp (feature, "arch12", 6) == 0))
{
reset_features = true;
disable = true;
@@ -120,11 +120,11 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
}
else if ((feature_len == 3 && *feature == 'z'
- && (MEMCMP_DEFAULT (feature, "z15", 3) == 0
- || MEMCMP_DEFAULT (feature, "z16", 3) == 0))
+ && (memcmp (feature, "z15", 3) == 0
+ || memcmp (feature, "z16", 3) == 0))
|| (feature_len == 6
- && (MEMCMP_DEFAULT (feature, "arch13", 6) == 0
- || MEMCMP_DEFAULT (feature, "arch14", 6) == 0)))
+ && (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. */
@@ -134,14 +134,14 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
else if (*feature == 'H')
{
if (feature_len == 15
- && MEMCMP_DEFAULT (feature, "HWCAP_S390_VXRS", 15) == 0)
+ && 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_DEFAULT (feature, "HWCAP_S390_VXRS_EXT", 19) == 0)
+ && memcmp (feature, "HWCAP_S390_VXRS_EXT", 19) == 0)
{
hwcap_mask = HWCAP_S390_VXRS_EXT;
if (disable)
@@ -150,7 +150,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
hwcap_mask |= HWCAP_S390_VXRS;
}
else if (feature_len == 20
- && MEMCMP_DEFAULT (feature, "HWCAP_S390_VXRS_EXT2", 20) == 0)
+ && memcmp (feature, "HWCAP_S390_VXRS_EXT2", 20) == 0)
{
hwcap_mask = HWCAP_S390_VXRS_EXT2;
if (!disable)
@@ -160,7 +160,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
else if (*feature == 'S')
{
if (feature_len == 10
- && MEMCMP_DEFAULT (feature, "STFLE_MIE3", 10) == 0)
+ && memcmp (feature, "STFLE_MIE3", 10) == 0)
{
stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
}
diff --git a/sysdeps/s390/multiarch/dl-symbol-redir-ifunc.h b/sysdeps/s390/multiarch/dl-symbol-redir-ifunc.h
index aa084fdcde..0f58897c48 100644
--- a/sysdeps/s390/multiarch/dl-symbol-redir-ifunc.h
+++ b/sysdeps/s390/multiarch/dl-symbol-redir-ifunc.h
@@ -20,10 +20,12 @@
#define _DL_IFUNC_GENERIC_H
#include <ifunc-memset.h>
+#include <ifunc-memcmp.h>
#define IFUNC_SYMBOL_STR1(s) #s
#define IFUNC_SYMBOL_STR(s) IFUNC_SYMBOL_STR1(s)
asm ("memset = " IFUNC_SYMBOL_STR(MEMSET_DEFAULT));
+asm ("memcmp = " IFUNC_SYMBOL_STR(MEMCMP_DEFAULT));
#endif
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 11/11] elf: Do not duplicate the GLIBC_TUNABLES string
2023-10-10 18:01 [PATCH 00/11] Improve tunable handling Adhemerval Zanella
` (9 preceding siblings ...)
2023-10-10 18:01 ` [PATCH 10/11] s390: " Adhemerval Zanella
@ 2023-10-10 18:01 ` Adhemerval Zanella
2023-10-12 17:56 ` Noah Goldstein
10 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2023-10-10 18:01 UTC (permalink / raw)
To: libc-alpha, Siddhesh Poyarekar
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 | 66 +++----
elf/dl-tunables.h | 6 +-
sysdeps/generic/dl-tunables-parse.h | 128 ++++++++++++++
sysdeps/s390/cpu-features.c | 167 +++++++-----------
.../unix/sysv/linux/aarch64/cpu-features.c | 38 ++--
.../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 | 151 ++++++++++++++++
10 files changed, 456 insertions(+), 273 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 b39c14694c..869846f9da 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -36,28 +36,6 @@
#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,
char ***prev_envp)
@@ -134,14 +112,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 +136,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 respectibles values. VALSTRING is a duplicated values, where
- delimiters ':' are replaced with '\0', so string tunables are null
- terminated.
+ and their respectibles values. The VALSTRING is parse 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 a 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 +180,7 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
/* Skip the ':' or '='. */
p++;
- char *value = p;
+ const char *value = p;
while (*p != '=' && *p != ':' && *p != '\0')
p++;
@@ -211,8 +189,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 +197,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 +208,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 +220,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
@@ -264,9 +241,12 @@ __tunables_init (char **envp)
while ((envp = get_next_env (envp, &envname, &len, &envval,
&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 +264,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, 0);
break;
}
}
@@ -298,7 +278,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 +304,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 +341,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/sysdeps/generic/dl-tunables-parse.h b/sysdeps/generic/dl-tunables-parse.h
new file mode 100644
index 0000000000..d3e751ba55
--- /dev/null
+++ b/sysdeps/generic/dl-tunables-parse.h
@@ -0,0 +1,128 @@
+/* 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 <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 comma. 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"))
+ {
+ [...]
+ }
+ }
+*/
+
+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)
+{
+ 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..8b1466fa7b 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,85 +67,65 @@ 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. */
@@ -187,14 +149,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 dc09c1c827..3f1a6bcd62 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,28 +35,32 @@
struct cpu_list
{
const char *name;
+ size_t len;
uint64_t midr;
};
-static struct cpu_list cpu_list[] = {
- {"falkor", 0x510FC000},
- {"thunderxt88", 0x430F0A10},
- {"thunderx2t99", 0x431F0AF0},
- {"thunderx2t99p1", 0x420F5160},
- {"phecda", 0x680F0000},
- {"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 ("falkor", 0x510FC000),
+ CPU_LIST_ENTRY ("thunderxt88", 0x430F0A10),
+ CPU_LIST_ENTRY ("thunderx2t99", 0x431F0AF0),
+ CPU_LIST_ENTRY ("thunderx2t99p1", 0x420F5160),
+ CPU_LIST_ENTRY ("phecda", 0x680F0000),
+ 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;
}
@@ -65,7 +71,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..0608209768 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..a6cb9e6bb6
--- /dev/null
+++ b/sysdeps/x86/tst-hwcap-tunables.c
@@ -0,0 +1,151 @@
+/* Tests for powerpc 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/>. */
+
+#pragma GCC optimize ("O0")
+
+#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);
+
+ fprintf (stderr, "[%s] %s\n", __func__, argv[1]);
+ 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] 27+ messages in thread
* Re: [PATCH 01/11] elf: Remove /etc/suid-debug support
2023-10-10 18:01 ` [PATCH 01/11] elf: Remove /etc/suid-debug support Adhemerval Zanella
@ 2023-10-12 8:44 ` Florian Weimer
2023-10-12 10:43 ` Siddhesh Poyarekar
2023-10-13 13:47 ` Adhemerval Zanella Netto
0 siblings, 2 replies; 27+ messages in thread
From: Florian Weimer @ 2023-10-12 8:44 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha, Siddhesh Poyarekar
* Adhemerval Zanella:
> Since malloc debug support moved to a different library
> (libc_malloc_debug.so), the glibc.malloc.check requires preloading the
> debug library to enable it. It means that suid-debug support has not
> been working since 2.34.
Theoretically, it would be possible to get this working again using
/etc/ld.so.preload.
But I'm okay with removing this feature.
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 5107d16fe3..318a3661f0 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -2670,9 +2670,6 @@ process_envvars (struct dl_main_state *state)
> }
> while (*nextp != '\0');
>
> - if (__access ("/etc/suid-debug", F_OK) != 0)
> - GLRO(dl_debug_mask) = 0;
> -
This change looks wrong. The assignment to GLRO(dl_debug_mask) should
remain.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 04/11] elf: Add GLIBC_TUNABLES to unsecvars
2023-10-10 18:01 ` [PATCH 04/11] elf: Add GLIBC_TUNABLES " Adhemerval Zanella
@ 2023-10-12 8:46 ` Florian Weimer
2023-10-13 13:51 ` Adhemerval Zanella Netto
0 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer @ 2023-10-12 8:46 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha, Siddhesh Poyarekar
* Adhemerval Zanella:
> setuid/setgid process now ignores any glibc tunables, and filters out
> all environment variables that might changes its behavior. This patch
> also adds GLIBC_TUNABLES, so any spawned process by setuid/setgid
> processes should set tunable explicitly.
This should be committed earlier, before the patch that removes
SXID_ERASE support.
Otherwise:
Reviewed-by: Florian Weimer <fweimer@redhat.com>
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 03/11] elf: Add all malloc tunable to unsecvars
2023-10-10 18:01 ` [PATCH 03/11] elf: Add all malloc tunable to unsecvars Adhemerval Zanella
@ 2023-10-12 8:47 ` Florian Weimer
2023-10-13 13:53 ` Adhemerval Zanella Netto
0 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer @ 2023-10-12 8:47 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha, Siddhesh Poyarekar
* Adhemerval Zanella:
> @@ -36,57 +31,22 @@
>
> static char SETGID_CHILD[] = "setgid-child";
>
> -#ifndef test_child
> static int
> test_child (void)
> {
> - if (getenv ("MALLOC_CHECK_") != NULL)
> - {
> - printf ("MALLOC_CHECK_ is still set\n");
> - return 1;
> - }
> -
> - if (getenv ("MALLOC_MMAP_THRESHOLD_") == NULL)
> - {
> - printf ("MALLOC_MMAP_THRESHOLD_ lost\n");
> - return 1;
> - }
> + int ret = 0;
>
> - if (getenv ("LD_HWCAP_MASK") != NULL)
> + const char *nextp = UNSECURE_ENVVARS;
> + do
> {
> - printf ("LD_HWCAP_MASK still set\n");
> - return 1;
> + const char *env = getenv (nextp);
> + ret |= env != NULL;
> + nextp = strchr (nextp, '\0') + 1;
> }
> + while (*nextp != '\0');
I think we should keep some tests that are independent of
UNSECURE_ENVVARS.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 01/11] elf: Remove /etc/suid-debug support
2023-10-12 8:44 ` Florian Weimer
@ 2023-10-12 10:43 ` Siddhesh Poyarekar
2023-10-12 16:01 ` Siddhesh Poyarekar
2023-10-13 13:47 ` Adhemerval Zanella Netto
1 sibling, 1 reply; 27+ messages in thread
From: Siddhesh Poyarekar @ 2023-10-12 10:43 UTC (permalink / raw)
To: Florian Weimer, Adhemerval Zanella; +Cc: libc-alpha
On 2023-10-12 04:44, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> Since malloc debug support moved to a different library
>> (libc_malloc_debug.so), the glibc.malloc.check requires preloading the
>> debug library to enable it. It means that suid-debug support has not
>> been working since 2.34.
>
> Theoretically, it would be possible to get this working again using
> /etc/ld.so.preload.
>
> But I'm okay with removing this feature.
>
>> diff --git a/elf/rtld.c b/elf/rtld.c
>> index 5107d16fe3..318a3661f0 100644
>> --- a/elf/rtld.c
>> +++ b/elf/rtld.c
>> @@ -2670,9 +2670,6 @@ process_envvars (struct dl_main_state *state)
>> }
>> while (*nextp != '\0');
>>
>> - if (__access ("/etc/suid-debug", F_OK) != 0)
>> - GLRO(dl_debug_mask) = 0;
>> -
>
> This change looks wrong. The assignment to GLRO(dl_debug_mask) should
> remain.
Or maybe process_dl_debug ought to be fixed to bail out on
__libc_enable_secure?
Sid
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 01/11] elf: Remove /etc/suid-debug support
2023-10-12 10:43 ` Siddhesh Poyarekar
@ 2023-10-12 16:01 ` Siddhesh Poyarekar
0 siblings, 0 replies; 27+ messages in thread
From: Siddhesh Poyarekar @ 2023-10-12 16:01 UTC (permalink / raw)
To: Florian Weimer, Adhemerval Zanella; +Cc: libc-alpha
On 2023-10-12 06:43, Siddhesh Poyarekar wrote:
> On 2023-10-12 04:44, Florian Weimer wrote:
>> * Adhemerval Zanella:
>>
>>> Since malloc debug support moved to a different library
>>> (libc_malloc_debug.so), the glibc.malloc.check requires preloading the
>>> debug library to enable it. It means that suid-debug support has not
>>> been working since 2.34.
>>
>> Theoretically, it would be possible to get this working again using
>> /etc/ld.so.preload.
>>
>> But I'm okay with removing this feature.
>>
>>> diff --git a/elf/rtld.c b/elf/rtld.c
>>> index 5107d16fe3..318a3661f0 100644
>>> --- a/elf/rtld.c
>>> +++ b/elf/rtld.c
>>> @@ -2670,9 +2670,6 @@ process_envvars (struct dl_main_state *state)
>>> }
>>> while (*nextp != '\0');
>>> - if (__access ("/etc/suid-debug", F_OK) != 0)
>>> - GLRO(dl_debug_mask) = 0;
>>> -
>>
>> This change looks wrong. The assignment to GLRO(dl_debug_mask) should
>> remain.
>
> Or maybe process_dl_debug ought to be fixed to bail out on
> __libc_enable_secure?
Also another related exercise could be to look at the various LD_*
variables that are processed above and avoid processing those that also
figure in unsecvars.h, thus making sure that not only do they get
erased, but also get ignored in setuid programs.
Thanks,
Sid
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 11/11] elf: Do not duplicate the GLIBC_TUNABLES string
2023-10-10 18:01 ` [PATCH 11/11] elf: Do not duplicate the GLIBC_TUNABLES string Adhemerval Zanella
@ 2023-10-12 17:56 ` Noah Goldstein
2023-10-13 14:40 ` Adhemerval Zanella Netto
0 siblings, 1 reply; 27+ messages in thread
From: Noah Goldstein @ 2023-10-12 17:56 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha, Siddhesh Poyarekar
On Tue, Oct 10, 2023 at 1:07 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> 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 | 66 +++----
> elf/dl-tunables.h | 6 +-
> sysdeps/generic/dl-tunables-parse.h | 128 ++++++++++++++
> sysdeps/s390/cpu-features.c | 167 +++++++-----------
> .../unix/sysv/linux/aarch64/cpu-features.c | 38 ++--
> .../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 | 151 ++++++++++++++++
> 10 files changed, 456 insertions(+), 273 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 b39c14694c..869846f9da 100644
> --- a/elf/dl-tunables.c
> +++ b/elf/dl-tunables.c
> @@ -36,28 +36,6 @@
> #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,
> char ***prev_envp)
> @@ -134,14 +112,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 +136,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 respectibles values. VALSTRING is a duplicated values, where
> - delimiters ':' are replaced with '\0', so string tunables are null
> - terminated.
> + and their respectibles values. The VALSTRING is parse 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 a 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 +180,7 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
> /* Skip the ':' or '='. */
> p++;
>
> - char *value = p;
> + const char *value = p;
>
> while (*p != '=' && *p != ':' && *p != '\0')
> p++;
> @@ -211,8 +189,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 +197,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 +208,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 +220,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
> @@ -264,9 +241,12 @@ __tunables_init (char **envp)
> while ((envp = get_next_env (envp, &envname, &len, &envval,
> &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 +264,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, 0);
> break;
> }
> }
> @@ -298,7 +278,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 +304,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 +341,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/sysdeps/generic/dl-tunables-parse.h b/sysdeps/generic/dl-tunables-parse.h
> new file mode 100644
> index 0000000000..d3e751ba55
> --- /dev/null
> +++ b/sysdeps/generic/dl-tunables-parse.h
> @@ -0,0 +1,128 @@
> +/* 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 <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 comma. 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"))
> + {
> + [...]
> + }
> + }
> +*/
> +
> +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)
> +{
> + 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..8b1466fa7b 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,85 +67,65 @@ 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. */
> @@ -187,14 +149,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 dc09c1c827..3f1a6bcd62 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,28 +35,32 @@
> struct cpu_list
> {
> const char *name;
> + size_t len;
> uint64_t midr;
> };
>
> -static struct cpu_list cpu_list[] = {
> - {"falkor", 0x510FC000},
> - {"thunderxt88", 0x430F0A10},
> - {"thunderx2t99", 0x431F0AF0},
> - {"thunderx2t99p1", 0x420F5160},
> - {"phecda", 0x680F0000},
> - {"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 ("falkor", 0x510FC000),
> + CPU_LIST_ENTRY ("thunderxt88", 0x430F0A10),
> + CPU_LIST_ENTRY ("thunderx2t99", 0x431F0AF0),
> + CPU_LIST_ENTRY ("thunderx2t99p1", 0x420F5160),
> + CPU_LIST_ENTRY ("phecda", 0x680F0000),
> + 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;
> }
> @@ -65,7 +71,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..0608209768 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..a6cb9e6bb6
> --- /dev/null
> +++ b/sysdeps/x86/tst-hwcap-tunables.c
> @@ -0,0 +1,151 @@
> +/* Tests for powerpc 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/>. */
> +
> +#pragma GCC optimize ("O0")
> +
> +#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);
> +
> + fprintf (stderr, "[%s] %s\n", __func__, argv[1]);
> + 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
>
Nit: When applying
```
Applying: elf: Do not duplicate the GLIBC_TUNABLES string
.git/rebase-apply/patch:258: trailing whitespace.
/*
.git/rebase-apply/patch:1135: space before tab in indent.
if (strcmp (impls[i].name, tests[ntest].funcs[f]) == 0)
warning: 2 lines add whitespace errors.
```
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 09/11] x86: Use dl-symbol-redir-ifunc.h on cpu-tunables
2023-10-10 18:01 ` [PATCH 09/11] x86: Use dl-symbol-redir-ifunc.h on cpu-tunables Adhemerval Zanella
@ 2023-10-12 18:11 ` Noah Goldstein
0 siblings, 0 replies; 27+ messages in thread
From: Noah Goldstein @ 2023-10-12 18:11 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha, Siddhesh Poyarekar
On Tue, Oct 10, 2023 at 1:07 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> The dl-symbol-redir-ifunc.h redirects compiler-generated libcalls to
> arch-specific memory implementations to avoid ifun calls where it is not
> yet possible. The memcmp-isa-default-impl.h aims to fix the same issue
> by calling the specific memset implementation directly.
>
> Using the memcmp symbol directly allows the compile to inline the memset
> calls (especially because _dl_tunable_set_hwcaps uses constants values),
> generating better code.
>
> For i386, _dl_writev with PIE requires to use the old 'int $0x80'
> syscall mode because the calling the TLS register (gs) is not yet
> initialized.
>
> Checked on x86_64-linux-gnu.
> ---
> .../i686/multiarch/dl-symbol-redir-ifunc.h | 5 +++
> .../sysv/linux/i386/dl-writev.h} | 18 ++++-----
> sysdeps/x86/cpu-tunables.c | 39 ++++++-------------
> .../x86_64/multiarch/dl-symbol-redir-ifunc.h | 15 +++++++
> 4 files changed, 39 insertions(+), 38 deletions(-)
> rename sysdeps/{x86_64/memcmp-isa-default-impl.h => unix/sysv/linux/i386/dl-writev.h} (62%)
>
> diff --git a/sysdeps/i386/i686/multiarch/dl-symbol-redir-ifunc.h b/sysdeps/i386/i686/multiarch/dl-symbol-redir-ifunc.h
> index dee69d19db..220c586bd2 100644
> --- a/sysdeps/i386/i686/multiarch/dl-symbol-redir-ifunc.h
> +++ b/sysdeps/i386/i686/multiarch/dl-symbol-redir-ifunc.h
> @@ -19,6 +19,11 @@
> #ifndef _DL_IFUNC_GENERIC_H
> #define _DL_IFUNC_GENERIC_H
>
> +#ifndef SHARED
> +
> asm ("memset = __memset_ia32");
> +asm ("memcmp = __memcmp_ia32");
> +
> +#endif /* SHARED */
>
> #endif
> diff --git a/sysdeps/x86_64/memcmp-isa-default-impl.h b/sysdeps/unix/sysv/linux/i386/dl-writev.h
> similarity index 62%
> rename from sysdeps/x86_64/memcmp-isa-default-impl.h
> rename to sysdeps/unix/sysv/linux/i386/dl-writev.h
> index 0962e83c3d..624d0e46b0 100644
> --- a/sysdeps/x86_64/memcmp-isa-default-impl.h
> +++ b/sysdeps/unix/sysv/linux/i386/dl-writev.h
> @@ -1,5 +1,5 @@
> -/* Set default memcmp impl based on ISA level.
> - Copyright (C) 2022-2023 Free Software Foundation, Inc.
> +/* Message-writing for the dynamic linker. Linux/i386 version.
> + Copyright (C) 2013-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
> @@ -16,13 +16,9 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> -#include <isa-level.h>
> -#if MINIMUM_X86_ISA_LEVEL == 1 || MINIMUM_X86_ISA_LEVEL == 2
> -# define DEFAULT_MEMCMP __memcmp_sse2
> -#elif MINIMUM_X86_ISA_LEVEL == 3
> -# define DEFAULT_MEMCMP __memcmp_avx2_movbe
> -#elif MINIMUM_X86_ISA_LEVEL == 4
> -# define DEFAULT_MEMCMP __memcmp_evex_movbe
> -#else
> -# error "Unknown default memcmp implementation"
> +#if BUILD_PIE_DEFAULT
> +/* Can't use "call *%gs:SYSINFO_OFFSET" during startup in static PIE. */
> +# define I386_USE_SYSENTER 0
> #endif
> +
> +#include <sysdeps/unix/sysv/linux/dl-writev.h>
> diff --git a/sysdeps/x86/cpu-tunables.c b/sysdeps/x86/cpu-tunables.c
> index 0d4f328585..5697885226 100644
> --- a/sysdeps/x86/cpu-tunables.c
> +++ b/sysdeps/x86/cpu-tunables.c
> @@ -24,24 +24,11 @@
> #include <string.h>
> #include <cpu-features.h>
> #include <ldsodefs.h>
> -
> -/* We can't use IFUNC memcmp nor strlen in init_cpu_features from libc.a
> - since IFUNC must be set up by init_cpu_features. */
> -#if defined USE_MULTIARCH && !defined SHARED
> -# ifdef __x86_64__
> -/* DEFAULT_MEMCMP by sysdeps/x86_64/memcmp-isa-default-impl.h. */
> -# include <sysdeps/x86_64/memcmp-isa-default-impl.h>
> -# else
> -# define DEFAULT_MEMCMP __memcmp_ia32
> -# endif
> -extern __typeof (memcmp) DEFAULT_MEMCMP;
> -#else
> -# define DEFAULT_MEMCMP memcmp
> -#endif
> +#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 (!DEFAULT_MEMCMP (f, #name, len)) \
> + if (memcmp (f, #name, len) == 0) \
> { \
> CPU_FEATURE_UNSET (cpu_features, name) \
> break; \
> @@ -51,7 +38,7 @@ extern __typeof (memcmp) DEFAULT_MEMCMP;
> which isn't available. */
> #define CHECK_GLIBC_IFUNC_PREFERRED_OFF(f, cpu_features, name, len) \
> _Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
> - if (!DEFAULT_MEMCMP (f, #name, len)) \
> + if (memcmp (f, #name, len) == 0) \
> { \
> cpu_features->preferred[index_arch_##name] \
> &= ~bit_arch_##name; \
> @@ -62,7 +49,7 @@ extern __typeof (memcmp) DEFAULT_MEMCMP;
> #define CHECK_GLIBC_IFUNC_PREFERRED_BOTH(f, cpu_features, name, \
> disable, len) \
> _Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
> - if (!DEFAULT_MEMCMP (f, #name, len)) \
> + if (memcmp (f, #name, len) == 0) \
> { \
> if (disable) \
> cpu_features->preferred[index_arch_##name] &= ~bit_arch_##name; \
> @@ -76,7 +63,7 @@ extern __typeof (memcmp) DEFAULT_MEMCMP;
> #define CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH(f, cpu_features, name, \
> need, disable, len) \
> _Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
> - if (!DEFAULT_MEMCMP (f, #name, len)) \
> + if (memcmp (f, #name, len) == 0) \
> { \
> if (disable) \
> cpu_features->preferred[index_arch_##name] &= ~bit_arch_##name; \
> @@ -177,7 +164,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> 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 (!DEFAULT_MEMCMP (n, "XSAVEC", 6))
> + if (memcmp (n, "XSAVEC", 6) == 0)
> {
> /* Update xsave_state_size to XSAVE state size. */
> cpu_features->xsave_state_size
> @@ -290,12 +277,11 @@ attribute_hidden
> void
> TUNABLE_CALLBACK (set_x86_ibt) (tunable_val_t *valp)
> {
> - if (DEFAULT_MEMCMP (valp->strval, "on", sizeof ("on")) == 0)
> + if (memcmp (valp->strval, "on", sizeof ("on")) == 0)
> GL(dl_x86_feature_control).ibt = cet_always_on;
> - else if (DEFAULT_MEMCMP (valp->strval, "off", sizeof ("off")) == 0)
> + else if (memcmp (valp->strval, "off", sizeof ("off")) == 0)
> GL(dl_x86_feature_control).ibt = cet_always_off;
> - else if (DEFAULT_MEMCMP (valp->strval, "permissive",
> - sizeof ("permissive")) == 0)
> + else if (memcmp (valp->strval, "permissive", sizeof ("permissive")) == 0)
> GL(dl_x86_feature_control).ibt = cet_permissive;
> }
>
> @@ -303,12 +289,11 @@ attribute_hidden
> void
> TUNABLE_CALLBACK (set_x86_shstk) (tunable_val_t *valp)
> {
> - if (DEFAULT_MEMCMP (valp->strval, "on", sizeof ("on")) == 0)
> + if (memcmp (valp->strval, "on", sizeof ("on")) == 0)
> GL(dl_x86_feature_control).shstk = cet_always_on;
> - else if (DEFAULT_MEMCMP (valp->strval, "off", sizeof ("off")) == 0)
> + else if (memcmp (valp->strval, "off", sizeof ("off")) == 0)
> GL(dl_x86_feature_control).shstk = cet_always_off;
> - else if (DEFAULT_MEMCMP (valp->strval, "permissive",
> - sizeof ("permissive")) == 0)
> + else if (memcmp (valp->strval, "permissive", sizeof ("permissive")) == 0)
> GL(dl_x86_feature_control).shstk = cet_permissive;
> }
> #endif
> diff --git a/sysdeps/x86_64/multiarch/dl-symbol-redir-ifunc.h b/sysdeps/x86_64/multiarch/dl-symbol-redir-ifunc.h
> index 3fe73ca1c3..c7d8961bb6 100644
> --- a/sysdeps/x86_64/multiarch/dl-symbol-redir-ifunc.h
> +++ b/sysdeps/x86_64/multiarch/dl-symbol-redir-ifunc.h
> @@ -19,6 +19,8 @@
> #ifndef _DL_IFUNC_GENERIC_H
> #define _DL_IFUNC_GENERIC_H
>
> +#ifndef SHARED
> +
> #include <isa-level.h>
>
> #if MINIMUM_X86_ISA_LEVEL >= 4
> @@ -31,4 +33,17 @@
>
> asm ("memset = " HAVE_MEMSET_IFUNC_GENERIC);
>
> +
> +#if MINIMUM_X86_ISA_LEVEL >= 4
> +# define HAVE_MEMCMP_IFUNC_GENERIC "__memcmp_evex_movbe"
> +#elif MINIMUM_X86_ISA_LEVEL == 3
> +# define HAVE_MEMCMP_IFUNC_GENERIC "__memcmp_avx2_movbe"
> +#else
> +# define HAVE_MEMCMP_IFUNC_GENERIC "__memcmp_sse2"
> +#endif
> +
> +asm ("memcmp = " HAVE_MEMCMP_IFUNC_GENERIC);
> +
> +#endif /* SHARED */
> +
> #endif
> --
> 2.34.1
>
LGTM.
Reviewed-by: Noah Goldstein <goldstein.w.n@gmail.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 01/11] elf: Remove /etc/suid-debug support
2023-10-12 8:44 ` Florian Weimer
2023-10-12 10:43 ` Siddhesh Poyarekar
@ 2023-10-13 13:47 ` Adhemerval Zanella Netto
1 sibling, 0 replies; 27+ messages in thread
From: Adhemerval Zanella Netto @ 2023-10-13 13:47 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha, Siddhesh Poyarekar
On 12/10/23 05:44, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> Since malloc debug support moved to a different library
>> (libc_malloc_debug.so), the glibc.malloc.check requires preloading the
>> debug library to enable it. It means that suid-debug support has not
>> been working since 2.34.
>
> Theoretically, it would be possible to get this working again using
> /etc/ld.so.preload.
>
> But I'm okay with removing this feature.
Indeed, but requiring ld.so.preload setup would end up with a clunky
interface (two different files with possible multiple way of failures).
I think this interface made sense back when malloc debugging have limit
options, specially in some environments.
>
>> diff --git a/elf/rtld.c b/elf/rtld.c
>> index 5107d16fe3..318a3661f0 100644
>> --- a/elf/rtld.c
>> +++ b/elf/rtld.c
>> @@ -2670,9 +2670,6 @@ process_envvars (struct dl_main_state *state)
>> }
>> while (*nextp != '\0');
>>
>> - if (__access ("/etc/suid-debug", F_OK) != 0)
>> - GLRO(dl_debug_mask) = 0;
>> -
>
> This change looks wrong. The assignment to GLRO(dl_debug_mask) should
> remain.
Right.
> Or maybe process_dl_debug ought to be fixed to bail out on __libc_enable_secure?
It is an option and I do like this, but for this patchset I think the simple
solution would to keep 'GLRO(dl_debug_mask) = 0;' for setuid on process_envvars.
I will send another patch that just skip the LD_DEBUG parsing for setuid.
> Also another related exercise could be to look at the various LD_* variables that are processed above and avoid processing those that also figure in unsecvars.h, thus making sure that not only do they get erased, but also get ignored in setuid programs.
It is on my plan to check on this.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 04/11] elf: Add GLIBC_TUNABLES to unsecvars
2023-10-12 8:46 ` Florian Weimer
@ 2023-10-13 13:51 ` Adhemerval Zanella Netto
2023-10-13 14:11 ` Florian Weimer
0 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella Netto @ 2023-10-13 13:51 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha, Siddhesh Poyarekar
On 12/10/23 05:46, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> setuid/setgid process now ignores any glibc tunables, and filters out
>> all environment variables that might changes its behavior. This patch
>> also adds GLIBC_TUNABLES, so any spawned process by setuid/setgid
>> processes should set tunable explicitly.
>
> This should be committed earlier, before the patch that removes
> SXID_ERASE support.
>
> Otherwise:
>
> Reviewed-by: Florian Weimer <fweimer@redhat.com>
Do you mean move it before 'elf: Ignore GLIBC_TUNABLES for setuid/setgid binaries'
patch?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 03/11] elf: Add all malloc tunable to unsecvars
2023-10-12 8:47 ` Florian Weimer
@ 2023-10-13 13:53 ` Adhemerval Zanella Netto
2023-10-13 14:12 ` Florian Weimer
0 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella Netto @ 2023-10-13 13:53 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha, Siddhesh Poyarekar
On 12/10/23 05:47, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> @@ -36,57 +31,22 @@
>>
>> static char SETGID_CHILD[] = "setgid-child";
>>
>> -#ifndef test_child
>> static int
>> test_child (void)
>> {
>> - if (getenv ("MALLOC_CHECK_") != NULL)
>> - {
>> - printf ("MALLOC_CHECK_ is still set\n");
>> - return 1;
>> - }
>> -
>> - if (getenv ("MALLOC_MMAP_THRESHOLD_") == NULL)
>> - {
>> - printf ("MALLOC_MMAP_THRESHOLD_ lost\n");
>> - return 1;
>> - }
>> + int ret = 0;
>>
>> - if (getenv ("LD_HWCAP_MASK") != NULL)
>> + const char *nextp = UNSECURE_ENVVARS;
>> + do
>> {
>> - printf ("LD_HWCAP_MASK still set\n");
>> - return 1;
>> + const char *env = getenv (nextp);
>> + ret |= env != NULL;
>> + nextp = strchr (nextp, '\0') + 1;
>> }
>> + while (*nextp != '\0');
>
> I think we should keep some tests that are independent of
> UNSECURE_ENVVARS.
Not sure what which tests you mean here, my understanding is elf/tst-env-setuid.c
is testing that UNSECURE_ENVVARS is being correctly filtered out.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 04/11] elf: Add GLIBC_TUNABLES to unsecvars
2023-10-13 13:51 ` Adhemerval Zanella Netto
@ 2023-10-13 14:11 ` Florian Weimer
2023-10-13 14:26 ` Adhemerval Zanella Netto
0 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer @ 2023-10-13 14:11 UTC (permalink / raw)
To: Adhemerval Zanella Netto; +Cc: libc-alpha, Siddhesh Poyarekar
* Adhemerval Zanella Netto:
> On 12/10/23 05:46, Florian Weimer wrote:
>> * Adhemerval Zanella:
>>
>>> setuid/setgid process now ignores any glibc tunables, and filters out
>>> all environment variables that might changes its behavior. This patch
>>> also adds GLIBC_TUNABLES, so any spawned process by setuid/setgid
>>> processes should set tunable explicitly.
>>
>> This should be committed earlier, before the patch that removes
>> SXID_ERASE support.
>>
>> Otherwise:
>>
>> Reviewed-by: Florian Weimer <fweimer@redhat.com>
>
> Do you mean move it before 'elf: Ignore GLIBC_TUNABLES for
> setuid/setgid binaries' patch?
Yes, exactly.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 03/11] elf: Add all malloc tunable to unsecvars
2023-10-13 13:53 ` Adhemerval Zanella Netto
@ 2023-10-13 14:12 ` Florian Weimer
2023-10-13 14:27 ` Adhemerval Zanella Netto
0 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer @ 2023-10-13 14:12 UTC (permalink / raw)
To: Adhemerval Zanella Netto; +Cc: libc-alpha, Siddhesh Poyarekar
* Adhemerval Zanella Netto:
> On 12/10/23 05:47, Florian Weimer wrote:
>> * Adhemerval Zanella:
>>
>>> @@ -36,57 +31,22 @@
>>>
>>> static char SETGID_CHILD[] = "setgid-child";
>>>
>>> -#ifndef test_child
>>> static int
>>> test_child (void)
>>> {
>>> - if (getenv ("MALLOC_CHECK_") != NULL)
>>> - {
>>> - printf ("MALLOC_CHECK_ is still set\n");
>>> - return 1;
>>> - }
>>> -
>>> - if (getenv ("MALLOC_MMAP_THRESHOLD_") == NULL)
>>> - {
>>> - printf ("MALLOC_MMAP_THRESHOLD_ lost\n");
>>> - return 1;
>>> - }
>>> + int ret = 0;
>>>
>>> - if (getenv ("LD_HWCAP_MASK") != NULL)
>>> + const char *nextp = UNSECURE_ENVVARS;
>>> + do
>>> {
>>> - printf ("LD_HWCAP_MASK still set\n");
>>> - return 1;
>>> + const char *env = getenv (nextp);
>>> + ret |= env != NULL;
>>> + nextp = strchr (nextp, '\0') + 1;
>>> }
>>> + while (*nextp != '\0');
>>
>> I think we should keep some tests that are independent of
>> UNSECURE_ENVVARS.
>
> Not sure what which tests you mean here, my understanding is
> elf/tst-env-setuid.c is testing that UNSECURE_ENVVARS is being
> correctly filtered out.
I mean that we should test some variables we know should be there in
UNSECURE_ENVVARS, without relying on UNSECURE_ENVVARS to drive the test.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 04/11] elf: Add GLIBC_TUNABLES to unsecvars
2023-10-13 14:11 ` Florian Weimer
@ 2023-10-13 14:26 ` Adhemerval Zanella Netto
0 siblings, 0 replies; 27+ messages in thread
From: Adhemerval Zanella Netto @ 2023-10-13 14:26 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha, Siddhesh Poyarekar
On 13/10/23 11:11, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
>
>> On 12/10/23 05:46, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> setuid/setgid process now ignores any glibc tunables, and filters out
>>>> all environment variables that might changes its behavior. This patch
>>>> also adds GLIBC_TUNABLES, so any spawned process by setuid/setgid
>>>> processes should set tunable explicitly.
>>>
>>> This should be committed earlier, before the patch that removes
>>> SXID_ERASE support.
>>>
>>> Otherwise:
>>>
>>> Reviewed-by: Florian Weimer <fweimer@redhat.com>
>>
>> Do you mean move it before 'elf: Ignore GLIBC_TUNABLES for
>> setuid/setgid binaries' patch?
>
> Yes, exactly.
Alright, I will change it.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 03/11] elf: Add all malloc tunable to unsecvars
2023-10-13 14:12 ` Florian Weimer
@ 2023-10-13 14:27 ` Adhemerval Zanella Netto
0 siblings, 0 replies; 27+ messages in thread
From: Adhemerval Zanella Netto @ 2023-10-13 14:27 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha, Siddhesh Poyarekar
On 13/10/23 11:12, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
>
>> On 12/10/23 05:47, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> @@ -36,57 +31,22 @@
>>>>
>>>> static char SETGID_CHILD[] = "setgid-child";
>>>>
>>>> -#ifndef test_child
>>>> static int
>>>> test_child (void)
>>>> {
>>>> - if (getenv ("MALLOC_CHECK_") != NULL)
>>>> - {
>>>> - printf ("MALLOC_CHECK_ is still set\n");
>>>> - return 1;
>>>> - }
>>>> -
>>>> - if (getenv ("MALLOC_MMAP_THRESHOLD_") == NULL)
>>>> - {
>>>> - printf ("MALLOC_MMAP_THRESHOLD_ lost\n");
>>>> - return 1;
>>>> - }
>>>> + int ret = 0;
>>>>
>>>> - if (getenv ("LD_HWCAP_MASK") != NULL)
>>>> + const char *nextp = UNSECURE_ENVVARS;
>>>> + do
>>>> {
>>>> - printf ("LD_HWCAP_MASK still set\n");
>>>> - return 1;
>>>> + const char *env = getenv (nextp);
>>>> + ret |= env != NULL;
>>>> + nextp = strchr (nextp, '\0') + 1;
>>>> }
>>>> + while (*nextp != '\0');
>>>
>>> I think we should keep some tests that are independent of
>>> UNSECURE_ENVVARS.
>>
>> Not sure what which tests you mean here, my understanding is
>> elf/tst-env-setuid.c is testing that UNSECURE_ENVVARS is being
>> correctly filtered out.
>
> I mean that we should test some variables we know should be there in
> UNSECURE_ENVVARS, without relying on UNSECURE_ENVVARS to drive the test.
Alright, I will extend the test.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 11/11] elf: Do not duplicate the GLIBC_TUNABLES string
2023-10-12 17:56 ` Noah Goldstein
@ 2023-10-13 14:40 ` Adhemerval Zanella Netto
0 siblings, 0 replies; 27+ messages in thread
From: Adhemerval Zanella Netto @ 2023-10-13 14:40 UTC (permalink / raw)
To: Noah Goldstein; +Cc: libc-alpha, Siddhesh Poyarekar
On 12/10/23 14:56, Noah Goldstein wrote:
>>
>
> Nit: When applying
> ```
> Applying: elf: Do not duplicate the GLIBC_TUNABLES string
> .git/rebase-apply/patch:258: trailing whitespace.
> /*
> .git/rebase-apply/patch:1135: space before tab in indent.
> if (strcmp (impls[i].name, tests[ntest].funcs[f]) == 0)
> warning: 2 lines add whitespace errors.
> ```
Thanks, I have fixed them.
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2023-10-13 14:40 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-10 18:01 [PATCH 00/11] Improve tunable handling Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 01/11] elf: Remove /etc/suid-debug support Adhemerval Zanella
2023-10-12 8:44 ` Florian Weimer
2023-10-12 10:43 ` Siddhesh Poyarekar
2023-10-12 16:01 ` Siddhesh Poyarekar
2023-10-13 13:47 ` Adhemerval Zanella Netto
2023-10-10 18:01 ` [PATCH 02/11] elf: Ignore GLIBC_TUNABLES for setuid/setgid binaries Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 03/11] elf: Add all malloc tunable to unsecvars Adhemerval Zanella
2023-10-12 8:47 ` Florian Weimer
2023-10-13 13:53 ` Adhemerval Zanella Netto
2023-10-13 14:12 ` Florian Weimer
2023-10-13 14:27 ` Adhemerval Zanella Netto
2023-10-10 18:01 ` [PATCH 04/11] elf: Add GLIBC_TUNABLES " Adhemerval Zanella
2023-10-12 8:46 ` Florian Weimer
2023-10-13 13:51 ` Adhemerval Zanella Netto
2023-10-13 14:11 ` Florian Weimer
2023-10-13 14:26 ` Adhemerval Zanella Netto
2023-10-10 18:01 ` [PATCH 05/11] elf: Do not process invalid tunable format Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 06/11] elf: Do not parse ill-formatted strings Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 07/11] elf: Fix _dl_debug_vdprintf to work before self-relocation Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 08/11] elf: Emit warning if tunable is ill-formatted Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 09/11] x86: Use dl-symbol-redir-ifunc.h on cpu-tunables Adhemerval Zanella
2023-10-12 18:11 ` Noah Goldstein
2023-10-10 18:01 ` [PATCH 10/11] s390: " Adhemerval Zanella
2023-10-10 18:01 ` [PATCH 11/11] elf: Do not duplicate the GLIBC_TUNABLES string Adhemerval Zanella
2023-10-12 17:56 ` Noah Goldstein
2023-10-13 14:40 ` Adhemerval Zanella Netto
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).