* [PATCH v7 0/4] glibc tunables @ 2016-12-30 4:09 Siddhesh Poyarekar 2016-12-30 4:09 ` [PATCH 1/4] Add framework for tunables Siddhesh Poyarekar ` (3 more replies) 0 siblings, 4 replies; 27+ messages in thread From: Siddhesh Poyarekar @ 2016-12-30 4:09 UTC (permalink / raw) To: libc-alpha; +Cc: fweimer, carlos Hi, Here's v7 of the tunables patchset. Changes are as follows: - Fixed up issues that Florian pointed out - Replaced mmap call with an INTERNAL_SYSCALL to avoid setting errno - Dropped the call to __getpagesize since the kernel is capable of rounding requests to nearest page size I tried to avoid the copying of the tunables string, but it complicated the tunables interface by introducing a length attribute for strings so I dropped that idea. Tested on x86_64. Siddhesh Poyarekar (4): Add framework for tunables Initialize tunable list with the GLIBC_TUNABLES environment variable Enhance --enable-tunables to select tunables frontend at build time User manual documentation for tunables INSTALL | 18 ++ Makeconfig | 16 + README.tunables | 85 ++++++ config.h.in | 3 + config.make.in | 2 + configure | 17 ++ configure.ac | 10 + csu/init-first.c | 2 - csu/libc-start.c | 8 + elf/Makefile | 13 + elf/Versions | 3 + elf/dl-support.c | 2 + elf/dl-sysdep.c | 4 + elf/dl-tunable-types.h | 46 +++ elf/dl-tunables.c | 457 +++++++++++++++++++++++++++++ elf/dl-tunables.h | 92 ++++++ elf/dl-tunables.list | 69 +++++ elf/rtld.c | 2 + malloc/Makefile | 6 + malloc/arena.c | 54 ++++ malloc/tst-malloc-usable-static-tunables.c | 1 + malloc/tst-malloc-usable-static.c | 1 + malloc/tst-malloc-usable-tunables.c | 1 + manual/Makefile | 3 +- manual/install.texi | 21 ++ manual/probes.texi | 2 +- manual/tunables.texi | 185 ++++++++++++ scripts/gen-tunables.awk | 157 ++++++++++ sysdeps/mach/hurd/dl-sysdep.c | 4 + 29 files changed, 1280 insertions(+), 4 deletions(-) create mode 100644 README.tunables create mode 100644 elf/dl-tunable-types.h create mode 100644 elf/dl-tunables.c create mode 100644 elf/dl-tunables.h create mode 100644 elf/dl-tunables.list create mode 100644 malloc/tst-malloc-usable-static-tunables.c create mode 100644 malloc/tst-malloc-usable-static.c create mode 100644 malloc/tst-malloc-usable-tunables.c create mode 100644 manual/tunables.texi create mode 100644 scripts/gen-tunables.awk -- 2.7.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/4] Add framework for tunables 2016-12-30 4:09 [PATCH v7 0/4] glibc tunables Siddhesh Poyarekar @ 2016-12-30 4:09 ` Siddhesh Poyarekar 2017-02-06 22:36 ` Andreas Schwab 2017-02-06 22:59 ` Andreas Schwab 2016-12-30 4:09 ` [PATCH 3/4] Enhance --enable-tunables to select tunables frontend at build time Siddhesh Poyarekar ` (2 subsequent siblings) 3 siblings, 2 replies; 27+ messages in thread From: Siddhesh Poyarekar @ 2016-12-30 4:09 UTC (permalink / raw) To: libc-alpha; +Cc: fweimer, carlos The tunables framework allows us to uniformly manage and expose global variables inside glibc as switches to users. tunables/README has instructions for glibc developers to add new tunables. Tunables support can be enabled by passing the --enable-tunables configure flag to the configure script. This patch only adds a framework and does not pose any limitations on how tunable values are read from the user. It also adds environment variables used in malloc behaviour tweaking to the tunables framework as a PoC of the compatibility interface. * manual/install.texi: Add --enable-tunables option. * INSTALL: Regenerate. * README.tunables: New file. * Makeconfig (CPPFLAGS): Define TOP_NAMESPACE. (before-compile): Generate dl-tunable-list.h early. * config.h.in: Add HAVE_TUNABLES. * config.make.in: Add have-tunables. * configure.ac: Add --enable-tunables option. * configure: Regenerate. * csu/init-first.c (__libc_init_first): Move __libc_init_secure earlier... * csu/init-first.c (LIBC_START_MAIN):... to here. Include dl-tunables.h, libc-internal.h. (LIBC_START_MAIN) [!SHARED]: Initialize tunables for static binaries. * elf/Makefile (dl-routines): Add dl-tunables. * elf/Versions (ld): Add __tunable_set_val to GLIBC_PRIVATE namespace. * elf/dl-support (_dl_nondynamic_init): Unset MALLOC_CHECK_ only when !HAVE_TUNABLES. * elf/rtld.c (process_envvars): Likewise. * elf/dl-sysdep.c [HAVE_TUNABLES]: Include dl-tunables.h (_dl_sysdep_start): Call __tunables_init. * elf/dl-tunable-types.h: New file. * elf/dl-tunables.c: New file. * elf/dl-tunables.h: New file. * elf/dl-tunables.list: New file. * malloc/tst-malloc-usable-static.c: New test case. * malloc/Makefile (tests-static): Add it. * malloc/arena.c [HAVE_TUNABLES]: Include dl-tunables.h. Define TUNABLE_NAMESPACE. (DL_TUNABLE_CALLBACK (set_mallopt_check)): New function. (DL_TUNABLE_CALLBACK_FNDECL): New macro. Use it to define callback functions. (ptmalloc_init): Set tunable values. * scripts/gen-tunables.awk: New file. * sysdeps/mach/hurd/dl-sysdep.c: Include dl-tunables.h. (_dl_sysdep_start): Call __tunables_init. --- INSTALL | 5 + Makeconfig | 16 ++ README.tunables | 85 ++++++++++ config.h.in | 3 + config.make.in | 1 + configure | 16 ++ configure.ac | 10 ++ csu/init-first.c | 2 - csu/libc-start.c | 8 + elf/Makefile | 5 + elf/Versions | 3 + elf/dl-support.c | 2 + elf/dl-sysdep.c | 4 + elf/dl-tunable-types.h | 46 ++++++ elf/dl-tunables.c | 320 ++++++++++++++++++++++++++++++++++++++ elf/dl-tunables.h | 88 +++++++++++ elf/dl-tunables.list | 69 ++++++++ elf/rtld.c | 2 + malloc/Makefile | 2 + malloc/arena.c | 54 +++++++ malloc/tst-malloc-usable-static.c | 1 + manual/install.texi | 5 + scripts/gen-tunables.awk | 157 +++++++++++++++++++ sysdeps/mach/hurd/dl-sysdep.c | 4 + 24 files changed, 906 insertions(+), 2 deletions(-) create mode 100644 README.tunables create mode 100644 elf/dl-tunable-types.h create mode 100644 elf/dl-tunables.c create mode 100644 elf/dl-tunables.h create mode 100644 elf/dl-tunables.list create mode 100644 malloc/tst-malloc-usable-static.c create mode 100644 scripts/gen-tunables.awk diff --git a/INSTALL b/INSTALL index 104f36b..25619fc 100644 --- a/INSTALL +++ b/INSTALL @@ -169,6 +169,11 @@ will be used, and CFLAGS sets optimization options for the compiler. By default for x86_64, the GNU C Library is built with the vector math library. Use this option to disable the vector math library. +'--enable-tunables' + Tunables support allows additional library parameters to be + customized at runtime. This is an experimental feature and affects + startup time and is thus disabled by default. + '--build=BUILD-SYSTEM' '--host=HOST-SYSTEM' These options are for cross-compiling. If you specify both options diff --git a/Makeconfig b/Makeconfig index 0158eaa..b173e4c 100644 --- a/Makeconfig +++ b/Makeconfig @@ -934,6 +934,11 @@ CPPFLAGS = $(config-extra-cppflags) $(CPPUNDEFS) $(CPPFLAGS-config) \ $(foreach lib,$(libof-$(basename $(@F))) \ $(libof-$(<F)) $(libof-$(@F)),$(CPPFLAGS-$(lib))) \ $(CPPFLAGS-$(<F)) $(CPPFLAGS-$(@F)) $(CPPFLAGS-$(basename $(@F))) + +ifeq (yes,$(have-tunables)) +CPPFLAGS += -DTOP_NAMESPACE=glibc +endif + override CFLAGS = -std=gnu11 -fgnu89-inline $(config-extra-cflags) \ $(filter-out %frame-pointer,$(+cflags)) $(+gccwarn-c) \ $(sysdep-CFLAGS) $(CFLAGS-$(suffix $@)) $(CFLAGS-$(<F)) \ @@ -1108,6 +1113,17 @@ $(common-objpfx)libc-modules.stmp: $(..)scripts/gen-libc-modules.awk \ endif +# Build the tunables list header early since it could be used by any module in +# glibc. +ifeq (yes,$(have-tunables)) +before-compile += $(common-objpfx)dl-tunable-list.h + +$(common-objpfx)dl-tunable-list.h: $(..)scripts/gen-tunables.awk \ + $(..)elf/dl-tunables.list + $(AWK) -f $^ > $@.tmp + mv $@.tmp $@ +endif + common-generated += libc-modules.h libc-modules.stmp # The name under which the run-time dynamic linker is installed. diff --git a/README.tunables b/README.tunables new file mode 100644 index 0000000..df74f3b --- /dev/null +++ b/README.tunables @@ -0,0 +1,85 @@ + TUNABLE FRAMEWORK + ================= + +Tunables is a feature in the GNU C Library that allows application authors and +distribution maintainers to alter the runtime library behaviour to match their +workload. + +The tunable framework allows modules within glibc to register variables that +may be tweaked through an environment variable. It aims to enforce a strict +namespace rule to bring consistency to naming of these tunable environment +variables across the project. This document is a guide for glibc developers to +add tunables to the framework. + +ADDING A NEW TUNABLE +-------------------- + +The TOP_NAMESPACE macro is defined by default as 'glibc'. If distributions +intend to add their own tunables, they should do so in a different top +namespace by overriding the TOP_NAMESPACE macro for that tunable. Downstream +implementations are discouraged from using the 'glibc' top namespace for +tunables they don't already have consensus to push upstream. + +There are two steps to adding a tunable: + +1. Add a tunable ID: + +Modules that wish to use the tunables interface must define the +TUNABLE_NAMESPACE macro. Following this, for each tunable you want to +add, make an entry in elf/dl-tunables.list. The format of the file is as +follows: + +TOP_NAMESPACE { + NAMESPACE1 { + TUNABLE1 { + # tunable attributes, one per line + } + # A tunable with default attributes, i.e. string variable. + TUNABLE2 + TUNABLE3 { + # its attributes + } + } + NAMESPACE2 { + ... + } +} + +The list of allowed attributes are: + +- type: Data type. Defaults to STRING. Allowed types are: + INT_32, SIZE_T and STRING. + +- minval: Optional minimum acceptable value. For a string type + this is the minimum length of the value. + +- maxval: Optional maximum acceptable value. For a string type + this is the maximum length of the value. + +- env_alias: An alias environment variable + +- is_secure: Specify whether the tunable should be read for setuid + binaries. True allows the tunable to be read for + setuid binaries while false disables it. Note that + even if this is set as true and the value is read, it + may not be used if it does not validate against the + acceptable values or is not considered safe by the + module. + +2. Call either the TUNABLE_SET_VALUE and pass into it the tunable name and a + pointer to the variable that should be set with the tunable value. + If additional work needs to be done after setting the value, use the + TUNABLE_SET_VALUE_WITH_CALLBACK instead and additionally pass a pointer to + the function that should be called if the tunable value has been set. + +FUTURE WORK +----------- + +The framework currently only allows a one-time initialization of variables +through environment variables and in some cases, modification of variables via +an API call. A future goals for this project include: + +- Setting system-wide and user-wide defaults for tunables through some + mechanism like a configuration file. + +- Allow tweaking of some tunables at runtime diff --git a/config.h.in b/config.h.in index 82f95a6..7bfe923 100644 --- a/config.h.in +++ b/config.h.in @@ -256,4 +256,7 @@ /* PowerPC32 uses fctidz for floating point to long long conversions. */ #define HAVE_PPC_FCTIDZ 0 +/* Build glibc with tunables support. */ +#define HAVE_TUNABLES 0 + #endif diff --git a/config.make.in b/config.make.in index 4422025..2f8dae2 100644 --- a/config.make.in +++ b/config.make.in @@ -96,6 +96,7 @@ use-nscd = @use_nscd@ build-hardcoded-path-in-tests= @hardcoded_path_in_tests@ build-pt-chown = @build_pt_chown@ enable-lock-elision = @enable_lock_elision@ +have-tunables = @have_tunables@ # Build tools. CC = @CC@ diff --git a/configure b/configure index c88f6fe..d80d738 100755 --- a/configure +++ b/configure @@ -666,6 +666,7 @@ libc_cv_ssp base_machine add_on_subdirs add_ons +have_tunables build_pt_chown build_nscd link_obsolete_rpc @@ -782,6 +783,7 @@ enable_systemtap enable_build_nscd enable_nscd enable_pt_chown +enable_tunables enable_mathvec with_cpu ' @@ -1452,6 +1454,7 @@ Optional Features: --disable-build-nscd disable building and installing the nscd daemon --disable-nscd library functions will not contact the nscd daemon --enable-pt_chown Enable building and installing pt_chown + --enable-tunables Enable tunables support --enable-mathvec Enable building and installing mathvec [default depends on architecture] @@ -3698,6 +3701,19 @@ if test "$build_pt_chown" = yes; then fi +# Check whether --enable-tunables was given. +if test "${enable_tunables+set}" = set; then : + enableval=$enable_tunables; have_tunables=$enableval +else + have_tunables=no +fi + + +if test "$have_tunables" = yes; then + $as_echo "#define HAVE_TUNABLES 1" >>confdefs.h + +fi + # The abi-tags file uses a fairly simplistic model for name recognition that # can't distinguish i486-pc-linux-gnu fully from i486-pc-gnu. So we mutate a # $host_os of `gnu*' here to be `gnu-gnu*' just so that it can tell. diff --git a/configure.ac b/configure.ac index 2782bfa..22f5cab 100644 --- a/configure.ac +++ b/configure.ac @@ -421,6 +421,16 @@ if test "$build_pt_chown" = yes; then AC_DEFINE(HAVE_PT_CHOWN) fi +AC_ARG_ENABLE([tunables], + [AS_HELP_STRING([--enable-tunables], + [Enable tunables support])], + [have_tunables=$enableval], + [have_tunables=no]) +AC_SUBST(have_tunables) +if test "$have_tunables" = yes; then + AC_DEFINE(HAVE_TUNABLES) +fi + # The abi-tags file uses a fairly simplistic model for name recognition that # can't distinguish i486-pc-linux-gnu fully from i486-pc-gnu. So we mutate a # $host_os of `gnu*' here to be `gnu-gnu*' just so that it can tell. diff --git a/csu/init-first.c b/csu/init-first.c index 77c6e1c..465f25b 100644 --- a/csu/init-first.c +++ b/csu/init-first.c @@ -72,8 +72,6 @@ _init (int argc, char **argv, char **envp) __environ = envp; #ifndef SHARED - __libc_init_secure (); - /* First the initialization which normally would be done by the dynamic linker. */ _dl_non_dynamic_init (); diff --git a/csu/libc-start.c b/csu/libc-start.c index cc59073..15db9b4 100644 --- a/csu/libc-start.c +++ b/csu/libc-start.c @@ -21,6 +21,9 @@ #include <unistd.h> #include <ldsodefs.h> #include <exit-thread.h> +#include <libc-internal.h> + +#include <elf/dl-tunables.h> extern void __libc_init_first (int argc, char **argv, char **envp); @@ -174,6 +177,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), } } + /* Initialize very early so that tunables can use it. */ + __libc_init_secure (); + + __tunables_init (__environ); + /* Perform IREL{,A} relocations. */ apply_irel (); diff --git a/elf/Makefile b/elf/Makefile index 8a2ce02..de28d99 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -35,6 +35,11 @@ dl-routines = $(addprefix dl-,load lookup object reloc deps hwcaps \ ifeq (yes,$(use-ldconfig)) dl-routines += dl-cache endif + +ifeq (yes,$(have-tunables)) +dl-routines += dl-tunables +endif + all-dl-routines = $(dl-routines) $(sysdep-dl-routines) # But they are absent from the shared libc, because that code is in ld.so. elide-routines.os = $(all-dl-routines) dl-support enbl-secure dl-origin \ diff --git a/elf/Versions b/elf/Versions index 3d57e36..6abe9db 100644 --- a/elf/Versions +++ b/elf/Versions @@ -70,5 +70,8 @@ ld { # Internal error handling support. Interposed by libc.so. _dl_signal_error; _dl_catch_error; + + # Set value of a tunable. + __tunable_set_val; } } diff --git a/elf/dl-support.c b/elf/dl-support.c index c30194c..d350d6d 100644 --- a/elf/dl-support.c +++ b/elf/dl-support.c @@ -354,8 +354,10 @@ _dl_non_dynamic_init (void) cp = (const char *) __rawmemchr (cp, '\0') + 1; } +#if !HAVE_TUNABLES if (__access ("/etc/suid-debug", F_OK) != 0) __unsetenv ("MALLOC_CHECK_"); +#endif } #ifdef DL_PLATFORM_INIT diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c index eaa7155..4283767 100644 --- a/elf/dl-sysdep.c +++ b/elf/dl-sysdep.c @@ -44,6 +44,8 @@ #include <hp-timing.h> #include <tls.h> +#include <dl-tunables.h> + extern char **_environ attribute_hidden; extern char _end[] attribute_hidden; @@ -219,6 +221,8 @@ _dl_sysdep_start (void **start_argptr, } #endif + __tunables_init (_environ); + #ifdef DL_SYSDEP_INIT DL_SYSDEP_INIT; #endif diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h new file mode 100644 index 0000000..d1591b6 --- /dev/null +++ b/elf/dl-tunable-types.h @@ -0,0 +1,46 @@ +/* Tunable type information. + + Copyright (C) 2016 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/>. */ + +#ifndef _TUNABLE_TYPES_H_ +# define _TUNABLE_TYPES_H_ +#include <stddef.h> + +typedef void (*tunable_callback_t) (void *); + +typedef enum +{ + TUNABLE_TYPE_INT_32, + TUNABLE_TYPE_SIZE_T, + TUNABLE_TYPE_STRING +} tunable_type_code_t; + +typedef struct +{ + tunable_type_code_t type_code; + int64_t min; + int64_t max; +} tunable_type_t; + +typedef union +{ + int64_t numval; + const char *strval; +} tunable_val_t; + +#endif diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c new file mode 100644 index 0000000..df60c57 --- /dev/null +++ b/elf/dl-tunables.c @@ -0,0 +1,320 @@ +/* The tunable framework. See the README.tunables to know how to use the + tunable in a glibc module. + + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <stdint.h> +#include <stdbool.h> +#include <unistd.h> +#include <stdlib.h> +#include <libc-internal.h> +#include <sysdep.h> +#include <fcntl.h> +#include <ldsodefs.h> + +#define TUNABLES_INTERNAL 1 +#include "dl-tunables.h" + +/* Compare environment names, bounded by the name hardcoded in glibc. */ +static bool +is_name (const char *orig, const char *envname) +{ + for (;*orig != '\0' && *envname != '\0'; envname++, orig++) + if (*orig != *envname) + break; + + /* The ENVNAME is immediately followed by a value. */ + if (*orig == '\0' && *envname == '=') + return true; + else + return false; +} + +static char ** +get_next_env (char **envp, char **name, size_t *namelen, char **val) +{ + while (envp != NULL && *envp != NULL) + { + char *envline = *envp; + int len = 0; + + while (envline[len] != '\0' && envline[len] != '=') + len++; + + /* Just the name and no value, go to the next one. */ + if (envline[len] == '\0') + continue; + + *name = envline; + *namelen = len; + *val = &envline[len + 1]; + + return ++envp; + } + + return NULL; +} + +static int +tunables_unsetenv (char **ep, const char *name) +{ + while (*ep != NULL) + { + size_t cnt = 0; + + while ((*ep)[cnt] == name[cnt] && name[cnt] != '\0') + ++cnt; + + if (name[cnt] == '\0' && (*ep)[cnt] == '=') + { + /* Found it. Remove this pointer by moving later ones to + the front. */ + char **dp = ep; + + do + dp[0] = dp[1]; + while (*dp++); + /* Continue the loop in case NAME appears again. */ + } + else + ++ep; + } + + return 0; +} + +/* A stripped down strtoul-like implementation for very early use. It does not + set errno if the result is outside bounds because it gets called before + errno may have been set up. */ +static unsigned long int +tunables_strtoul (const char *nptr) +{ + unsigned long int result = 0; + long int sign = 1; + unsigned max_digit; + + while (*nptr == ' ' || *nptr == '\t') + ++nptr; + + if (*nptr == '-') + { + sign = -1; + ++nptr; + } + else if (*nptr == '+') + ++nptr; + + if (*nptr < '0' || *nptr > '9') + return 0UL; + + int base = 10; + max_digit = 9; + if (*nptr == '0') + { + if (nptr[1] == 'x' || nptr[1] == 'X') + { + base = 16; + nptr += 2; + } + else + { + base = 8; + max_digit = 7; + } + } + + while (1) + { + unsigned long int digval; + if (*nptr >= '0' && *nptr <= '0' + max_digit) + digval = *nptr - '0'; + else if (base == 16) + { + if (*nptr >= 'a' && *nptr <= 'f') + digval = *nptr - 'a' + 10; + else if (*nptr >= 'A' && *nptr <= 'F') + digval = *nptr - 'A' + 10; + else + break; + } + else + break; + + if (result > ULONG_MAX / base + || (result == ULONG_MAX / base && digval > ULONG_MAX % base)) + return ULONG_MAX; + result *= base; + result += digval; + ++nptr; + } + + return result * sign; +} + +/* Initialize the internal type if the value validates either using the + explicit constraints of the tunable or with the implicit constraints of its + type. */ +static void +tunable_set_val_if_valid_range (tunable_t *cur, const char *strval, + int64_t default_min, int64_t default_max) +{ + int64_t val = tunables_strtoul (strval); + + int64_t min = cur->type.min; + int64_t max = cur->type.max; + + if (min == max) + { + min = default_min; + max = default_max; + } + + if (val >= min && val <= max) + { + cur->val.numval = val; + cur->strval = strval; + } +} + +/* 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) +{ + switch (cur->type.type_code) + { + case TUNABLE_TYPE_INT_32: + { + tunable_set_val_if_valid_range (cur, strval, INT32_MIN, INT32_MAX); + break; + } + case TUNABLE_TYPE_SIZE_T: + { + tunable_set_val_if_valid_range (cur, strval, 0, SIZE_MAX); + break; + } + case TUNABLE_TYPE_STRING: + { + cur->val.strval = cur->strval = strval; + break; + } + default: + __builtin_trap (); + } +} + +/* Disable a tunable if it is set. */ +static void +disable_tunable (tunable_id_t id, char **envp) +{ + const char *env_alias = tunable_list[id].env_alias; + + if (env_alias != NULL) + tunables_unsetenv (envp, tunable_list[id].env_alias); +} + +/* Disable the glibc.malloc.check tunable in SETUID/SETGID programs unless + the system administrator overrides it by creating 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 inline void +__always_inline +maybe_disable_malloc_check (void) +{ + if (__libc_enable_secure && __access_noerrno ("/etc/suid-debug", F_OK) != 0) + disable_tunable (TUNABLE_ENUM_NAME(glibc, malloc, check), __environ); +} + +/* 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. */ +void +__tunables_init (char **envp) +{ + char *envname = NULL; + char *envval = NULL; + size_t len = 0; + + maybe_disable_malloc_check (); + + while ((envp = get_next_env (envp, &envname, &len, &envval)) != NULL) + { + for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) + { + tunable_t *cur = &tunable_list[i]; + + /* Skip over tunables that have either been set already or should be + skipped. */ + if (cur->strval != NULL || cur->env_alias == NULL + || (__libc_enable_secure && !cur->is_secure)) + continue; + + const char *name = cur->env_alias; + + /* We have a match. Initialize and move on to the next line. */ + if (is_name (name, envname)) + { + tunable_initialize (cur, envval); + break; + } + } + } +} + +/* Set the tunable value. This is called by the module that the tunable exists + in. */ +void +__tunable_set_val (tunable_id_t id, void *valp, tunable_callback_t callback) +{ + tunable_t *cur = &tunable_list[id]; + + /* Don't do anything if our tunable was not set during initialization or if + it failed validation. */ + if (cur->strval == NULL) + return; + + if (valp == NULL) + goto cb; + + switch (cur->type.type_code) + { + case TUNABLE_TYPE_INT_32: + { + *((int32_t *) valp) = (int32_t) cur->val.numval; + break; + } + case TUNABLE_TYPE_SIZE_T: + { + *((size_t *) valp) = (size_t) cur->val.numval; + break; + } + case TUNABLE_TYPE_STRING: + { + *((const char **)valp) = cur->val.strval; + break; + } + default: + __builtin_unreachable (); + } + +cb: + if (callback) + callback (&cur->val); +} diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h new file mode 100644 index 0000000..7762c46 --- /dev/null +++ b/elf/dl-tunables.h @@ -0,0 +1,88 @@ +/* The tunable framework. See the README to know how to use the tunable in + a glibc module. + + Copyright (C) 2016 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/>. */ + +#ifndef _TUNABLES_H_ +#define _TUNABLES_H_ + +# if !HAVE_TUNABLES +static inline void +__always_inline +__tunables_init (char **unused __attribute_unused) +{ + return; +} +# else + +# include <stddef.h> +# include "dl-tunable-types.h" + +/* A tunable. */ +struct _tunable +{ + const char *name; /* Internal name of the tunable. */ + tunable_type_t type; /* Data type of the tunable. */ + tunable_val_t val; /* The value. */ + const char *strval; /* The string containing the value, + points into envp. */ + bool is_secure; /* Whether the tunable must be read + even for setuid binaries. 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; /* The compatibility environment + variable name. */ +}; + +typedef struct _tunable tunable_t; + +/* Full name for a tunable is top_ns.tunable_ns.id. */ +# define TUNABLE_NAME_S(top,ns,id) #top "." #ns "." #id + +# define TUNABLE_ENUM_NAME(top,ns,id) TUNABLE_ENUM_NAME1 (top,ns,id) +# define TUNABLE_ENUM_NAME1(top,ns,id) top ## _ ## ns ## _ ## id + +# include "dl-tunable-list.h" + +extern void __tunables_init (char **); +extern void __tunable_set_val (tunable_id_t, void *, tunable_callback_t); + +/* Check if the tunable has been set to a non-default value and if it is, copy + it over into __VAL. */ +# define TUNABLE_SET_VAL(__id,__val) \ +({ \ + __tunable_set_val \ + (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val), \ + NULL); \ +}) + +/* Same as TUNABLE_SET_VAL, but also call the callback function __CB. */ +# define TUNABLE_SET_VAL_WITH_CALLBACK(__id,__val,__cb) \ +({ \ + __tunable_set_val \ + (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val), \ + DL_TUNABLE_CALLBACK (__cb)); \ +}) + +/* Namespace sanity for callback functions. Use this macro to keep the + namespace of the modules clean. */ +# define DL_TUNABLE_CALLBACK(__name) _dl_tunable_ ## __name +# endif +#endif diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list new file mode 100644 index 0000000..11504c4 --- /dev/null +++ b/elf/dl-tunables.list @@ -0,0 +1,69 @@ +# Copyright (C) 2016 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/>. + +# Allowed attributes for tunables: +# +# type: Defaults to STRING +# minval: Optional minimum acceptable value +# maxval: Optional maximum acceptable value +# env_alias: An alias environment variable +# is_secure: Specify whether the environment variable should be read for +# setuid binaries. + +glibc { + malloc { + check { + type: INT_32 + minval: 0 + maxval: 3 + env_alias: MALLOC_CHECK_ + is_secure: true + } + top_pad { + type: SIZE_T + env_alias: MALLOC_TOP_PAD_ + } + perturb { + type: INT_32 + minval: 0 + maxval: 0xff + env_alias: MALLOC_PERTURB_ + } + mmap_threshold { + type: SIZE_T + env_alias: MALLOC_MMAP_THRESHOLD_ + } + trim_threshold { + type: SIZE_T + env_alias: MALLOC_TRIM_THRESHOLD_ + } + mmap_max { + type: INT_32 + env_alias: MALLOC_MMAP_MAX_ + } + arena_max { + type: SIZE_T + env_alias: MALLOC_ARENA_MAX + minval: 1 + } + arena_test { + type: SIZE_T + env_alias: MALLOC_ARENA_TEST + minval: 1 + } + } +} diff --git a/elf/rtld.c b/elf/rtld.c index 4ec25d7..a60ead6 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -2510,7 +2510,9 @@ process_envvars (enum mode *modep) if (__access ("/etc/suid-debug", F_OK) != 0) { +#if !HAVE_TUNABLES unsetenv ("MALLOC_CHECK_"); +#endif GLRO(dl_debug_mask) = 0; } diff --git a/malloc/Makefile b/malloc/Makefile index b8efcd6..4e4104e 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -37,6 +37,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ tests-static := \ tst-interpose-static-nothread \ tst-interpose-static-thread \ + tst-malloc-usable-static tests += $(tests-static) test-srcs = tst-mtrace @@ -158,6 +159,7 @@ endif tst-mcheck-ENV = MALLOC_CHECK_=3 tst-malloc-usable-ENV = MALLOC_CHECK_=3 +tst-malloc-usable-static-ENV = $(tst-malloc-usable-ENV) # Uncomment this for test releases. For public releases it is too expensive. #CPPFLAGS-malloc.o += -DMALLOC_DEBUG=1 diff --git a/malloc/arena.c b/malloc/arena.c index eed4247..234035f 100644 --- a/malloc/arena.c +++ b/malloc/arena.c @@ -19,6 +19,11 @@ #include <stdbool.h> +#if HAVE_TUNABLES +# define TUNABLE_NAMESPACE malloc +#endif +#include <elf/dl-tunables.h> + /* Compile-time constants. */ #define HEAP_MIN_SIZE (32 * 1024) @@ -204,6 +209,34 @@ __malloc_fork_unlock_child (void) __libc_lock_init (list_lock); } +#if HAVE_TUNABLES +static inline int do_set_mallopt_check (int32_t value); +void +DL_TUNABLE_CALLBACK (set_mallopt_check) (void *valp) +{ + int32_t value = *(int32_t *) valp; + do_set_mallopt_check (value); + if (check_action != 0) + __malloc_check_init (); +} + +# define DL_TUNABLE_CALLBACK_FNDECL(__name, __type) \ +static inline int do_ ## __name (__type value); \ +void \ +DL_TUNABLE_CALLBACK (__name) (void *valp) \ +{ \ + __type value = *(__type *) valp; \ + do_ ## __name (value); \ +} + +DL_TUNABLE_CALLBACK_FNDECL (set_mmap_threshold, size_t) +DL_TUNABLE_CALLBACK_FNDECL (set_mmaps_max, int32_t) +DL_TUNABLE_CALLBACK_FNDECL (set_top_pad, size_t) +DL_TUNABLE_CALLBACK_FNDECL (set_perturb_byte, int32_t) +DL_TUNABLE_CALLBACK_FNDECL (set_trim_threshold, size_t) +DL_TUNABLE_CALLBACK_FNDECL (set_arena_max, size_t) +DL_TUNABLE_CALLBACK_FNDECL (set_arena_test, size_t) +#else /* Initialization routine. */ #include <string.h> extern char **_environ; @@ -238,6 +271,7 @@ next_env_entry (char ***position) return result; } +#endif #ifdef SHARED @@ -272,6 +306,24 @@ ptmalloc_init (void) #endif thread_arena = &main_arena; + +#if HAVE_TUNABLES + /* Ensure initialization/consolidation and do it under a lock so that a + thread attempting to use the arena in parallel waits on us till we + finish. */ + __libc_lock_lock (main_arena.mutex); + malloc_consolidate (&main_arena); + + TUNABLE_SET_VAL_WITH_CALLBACK (check, NULL, set_mallopt_check); + TUNABLE_SET_VAL_WITH_CALLBACK (top_pad, NULL, set_top_pad); + TUNABLE_SET_VAL_WITH_CALLBACK (perturb, NULL, set_perturb_byte); + TUNABLE_SET_VAL_WITH_CALLBACK (mmap_threshold, NULL, set_mmap_threshold); + TUNABLE_SET_VAL_WITH_CALLBACK (trim_threshold, NULL, set_trim_threshold); + TUNABLE_SET_VAL_WITH_CALLBACK (mmap_max, NULL, set_mmaps_max); + TUNABLE_SET_VAL_WITH_CALLBACK (arena_max, NULL, set_arena_max); + TUNABLE_SET_VAL_WITH_CALLBACK (arena_test, NULL, set_arena_test); + __libc_lock_unlock (main_arena.mutex); +#else const char *s = NULL; if (__glibc_likely (_environ != NULL)) { @@ -340,6 +392,8 @@ ptmalloc_init (void) if (check_action != 0) __malloc_check_init (); } +#endif + #if HAVE_MALLOC_INIT_HOOK void (*hook) (void) = atomic_forced_read (__malloc_initialize_hook); if (hook != NULL) diff --git a/malloc/tst-malloc-usable-static.c b/malloc/tst-malloc-usable-static.c new file mode 100644 index 0000000..8907db0 --- /dev/null +++ b/malloc/tst-malloc-usable-static.c @@ -0,0 +1 @@ +#include <malloc/tst-malloc-usable.c> diff --git a/manual/install.texi b/manual/install.texi index d02e870..d412962 100644 --- a/manual/install.texi +++ b/manual/install.texi @@ -200,6 +200,11 @@ configure with @option{--disable-werror}. By default for x86_64, @theglibc{} is built with the vector math library. Use this option to disable the vector math library. +@item --enable-tunables +Tunables support allows additional library parameters to be customized at +runtime. This is an experimental feature and affects startup time and is thus +disabled by default. + @item --build=@var{build-system} @itemx --host=@var{host-system} These options are for cross-compiling. If you specify both options and diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk new file mode 100644 index 0000000..b65b5a4 --- /dev/null +++ b/scripts/gen-tunables.awk @@ -0,0 +1,157 @@ +# Generate dl-tunable-list.h from dl-tunables.list + +BEGIN { + tunable="" + ns="" + top_ns="" +} + +# Skip over blank lines and comments. +/^#/ { + next +} + +/^[ \t]*$/ { + next +} + +# Beginning of either a top namespace, tunable namespace or a tunable, decided +# on the current value of TUNABLE, NS or TOP_NS. +$2 == "{" { + if (top_ns == "") { + top_ns = $1 + } + else if (ns == "") { + ns = $1 + } + else if (tunable == "") { + tunable = $1 + } + else { + printf ("Unexpected occurrence of '{': %s:%d\n", FILENAME, FNR) + exit 1 + } + + next +} + +# End of either a top namespace, tunable namespace or a tunable. +$1 == "}" { + if (tunable != "") { + # Tunables definition ended, now fill in default attributes. + if (!types[top_ns][ns][tunable]) { + types[top_ns][ns][tunable] = "STRING" + } + if (!minvals[top_ns][ns][tunable]) { + minvals[top_ns][ns][tunable] = "0" + } + if (!maxvals[top_ns][ns][tunable]) { + maxvals[top_ns][ns][tunable] = "0" + } + if (!env_alias[top_ns][ns][tunable]) { + env_alias[top_ns][ns][tunable] = "NULL" + } + if (!is_secure[top_ns][ns][tunable]) { + is_secure[top_ns][ns][tunable] = "false" + } + + tunable = "" + } + else if (ns != "") { + ns = "" + } + else if (top_ns != "") { + top_ns = "" + } + else { + printf ("syntax error: extra }: %s:%d\n", FILENAME, FNR) + exit 1 + } + next +} + +# Everything else, which could either be a tunable without any attributes or a +# tunable attribute. +{ + if (ns == "") { + printf("Line %d: Invalid tunable outside a namespace: %s\n", NR, $0) + exit 1 + } + + if (tunable == "") { + # We encountered a tunable without any attributes, so note it with a + # default. + types[top_ns][ns][$1] = "STRING" + next + } + + # Otherwise, we have encountered a tunable attribute. + split($0, arr, ":") + attr = gensub(/^[ \t]+|[ \t]+$/, "", "g", arr[1]) + val = gensub(/^[ \t]+|[ \t]+$/, "", "g", arr[2]) + + if (attr == "type") { + types[top_ns][ns][tunable] = val + } + else if (attr == "minval") { + minvals[top_ns][ns][tunable] = val + } + else if (attr == "maxval") { + maxvals[top_ns][ns][tunable] = val + } + else if (attr == "env_alias") { + env_alias[top_ns][ns][tunable] = sprintf("\"%s\"", val) + } + else if (attr == "is_secure") { + if (val == "true" || val == "false") { + is_secure[top_ns][ns][tunable] = val + } + else { + printf("Line %d: Invalid value (%s) for is_secure: %s, ", NR, val, + $0) + print("Allowed values are 'true' or 'false'") + exit 1 + } + } +} + +END { + if (ns != "") { + print "Unterminated namespace. Is a closing brace missing?" + exit 1 + } + + print "/* AUTOGENERATED by gen-tunables.awk. */" + print "#ifndef _TUNABLES_H_" + print "# error \"Do not include this file directly.\"" + print "# error \"Include tunables.h instead.\"" + print "#endif" + + # Now, the enum names + print "\ntypedef enum" + print "{" + for (t in types) { + for (n in types[t]) { + for (m in types[t][n]) { + printf (" TUNABLE_ENUM_NAME(%s, %s, %s),\n", t, n, m); + } + } + } + print "} tunable_id_t;\n" + + # Finally, the tunable list. + print "\n#ifdef TUNABLES_INTERNAL" + print "static tunable_t tunable_list[] = {" + for (t in types) { + for (n in types[t]) { + for (m in types[t][n]) { + printf (" {TUNABLE_NAME_S(%s, %s, %s)", t, n, m) + printf (", {TUNABLE_TYPE_%s, %s, %s}, {.numval = 0}, NULL, %s, %s},\n", + types[t][n][m], minvals[t][n][m], maxvals[t][n][m], + is_secure[t][n][m], env_alias[t][n][m]); + } + } + } + print "};" + print "#endif" +} diff --git a/sysdeps/mach/hurd/dl-sysdep.c b/sysdeps/mach/hurd/dl-sysdep.c index d730f82..42bccda 100644 --- a/sysdeps/mach/hurd/dl-sysdep.c +++ b/sysdeps/mach/hurd/dl-sysdep.c @@ -44,6 +44,8 @@ #include <dl-machine.h> #include <dl-procinfo.h> +#include <dl-tunables.h> + extern void __mach_init (void); extern int _dl_argc; @@ -143,6 +145,8 @@ _dl_sysdep_start (void **start_argptr, __libc_enable_secure = _dl_hurd_data->flags & EXEC_SECURE; + __tunables_init (_environ); + if (_dl_hurd_data->flags & EXEC_STACK_ARGS && _dl_hurd_data->user_entry == 0) _dl_hurd_data->user_entry = (vm_address_t) ENTRY_POINT; -- 2.7.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2016-12-30 4:09 ` [PATCH 1/4] Add framework for tunables Siddhesh Poyarekar @ 2017-02-06 22:36 ` Andreas Schwab 2017-02-07 6:49 ` Siddhesh Poyarekar 2017-02-06 22:59 ` Andreas Schwab 1 sibling, 1 reply; 27+ messages in thread From: Andreas Schwab @ 2017-02-06 22:36 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: libc-alpha, fweimer, carlos On Dez 30 2016, Siddhesh Poyarekar <siddhesh@sourceware.org> wrote: > diff --git a/malloc/arena.c b/malloc/arena.c > index eed4247..234035f 100644 > --- a/malloc/arena.c > +++ b/malloc/arena.c > @@ -19,6 +19,11 @@ > > #include <stdbool.h> > > +#if HAVE_TUNABLES > +# define TUNABLE_NAMESPACE malloc > +#endif > +#include <elf/dl-tunables.h> > + > /* Compile-time constants. */ > > #define HEAP_MIN_SIZE (32 * 1024) > @@ -204,6 +209,34 @@ __malloc_fork_unlock_child (void) > __libc_lock_init (list_lock); > } > > +#if HAVE_TUNABLES > +static inline int do_set_mallopt_check (int32_t value); > +void > +DL_TUNABLE_CALLBACK (set_mallopt_check) (void *valp) > +{ > + int32_t value = *(int32_t *) valp; This is wrong. The callback receives a pointer to tunable_val_t. > + do_set_mallopt_check (value); > + if (check_action != 0) > + __malloc_check_init (); > +} > + > +# define DL_TUNABLE_CALLBACK_FNDECL(__name, __type) \ > +static inline int do_ ## __name (__type value); \ > +void \ > +DL_TUNABLE_CALLBACK (__name) (void *valp) \ > +{ \ > + __type value = *(__type *) valp; \ Likewise. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2017-02-06 22:36 ` Andreas Schwab @ 2017-02-07 6:49 ` Siddhesh Poyarekar 2017-02-07 16:06 ` Andreas Schwab 0 siblings, 1 reply; 27+ messages in thread From: Siddhesh Poyarekar @ 2017-02-07 6:49 UTC (permalink / raw) To: Andreas Schwab; +Cc: libc-alpha, fweimer, carlos [-- Attachment #1: Type: text/plain, Size: 959 bytes --] On Tuesday 07 February 2017 04:06 AM, Andreas Schwab wrote: > On Dez 30 2016, Siddhesh Poyarekar <siddhesh@sourceware.org> wrote: > >> diff --git a/malloc/arena.c b/malloc/arena.c >> index eed4247..234035f 100644 >> --- a/malloc/arena.c >> +++ b/malloc/arena.c >> @@ -19,6 +19,11 @@ >> >> #include <stdbool.h> >> >> +#if HAVE_TUNABLES >> +# define TUNABLE_NAMESPACE malloc >> +#endif >> +#include <elf/dl-tunables.h> >> + >> /* Compile-time constants. */ >> >> #define HEAP_MIN_SIZE (32 * 1024) >> @@ -204,6 +209,34 @@ __malloc_fork_unlock_child (void) >> __libc_lock_init (list_lock); >> } >> >> +#if HAVE_TUNABLES >> +static inline int do_set_mallopt_check (int32_t value); >> +void >> +DL_TUNABLE_CALLBACK (set_mallopt_check) (void *valp) >> +{ >> + int32_t value = *(int32_t *) valp; > > This is wrong. The callback receives a pointer to tunable_val_t. Does the attached (sanity tested on little-endian) patch work for you? Siddhesh [-- Attachment #2: patch --] [-- Type: text/plain, Size: 2002 bytes --] diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h index a986f0b..37a4e80 100644 --- a/elf/dl-tunable-types.h +++ b/elf/dl-tunable-types.h @@ -21,8 +21,6 @@ # define _TUNABLE_TYPES_H_ #include <stddef.h> -typedef void (*tunable_callback_t) (void *); - typedef enum { TUNABLE_TYPE_INT_32, @@ -43,6 +41,8 @@ typedef union const char *strval; } tunable_val_t; +typedef void (*tunable_callback_t) (tunable_val_t *); + /* Security level for tunables. This decides what to do with individual tunables for AT_SECURE binaries. */ typedef enum diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index a8d53d6..e42aa67 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -455,6 +455,8 @@ __tunable_set_val (tunable_id_t id, void *valp, tunable_callback_t callback) if (cur->strval == NULL) return; + /* Caller does not need the value, just call the callback with our tunable + value. */ if (valp == NULL) goto cb; diff --git a/malloc/arena.c b/malloc/arena.c index b91d7d6..d49e4a2 100644 --- a/malloc/arena.c +++ b/malloc/arena.c @@ -212,9 +212,9 @@ __malloc_fork_unlock_child (void) #if HAVE_TUNABLES static inline int do_set_mallopt_check (int32_t value); void -DL_TUNABLE_CALLBACK (set_mallopt_check) (void *valp) +DL_TUNABLE_CALLBACK (set_mallopt_check) (tunable_val_t *valp) { - int32_t value = *(int32_t *) valp; + int32_t value = (int32_t) valp->numval; do_set_mallopt_check (value); if (check_action != 0) __malloc_check_init (); @@ -223,9 +223,9 @@ DL_TUNABLE_CALLBACK (set_mallopt_check) (void *valp) # define DL_TUNABLE_CALLBACK_FNDECL(__name, __type) \ static inline int do_ ## __name (__type value); \ void \ -DL_TUNABLE_CALLBACK (__name) (void *valp) \ +DL_TUNABLE_CALLBACK (__name) (tunable_val_t *valp) \ { \ - __type value = *(__type *) valp; \ + __type value = (__type) (valp)->numval; \ do_ ## __name (value); \ } ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2017-02-07 6:49 ` Siddhesh Poyarekar @ 2017-02-07 16:06 ` Andreas Schwab 2017-02-08 8:48 ` Siddhesh Poyarekar 0 siblings, 1 reply; 27+ messages in thread From: Andreas Schwab @ 2017-02-07 16:06 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: libc-alpha, fweimer, carlos On Feb 07 2017, Siddhesh Poyarekar <siddhesh@sourceware.org> wrote: > Does the attached (sanity tested on little-endian) patch work for you? https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:Factory/glibc-testsuite/p/ppc https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:Factory/glibc-testsuite/p/ppc64 https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:Factory/glibc-testsuite/s/s390x All four tst-malloc-usable tests are now passing. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2017-02-07 16:06 ` Andreas Schwab @ 2017-02-08 8:48 ` Siddhesh Poyarekar 0 siblings, 0 replies; 27+ messages in thread From: Siddhesh Poyarekar @ 2017-02-08 8:48 UTC (permalink / raw) To: Andreas Schwab; +Cc: libc-alpha, fweimer, carlos On Tuesday 07 February 2017 09:35 PM, Andreas Schwab wrote: > On Feb 07 2017, Siddhesh Poyarekar <siddhesh@sourceware.org> wrote: > >> Does the attached (sanity tested on little-endian) patch work for you? > > https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:Factory/glibc-testsuite/p/ppc > https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:Factory/glibc-testsuite/p/ppc64 > https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:Factory/glibc-testsuite/s/s390x > > All four tst-malloc-usable tests are now passing. Thanks, I have pushed the patch now. Siddhesh ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2016-12-30 4:09 ` [PATCH 1/4] Add framework for tunables Siddhesh Poyarekar 2017-02-06 22:36 ` Andreas Schwab @ 2017-02-06 22:59 ` Andreas Schwab 1 sibling, 0 replies; 27+ messages in thread From: Andreas Schwab @ 2017-02-06 22:59 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: libc-alpha, fweimer, carlos On Dez 30 2016, Siddhesh Poyarekar <siddhesh@sourceware.org> wrote: > +/* Set the tunable value. This is called by the module that the tunable exists > + in. */ > +void > +__tunable_set_val (tunable_id_t id, void *valp, tunable_callback_t callback) > +{ > + tunable_t *cur = &tunable_list[id]; > + > + /* Don't do anything if our tunable was not set during initialization or if > + it failed validation. */ > + if (cur->strval == NULL) > + return; > + > + if (valp == NULL) > + goto cb; Why goto? Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/4] Enhance --enable-tunables to select tunables frontend at build time 2016-12-30 4:09 [PATCH v7 0/4] glibc tunables Siddhesh Poyarekar 2016-12-30 4:09 ` [PATCH 1/4] Add framework for tunables Siddhesh Poyarekar @ 2016-12-30 4:09 ` Siddhesh Poyarekar 2016-12-30 14:55 ` Joseph Myers 2016-12-30 4:09 ` [PATCH 4/4] User manual documentation for tunables Siddhesh Poyarekar 2016-12-30 4:09 ` [PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar 3 siblings, 1 reply; 27+ messages in thread From: Siddhesh Poyarekar @ 2016-12-30 4:09 UTC (permalink / raw) To: libc-alpha; +Cc: fweimer, carlos At the GNU Tools Cauldron 2016, the state of the current tunables patchset was considered OK with the addition of a way to select the frontend to be used for the tunables. That is, to avoid being locked in to one type of frontend initially, it should be possible to build tunables with a different frontend with something as simple as a configure switch. To that effect, this patch enhances the --enable-tunables option to accept more values than just 'yes' or 'no'. The current frontend (and default when enable-tunables is 'yes') is called 'valstring', to select the frontend where a single environment variable is set to a colon-separated value string. More such frontends can be added in future. * Makeconfig (have-tunables): Check for non-negative instead of positive. * configure.ac: Add 'valstring' as a valid value for --enable-tunables. * configure: Regenerate. * elf/Makefile (have-tunables): Check for non-negative instead of positive. (CPPFLAGS-dl-tunables.c): Define TUNABLES_FRONTEND for dl-tunables.c. * elf/dl-tunables.c (GLIBC_TUNABLES): Define only when TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring. (tunables_strdup): Likewise. (disable_tunables): Likewise. (parse_tunables): Likewise. (__tunables_init): Process GLIBC_TUNABLES envvar only when. TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring. * elf/dl-tunables.h (TUNABLES_FRONTEND_valstring): New macro. (TUNABLES_FRONTEND_yes): New macro, define as TUNABLES_FRONTEND_valstring by default. * manual/install.texi: Document new acceptable values for --enable-tunables. * INSTALL: Regenerate. --- INSTALL | 15 ++++++++++++++- Makeconfig | 4 ++-- configure | 3 ++- configure.ac | 2 +- elf/Makefile | 4 +++- elf/dl-tunables.c | 12 +++++++++++- elf/dl-tunables.h | 4 ++++ manual/install.texi | 18 +++++++++++++++++- 8 files changed, 54 insertions(+), 8 deletions(-) diff --git a/INSTALL b/INSTALL index 25619fc..b023b38 100644 --- a/INSTALL +++ b/INSTALL @@ -172,7 +172,20 @@ will be used, and CFLAGS sets optimization options for the compiler. '--enable-tunables' Tunables support allows additional library parameters to be customized at runtime. This is an experimental feature and affects - startup time and is thus disabled by default. + startup time and is thus disabled by default. This option can take + the following values: + + * NO This is the default if the option is not passed to + configure. This disables tunables. + + * YES This is the default if the option is passed to configure. + This enables tunables and selects the default frontend + (currently VALSTRING). + + * VALSTRING This enables tunables and selects the VALSTRING + frontend for tunables. This frontend allows users to specify + tunables as a colon-separated list in a single environment + variable GLIBC_TUNABLES. '--build=BUILD-SYSTEM' '--host=HOST-SYSTEM' diff --git a/Makeconfig b/Makeconfig index b173e4c..1a2db6d 100644 --- a/Makeconfig +++ b/Makeconfig @@ -935,7 +935,7 @@ CPPFLAGS = $(config-extra-cppflags) $(CPPUNDEFS) $(CPPFLAGS-config) \ $(libof-$(<F)) $(libof-$(@F)),$(CPPFLAGS-$(lib))) \ $(CPPFLAGS-$(<F)) $(CPPFLAGS-$(@F)) $(CPPFLAGS-$(basename $(@F))) -ifeq (yes,$(have-tunables)) +ifneq (no,$(have-tunables)) CPPFLAGS += -DTOP_NAMESPACE=glibc endif @@ -1115,7 +1115,7 @@ endif # Build the tunables list header early since it could be used by any module in # glibc. -ifeq (yes,$(have-tunables)) +ifneq (no,$(have-tunables)) before-compile += $(common-objpfx)dl-tunable-list.h $(common-objpfx)dl-tunable-list.h: $(..)scripts/gen-tunables.awk \ diff --git a/configure b/configure index d80d738..eecd0ac 100755 --- a/configure +++ b/configure @@ -1454,7 +1454,8 @@ Optional Features: --disable-build-nscd disable building and installing the nscd daemon --disable-nscd library functions will not contact the nscd daemon --enable-pt_chown Enable building and installing pt_chown - --enable-tunables Enable tunables support + --enable-tunables Enable tunables support. Known values are 'yes', + 'no' and 'valstring' --enable-mathvec Enable building and installing mathvec [default depends on architecture] diff --git a/configure.ac b/configure.ac index 22f5cab..4a77411 100644 --- a/configure.ac +++ b/configure.ac @@ -423,7 +423,7 @@ fi AC_ARG_ENABLE([tunables], [AS_HELP_STRING([--enable-tunables], - [Enable tunables support])], + [Enable tunables support. Known values are 'yes', 'no' and 'valstring'])], [have_tunables=$enableval], [have_tunables=no]) AC_SUBST(have_tunables) diff --git a/elf/Makefile b/elf/Makefile index 3cda2c9..c3636a6 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -36,8 +36,10 @@ ifeq (yes,$(use-ldconfig)) dl-routines += dl-cache endif -ifeq (yes,$(have-tunables)) +ifneq (no,$(have-tunables)) dl-routines += dl-tunables +tunables-type = $(addprefix TUNABLES_FRONTEND_,$(have-tunables)) +CPPFLAGS-dl-tunables.c = -DTUNABLES_FRONTEND=$(tunables-type) # Make sure that the compiler does not insert any library calls in tunables # code paths. diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index d41b6dc..0277964 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -30,7 +30,9 @@ #define TUNABLES_INTERNAL 1 #include "dl-tunables.h" -#define GLIBC_TUNABLES "GLIBC_TUNABLES" +#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring +# define GLIBC_TUNABLES "GLIBC_TUNABLES" +#endif /* Compare environment or tunable names, bounded by the name hardcoded in glibc. */ @@ -48,6 +50,7 @@ is_name (const char *orig, const char *envname) return false; } +#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring static char * tunables_strdup (const char *in) { @@ -70,6 +73,7 @@ tunables_strdup (const char *in) return out; } +#endif static char ** get_next_env (char **envp, char **name, size_t *namelen, char **val) @@ -244,6 +248,7 @@ tunable_initialize (tunable_t *cur, const char *strval) } } +#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring static void parse_tunables (char *tunestr) { @@ -309,6 +314,7 @@ parse_tunables (char *tunestr) return; } } +#endif static size_t min_strlen (const char *s) @@ -329,6 +335,7 @@ disable_tunable (tunable_id_t id, char **envp) if (env_alias != NULL) tunables_unsetenv (envp, tunable_list[id].env_alias); +#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring char *tunable = getenv (GLIBC_TUNABLES); const char *cmp = tunable_list[id].name; const size_t len = min_strlen (cmp); @@ -345,6 +352,7 @@ disable_tunable (tunable_id_t id, char **envp) } tunable++; } +#endif } /* Disable the glibc.malloc.check tunable in SETUID/SETGID programs unless @@ -375,6 +383,7 @@ __tunables_init (char **envp) while ((envp = get_next_env (envp, &envname, &len, &envval)) != NULL) { +#if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring if (is_name (GLIBC_TUNABLES, envname)) { char *val = tunables_strdup (envval); @@ -382,6 +391,7 @@ __tunables_init (char **envp) parse_tunables (val); continue; } +#endif for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) { diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h index 7762c46..785a9b0 100644 --- a/elf/dl-tunables.h +++ b/elf/dl-tunables.h @@ -84,5 +84,9 @@ extern void __tunable_set_val (tunable_id_t, void *, tunable_callback_t); /* Namespace sanity for callback functions. Use this macro to keep the namespace of the modules clean. */ # define DL_TUNABLE_CALLBACK(__name) _dl_tunable_ ## __name + +# define TUNABLES_FRONTEND_valstring 1 +/* The default value for TUNABLES_FRONTEND. */ +# define TUNABLES_FRONTEND_yes TUNABLES_FRONTEND_valstring # endif #endif diff --git a/manual/install.texi b/manual/install.texi index d412962..321e68b 100644 --- a/manual/install.texi +++ b/manual/install.texi @@ -203,7 +203,23 @@ Use this option to disable the vector math library. @item --enable-tunables Tunables support allows additional library parameters to be customized at runtime. This is an experimental feature and affects startup time and is thus -disabled by default. +disabled by default. This option can take the following values: + +@itemize @bullet +@item @var{no} +This is the default if the option is not passed to configure. This disables +tunables. + +@item @var{yes} +This is the default if the option is passed to configure. This enables tunables +and selects the default frontend (currently @var{valstring}). + +@item @var{valstring} +This enables tunables and selects the @var{valstring} frontend for tunables. +This frontend allows users to specify tunables as a colon-separated list in a +single environment variable @var{GLIBC_TUNABLES}. + +@end itemize @item --build=@var{build-system} @itemx --host=@var{host-system} -- 2.7.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] Enhance --enable-tunables to select tunables frontend at build time 2016-12-30 4:09 ` [PATCH 3/4] Enhance --enable-tunables to select tunables frontend at build time Siddhesh Poyarekar @ 2016-12-30 14:55 ` Joseph Myers 0 siblings, 0 replies; 27+ messages in thread From: Joseph Myers @ 2016-12-30 14:55 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: libc-alpha, fweimer, carlos On Fri, 30 Dec 2016, Siddhesh Poyarekar wrote: > +@itemize @bullet > +@item @var{no} > +This is the default if the option is not passed to configure. This disables > +tunables. > + > +@item @var{yes} > +This is the default if the option is passed to configure. This enables tunables > +and selects the default frontend (currently @var{valstring}). > + > +@item @var{valstring} > +This enables tunables and selects the @var{valstring} frontend for tunables. > +This frontend allows users to specify tunables as a colon-separated list in a > +single environment variable @var{GLIBC_TUNABLES}. None of these uses of @var is correct. @var is for a metasyntactic variable. It is not for environment variables (use @env), and nor is it for literal text (note how the contents of @var get put in uppercase in plain text / info output such as the INSTALL file). You probably want "@table @code" for the list of possible configure option arguments, with no @var{} or other markup on the individual @item lines (and then use @samp{valstring} for references to it in the text). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 4/4] User manual documentation for tunables 2016-12-30 4:09 [PATCH v7 0/4] glibc tunables Siddhesh Poyarekar 2016-12-30 4:09 ` [PATCH 1/4] Add framework for tunables Siddhesh Poyarekar 2016-12-30 4:09 ` [PATCH 3/4] Enhance --enable-tunables to select tunables frontend at build time Siddhesh Poyarekar @ 2016-12-30 4:09 ` Siddhesh Poyarekar 2016-12-30 9:50 ` Florian Weimer 2016-12-30 4:09 ` [PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar 3 siblings, 1 reply; 27+ messages in thread From: Siddhesh Poyarekar @ 2016-12-30 4:09 UTC (permalink / raw) To: libc-alpha; +Cc: fweimer, carlos Create a new node for tunables documentation and add notes for the malloc tunables. * manual/tunables.texi: New chapter. * manual/Makefile (chapters): Add it. * manual/probes.texi (@node): Point to the Tunables chapter. --- manual/Makefile | 3 +- manual/probes.texi | 2 +- manual/tunables.texi | 185 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 188 insertions(+), 2 deletions(-) create mode 100644 manual/tunables.texi diff --git a/manual/Makefile b/manual/Makefile index f2f694f..ecc2bf6 100644 --- a/manual/Makefile +++ b/manual/Makefile @@ -38,7 +38,8 @@ chapters = $(addsuffix .texi, \ message search pattern io stdio llio filesys \ pipe socket terminal syslog math arith time \ resource setjmp signal startup process ipc job \ - nss users sysinfo conf crypt debug threads probes) + nss users sysinfo conf crypt debug threads \ + probes tunables) add-chapters = $(wildcard $(foreach d, $(add-ons), ../$d/$d.texi)) appendices = lang.texi header.texi install.texi maint.texi platform.texi \ contrib.texi diff --git a/manual/probes.texi b/manual/probes.texi index 237a918..eb91c62 100644 --- a/manual/probes.texi +++ b/manual/probes.texi @@ -1,5 +1,5 @@ @node Internal Probes -@c @node Internal Probes, , POSIX Threads, Top +@c @node Internal Probes, Tunables, POSIX Threads, Top @c %MENU% Probes to monitor libc internal behavior @chapter Internal probes diff --git a/manual/tunables.texi b/manual/tunables.texi new file mode 100644 index 0000000..1f1f9fc --- /dev/null +++ b/manual/tunables.texi @@ -0,0 +1,185 @@ +@node Tunables +@c @node Tunables, , Internal Probes, Top +@c %MENU% Tunable switches to alter libc internal behavior +@chapter Tunables + +Tunables is a feature in @theglibc{} that allows application authors and +distribution maintainers to alter the runtime library behaviour to match +their workload. These are implemented as a set of switches that may be +modified in different ways. The current default method to do this is via +the @env{GLIBC_TUNABLES} environment variable by setting it to a string +of colon-separated @var{name}=@var{value} pairs. For example, the following +environment variable export enables malloc checking and sets the malloc +trim threshold to 128 bytes. + +@code{export GLIBC_TUNABLES=glibc.malloc.trim_threshold=128:glibc.malloc.check=3} + +Tunables are not part of the @glibcadj{} stable ABI, and they are +subject to change or removal across releases. Additionally, the method to +modify tunable values may change between releases and across distributions. +It is possible to implement multiple `frontends' for the tunables allowing +distributions to choose their preferred method at build time. + +Finally, the set of tunables available may vary between distributions as +the tunables feature allows distributions to add their own tunables under +their own namespace. + +@menu +* Tunable names:: The structure of a tunable name +* Memory Allocation Tunables:: Tunables in the memory allocation subsystem +@end menu + +@node Tunable names +@section Tunable names + +A tunable name is split into three components, a top namespace, a tunable +namespace and the tunable name. The top namespace for tunables +implemented in @theglibc{} project is @code{glibc}. Distributions that +choose to add custom tunables in their maintained versions of @theglibc{} +may choose to do so under their own top namespace. + +The tunable namespace is a logical grouping of tunables in a single +module. This currently holds no special significance, although that may +change in future. + +The tunable name is the actual name of the tunable. It is possible that +different tunable namespaces may have tunables within them that have the +same name, likewise for top namespaces. Hence, we only support +identification of tunables by their full name, i.e. with the top +namespace, tunable namespace and tunable name, separated by periods. + +@node Memory Allocation Tunables +@section Memory Allocation Tunables + +@deftp Namespace glibc.malloc +Memory allocation behaviour can be modified by setting any of the +following tunables in the @code{malloc} namespace. +@end deftp + +@deftp Tunable glibc.malloc.check +This tunable supersedes the @code{MALLOC_CHECK_} environment variable and is +identical in features. + +Setting this tunable enables a special (less efficient) memory allocator for +the malloc family of functions that is designed to be tolerant against simple +errors such as double calls of free with the same argument, or overruns of a +single byte (off-by-one bugs). Not all such errors can be protected against, +however, and memory leaks can result. The following list describes the values +that this tunable can take and the effect they have on malloc functionality. + +@itemize @bullet +@item @code{0} Disable all error reporting. The alternate allocator is +selected and heap corruption detection is in place, but any such errors +detected are ignored. This is currently a supported use, but is not +recommended. +@item @code{1} Report errors. The alternate allocator is selected and heap +corruption, if detected, is reported as diagnostic messages to @code{stderr} +and the program continues execution. +@item @code{2} Abort on errors. The alternate allocator is selected and if +heap corruption is detected, the program is ended immediately by calling +@code{abort}. +@item @code{3} Fully enabled. The alternate allocator is selected and is fully +functional. That is, if heap corruption is detected, a verbose diagnostic +message is printed to @code{stderr} and the program is ended by calling +@code{abort}. +@end itemize + +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. +@end deftp + +@deftp Tunable glibc.malloc.top_pad +This tunable supersedes the @code{MALLOC_TOP_PAD_} environment variable and is +identical in features. + +This tunable determines the amount of extra memory to obtain from the system +when any of the arenas need to be extended. It also specifies the number of +bytes to retain when shrinking any of the arenas. This provides the necessary +hysteresis in heap size such that excessive amounts of system calls can be +avoided. + +The default value of this tunable is @samp{0}. +@end deftp + +@deftp Tunable glibc.malloc.perturb +This tunable supersedes the @code{MALLOC_PERTURB_} environment variable and is +identical in features. + +If set to a non-zero value, memory blocks are initialized with values depending +on some low order bits of this tunable when they are allocated (except when +allocated by calloc) and freed. This can be used to debug the use of +uninitialized or freed heap memory. Note that this option does not guarantee +that the freed block will have any specific values. It only guarantees that the +content the block had before it was freed will be overwritten. + +The default value of this tunable is @samp{0}. +@end deftp + +@deftp Tunable glibc.malloc.mmap_threshold +This tunable supersedes the @code{MALLOC_MMAP_THRESHOLD_} environment variable +and is identical in features. + +When this tunable is set, all chunks larger than this value are allocated +outside the normal heap, using the @code{mmap} system call. This way it is +guaranteed that the memory for these chunks can be returned to the system on +@code{free}. Note that requests smaller than this threshold might still be +allocated via @code{mmap}. + +If this tunable is not set, the default value is set as 128 KB and the +threshold is adjusted dynamically to suit the allocation patterns of the +program. If the tunable is set, the dynamic adjustment is disabled and the +value is set as static. +@end deftp + +@deftp Tunable glibc.malloc.trim_threshold +This tunable supersedes the @code{MALLOC_TRIM_THRESHOLD_} environment variable +and is identical in features. + +The value of this tunable is the minimum size (in bytes) of the top-most, +releasable chunk in an arena that will trigger a system call in order to return +memory to the system from that arena. + +If this tunable is not set, the default value is set as 128 KB and the +threshold is adjusted dynamically to suit the allocation patterns of the +program. If the tunable is set, the dynamic adjustment is disabled and the +value is set as static. +@end deftp + +@deftp Tunable glibc.malloc.mmap_max +This tunable supersedes the @code{MALLOC_MMAP_MAX_} environment variable and is +identical in features. + +The value of this tunable is maximum number of chunks to allocate with +@code{mmap}. Setting this to zero disables all use of @code{mmap}. + +The default value of this tunable is @samp{65536}. +@end deftp + +@deftp Tunable glibc.malloc.arena_test +This tunable supersedes the @code{MALLOC_ARENA_TEST} environment variable and is +identical in features. + +The @code{glibc.malloc.arena_test} tunable specifies the number of arenas that +can be created before the test on the limit to the number of arenas is +conducted. The value is ignored if @code{glibc.malloc.arena_max} is set. + +The default value of this tunable is 2 for 32-bit systems and 8 for 64-bit +systems. +@end deftp + +@deftp Tunable glibc.malloc.arena_max +This tunable supersedes the @code{MALLOC_ARENA_MAX} environment variable and is +identical in features. + +This tunable sets the number of arenas to use in a process regardless of the +number of cores in the system. + +The default value of this tunable is @code{0}, meaning that the limit on the +number of arenas is determined by the number of CPU cores online. For 32-bit +systems the limit is twice the number of cores online and on 64-bit systems, it +is 8 times the number of cores online. +@end deftp -- 2.7.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/4] User manual documentation for tunables 2016-12-30 4:09 ` [PATCH 4/4] User manual documentation for tunables Siddhesh Poyarekar @ 2016-12-30 9:50 ` Florian Weimer 2016-12-31 12:40 ` Siddhesh Poyarekar 0 siblings, 1 reply; 27+ messages in thread From: Florian Weimer @ 2016-12-30 9:50 UTC (permalink / raw) To: Siddhesh Poyarekar, libc-alpha; +Cc: carlos On 12/30/2016 05:08 AM, Siddhesh Poyarekar wrote: > Create a new node for tunables documentation and add notes for the > malloc tunables. > > * manual/tunables.texi: New chapter. > * manual/Makefile (chapters): Add it. > * manual/probes.texi (@node): Point to the Tunables chapter. > --- > manual/Makefile | 3 +- > manual/probes.texi | 2 +- > manual/tunables.texi | 185 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 188 insertions(+), 2 deletions(-) > create mode 100644 manual/tunables.texi > > diff --git a/manual/Makefile b/manual/Makefile > index f2f694f..ecc2bf6 100644 > --- a/manual/Makefile > +++ b/manual/Makefile > @@ -38,7 +38,8 @@ chapters = $(addsuffix .texi, \ > message search pattern io stdio llio filesys \ > pipe socket terminal syslog math arith time \ > resource setjmp signal startup process ipc job \ > - nss users sysinfo conf crypt debug threads probes) > + nss users sysinfo conf crypt debug threads \ > + probes tunables) > add-chapters = $(wildcard $(foreach d, $(add-ons), ../$d/$d.texi)) > appendices = lang.texi header.texi install.texi maint.texi platform.texi \ > contrib.texi > diff --git a/manual/probes.texi b/manual/probes.texi > index 237a918..eb91c62 100644 > --- a/manual/probes.texi > +++ b/manual/probes.texi > @@ -1,5 +1,5 @@ > @node Internal Probes > -@c @node Internal Probes, , POSIX Threads, Top > +@c @node Internal Probes, Tunables, POSIX Threads, Top > @c %MENU% Probes to monitor libc internal behavior > @chapter Internal probes > > diff --git a/manual/tunables.texi b/manual/tunables.texi > new file mode 100644 > index 0000000..1f1f9fc > --- /dev/null > +++ b/manual/tunables.texi > @@ -0,0 +1,185 @@ > +@node Tunables > +@c @node Tunables, , Internal Probes, Top > +@c %MENU% Tunable switches to alter libc internal behavior > +@chapter Tunables > + > +Tunables is a feature in @theglibc{} that allows application authors and “Tunables are”? Both variants are probably fine, but you already use the plural variant below. > +distribution maintainers to alter the runtime library behaviour to match Please use “behavior” consistently. > +their workload. These are implemented as a set of switches that may be > +modified in different ways. The current default method to do this is via > +the @env{GLIBC_TUNABLES} environment variable by setting it to a string > +of colon-separated @var{name}=@var{value} pairs. For example, the following > +environment variable export enables malloc checking and sets the malloc > +trim threshold to 128 bytes. Please end the sentence with a colon: > +@code{export GLIBC_TUNABLES=glibc.malloc.trim_threshold=128:glibc.malloc.check=3} This should use @example and perhaps POSIX shell syntax: @example GLIBC_TUNABLES=glibc.malloc.trim_threshold=128:glibc.malloc.check=3 export GLIBC_TUNABLES @end example amespace, tunable namespace and tunable name, separated by periods. > + > +@node Memory Allocation Tunables > +@section Memory Allocation Tunables > + > +@deftp Namespace glibc.malloc I think this should use: @deftp {Tunable namespace} glibc.malloc “Namespace” is too generic. > +@deftp Tunable glibc.malloc.check > +This tunable supersedes the @code{MALLOC_CHECK_} environment variable and is > +identical in features. > + > +Setting this tunable enables a special (less efficient) memory allocator for > +the malloc family of functions that is designed to be tolerant against simple > +errors such as double calls of free with the same argument, or overruns of a > +single byte (off-by-one bugs). I don't think this is true for the value 0. 0 uses the standard allocator and continues execution after that malloc detected errors. > +@itemize @bullet > +@item @code{0} Disable all error reporting. The alternate allocator is > +selected and heap corruption detection is in place, but any such errors > +detected are ignored. This is currently a supported use, but is not > +recommended. See above. Thanks, Florian ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/4] User manual documentation for tunables 2016-12-30 9:50 ` Florian Weimer @ 2016-12-31 12:40 ` Siddhesh Poyarekar 0 siblings, 0 replies; 27+ messages in thread From: Siddhesh Poyarekar @ 2016-12-31 12:40 UTC (permalink / raw) To: Florian Weimer, libc-alpha; +Cc: carlos On Friday 30 December 2016 03:20 PM, Florian Weimer wrote: > I don't think this is true for the value 0. 0 uses the standard > allocator and continues execution after that malloc detected errors. Ahh right, I'll fix that. Siddhesh ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-12-30 4:09 [PATCH v7 0/4] glibc tunables Siddhesh Poyarekar ` (2 preceding siblings ...) 2016-12-30 4:09 ` [PATCH 4/4] User manual documentation for tunables Siddhesh Poyarekar @ 2016-12-30 4:09 ` Siddhesh Poyarekar 2016-12-30 9:50 ` Florian Weimer 3 siblings, 1 reply; 27+ messages in thread From: Siddhesh Poyarekar @ 2016-12-30 4:09 UTC (permalink / raw) To: libc-alpha; +Cc: fweimer, carlos Read tunables values from the users using the GLIBC_TUNABLES environment variable. The value of this variable is a colon-separated list of name=value pairs. So a typical string would look like this: GLIBC_TUNABLES=glibc.malloc.mmap_threshold=2048:glibc.malloc.trim_threshold=1024 * config.make.in (have-loop-to-function): Define. * elf/Makefile (CFLAGS-dl-tunables.c): Add -fno-tree-loop-distribute-patterns. * elf/dl-tunables.c: Include libc-internals.h. (GLIBC_TUNABLES): New macro. (tunables_strdup): New function. (parse_tunables): New function. (min_strlen): New function. (__tunables_init): Use the new functions and macro. (disable_tunable): Disable tunable from GLIBC_TUNABLES. * malloc/tst-malloc-usable-tunables.c: New test case. * malloc/tst-malloc-usable-static-tunables.c: New test case. * malloc/Makefile (tests, tests-static): Add tests. --- config.make.in | 1 + elf/Makefile | 6 ++ elf/dl-tunables.c | 129 ++++++++++++++++++++++++++++- malloc/Makefile | 6 +- malloc/tst-malloc-usable-static-tunables.c | 1 + malloc/tst-malloc-usable-tunables.c | 1 + 6 files changed, 142 insertions(+), 2 deletions(-) create mode 100644 malloc/tst-malloc-usable-static-tunables.c create mode 100644 malloc/tst-malloc-usable-tunables.c diff --git a/config.make.in b/config.make.in index 2f8dae2..5836b32 100644 --- a/config.make.in +++ b/config.make.in @@ -71,6 +71,7 @@ have-hash-style = @libc_cv_hashstyle@ use-default-link = @use_default_link@ output-format = @libc_cv_output_format@ have-cxx-thread_local = @libc_cv_cxx_thread_local@ +have-loop-to-function = @libc_cv_cc_loop_to_function@ multi-arch = @multi_arch@ diff --git a/elf/Makefile b/elf/Makefile index de28d99..3cda2c9 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -38,6 +38,12 @@ endif ifeq (yes,$(have-tunables)) dl-routines += dl-tunables + +# Make sure that the compiler does not insert any library calls in tunables +# code paths. +ifeq (yes,$(have-loop-to-function)) +CFLAGS-dl-tunables.c = -fno-tree-loop-distribute-patterns +endif endif all-dl-routines = $(dl-routines) $(sysdep-dl-routines) diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index df60c57..d41b6dc 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -30,7 +30,10 @@ #define TUNABLES_INTERNAL 1 #include "dl-tunables.h" -/* Compare environment names, bounded by the name hardcoded in glibc. */ +#define GLIBC_TUNABLES "GLIBC_TUNABLES" + +/* Compare environment or tunable names, bounded by the name hardcoded in + glibc. */ static bool is_name (const char *orig, const char *envname) { @@ -45,6 +48,29 @@ is_name (const char *orig, const char *envname) return false; } +static char * +tunables_strdup (const char *in) +{ + size_t i = 0; + + while (in[i++]); + + /* Bare mmap system call to avoid setting errno which may not be available + yet. */ + INTERNAL_SYSCALL_DECL (err); + char *out = (char *) INTERNAL_SYSCALL_CALL (mmap, err, NULL, i, PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + if (INTERNAL_SYSCALL_ERROR_P (out, err)) + return NULL; + + i--; + + while (i-- > 0) + out[i] = in[i]; + + return out; +} + static char ** get_next_env (char **envp, char **name, size_t *namelen, char **val) { @@ -218,6 +244,82 @@ tunable_initialize (tunable_t *cur, const char *strval) } } +static void +parse_tunables (char *tunestr) +{ + if (tunestr == NULL || *tunestr == '\0') + return; + + char *p = tunestr; + + while (true) + { + char *name = p; + size_t len = 0; + + /* First, find where the name ends. */ + while (p[len] != '=' && p[len] != ':' && p[len] != '\0') + len++; + + /* If we reach the end of the string before getting a valid name-value + pair, bail out. */ + if (p[len] == '\0') + return; + + /* We did not find a valid name-value pair before encountering the + colon. */ + if (p[len]== ':') + { + p += len + 1; + continue; + } + + p += len + 1; + + char *value = p; + len = 0; + + while (p[len] != ':' && p[len] != '\0') + len++; + + char end = p[len]; + p[len] = '\0'; + + /* Add the tunable if it exists. */ + for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) + { + tunable_t *cur = &tunable_list[i]; + + /* If we are in a secure context (AT_SECURE) then ignore the tunable + unless it is explicitly marked as secure. Tunable values take + precendence over their envvar aliases. */ + if (__libc_enable_secure && !cur->is_secure) + continue; + + if (is_name (cur->name, name)) + { + tunable_initialize (cur, value); + break; + } + } + + if (end == ':') + p += len + 1; + else + return; + } +} + +static size_t +min_strlen (const char *s) +{ + size_t i = 0; + while (*s++ != '\0') + i++; + + return i; +} + /* Disable a tunable if it is set. */ static void disable_tunable (tunable_id_t id, char **envp) @@ -226,6 +328,23 @@ disable_tunable (tunable_id_t id, char **envp) if (env_alias != NULL) tunables_unsetenv (envp, tunable_list[id].env_alias); + + char *tunable = getenv (GLIBC_TUNABLES); + const char *cmp = tunable_list[id].name; + const size_t len = min_strlen (cmp); + + while (tunable && *tunable != '\0' && *tunable != ':') + { + if (is_name (tunable, cmp)) + { + tunable += len; + /* Overwrite the = and the value with colons. */ + while (*tunable != '\0' && *tunable != ':') + *tunable++ = ':'; + break; + } + tunable++; + } } /* Disable the glibc.malloc.check tunable in SETUID/SETGID programs unless @@ -256,6 +375,14 @@ __tunables_init (char **envp) while ((envp = get_next_env (envp, &envname, &len, &envval)) != NULL) { + if (is_name (GLIBC_TUNABLES, envname)) + { + char *val = tunables_strdup (envval); + if (val != NULL) + parse_tunables (val); + continue; + } + for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) { tunable_t *cur = &tunable_list[i]; diff --git a/malloc/Makefile b/malloc/Makefile index 4e4104e..a34e20c 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -33,11 +33,13 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ tst-mallocfork2 \ tst-interpose-nothread \ tst-interpose-thread \ + tst-malloc-usable-tunables tests-static := \ tst-interpose-static-nothread \ tst-interpose-static-thread \ - tst-malloc-usable-static + tst-malloc-usable-static \ + tst-malloc-usable-static-tunables tests += $(tests-static) test-srcs = tst-mtrace @@ -160,6 +162,8 @@ endif tst-mcheck-ENV = MALLOC_CHECK_=3 tst-malloc-usable-ENV = MALLOC_CHECK_=3 tst-malloc-usable-static-ENV = $(tst-malloc-usable-ENV) +tst-malloc-usable-tunables-ENV = GLIBC_TUNABLES=glibc.malloc.check=3 +tst-malloc-usable-static-tunables-ENV = $(tst-malloc-usable-tunables-ENV) # Uncomment this for test releases. For public releases it is too expensive. #CPPFLAGS-malloc.o += -DMALLOC_DEBUG=1 diff --git a/malloc/tst-malloc-usable-static-tunables.c b/malloc/tst-malloc-usable-static-tunables.c new file mode 100644 index 0000000..8907db0 --- /dev/null +++ b/malloc/tst-malloc-usable-static-tunables.c @@ -0,0 +1 @@ +#include <malloc/tst-malloc-usable.c> diff --git a/malloc/tst-malloc-usable-tunables.c b/malloc/tst-malloc-usable-tunables.c new file mode 100644 index 0000000..8907db0 --- /dev/null +++ b/malloc/tst-malloc-usable-tunables.c @@ -0,0 +1 @@ +#include <malloc/tst-malloc-usable.c> -- 2.7.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable 2016-12-30 4:09 ` [PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar @ 2016-12-30 9:50 ` Florian Weimer 0 siblings, 0 replies; 27+ messages in thread From: Florian Weimer @ 2016-12-30 9:50 UTC (permalink / raw) To: Siddhesh Poyarekar, libc-alpha; +Cc: carlos On 12/30/2016 05:08 AM, Siddhesh Poyarekar wrote: > + /* Bare mmap system call to avoid setting errno which may not be available > + yet. */ > + INTERNAL_SYSCALL_DECL (err); > + char *out = (char *) INTERNAL_SYSCALL_CALL (mmap, err, NULL, i, PROT_READ | PROT_WRITE, > + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); > + if (INTERNAL_SYSCALL_ERROR_P (out, err)) > + return NULL; INTERNAL_SYSCALL is not available in generic code. You should use sbrk (like the TCB allocation in the startup code; it would also reduce wasted memory), or mmap with a comment indicating that this will crash if mmap updates errno. (I believe the comment in the startup code about mmap is wrong.) Thanks, Florian ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v8 0/4] glibc tunables @ 2016-12-31 15:41 Siddhesh Poyarekar [not found] ` <1483198831-2232-2-git-send-email-siddhesh@sourceware.org> 0 siblings, 1 reply; 27+ messages in thread From: Siddhesh Poyarekar @ 2016-12-31 15:41 UTC (permalink / raw) To: libc-alpha; +Cc: fweimer, carlos Hi, Here is an updated version of the glibc tunables patchset. Changes: - Use sbrk instead of mmap for allocation of the tunables string and add a note about setting errno. I'll post a patch to add an __sbrk_noerrno in 2.26 to make this bullet-proof. - Go back to __builtin_unreachable because __builtin_trap may cause issues in other architectures. I will do a build test of other architectures and post an update before release if everything is OK. - Fixed up manual changes, including ones that Rical had suggested earlier - I had made those in another branch and forgotten about them. Siddhesh Poyarekar (4): Add framework for tunables Initialize tunable list with the GLIBC_TUNABLES environment variable Enhance --enable-tunables to select tunables frontend at build time User manual documentation for tunables INSTALL | 21 ++ Makeconfig | 16 + README.tunables | 85 ++++++ config.h.in | 3 + config.make.in | 2 + configure | 17 ++ configure.ac | 10 + csu/init-first.c | 2 - csu/libc-start.c | 8 + elf/Makefile | 13 + elf/Versions | 3 + elf/dl-support.c | 2 + elf/dl-sysdep.c | 4 + elf/dl-tunable-types.h | 46 +++ elf/dl-tunables.c | 457 +++++++++++++++++++++++++++++ elf/dl-tunables.h | 92 ++++++ elf/dl-tunables.list | 69 +++++ elf/rtld.c | 2 + malloc/Makefile | 6 + malloc/arena.c | 54 ++++ malloc/tst-malloc-usable-static-tunables.c | 1 + malloc/tst-malloc-usable-static.c | 1 + malloc/tst-malloc-usable-tunables.c | 1 + manual/Makefile | 3 +- manual/install.texi | 20 ++ manual/probes.texi | 2 +- manual/tunables.texi | 192 ++++++++++++ scripts/gen-tunables.awk | 157 ++++++++++ sysdeps/mach/hurd/dl-sysdep.c | 4 + 29 files changed, 1289 insertions(+), 4 deletions(-) create mode 100644 README.tunables create mode 100644 elf/dl-tunable-types.h create mode 100644 elf/dl-tunables.c create mode 100644 elf/dl-tunables.h create mode 100644 elf/dl-tunables.list create mode 100644 malloc/tst-malloc-usable-static-tunables.c create mode 100644 malloc/tst-malloc-usable-static.c create mode 100644 malloc/tst-malloc-usable-tunables.c create mode 100644 manual/tunables.texi create mode 100644 scripts/gen-tunables.awk -- 2.7.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <1483198831-2232-2-git-send-email-siddhesh@sourceware.org>]
* Re: [PATCH 1/4] Add framework for tunables [not found] ` <1483198831-2232-2-git-send-email-siddhesh@sourceware.org> @ 2016-12-31 16:49 ` Florian Weimer 2016-12-31 17:58 ` Siddhesh Poyarekar 0 siblings, 1 reply; 27+ messages in thread From: Florian Weimer @ 2016-12-31 16:49 UTC (permalink / raw) To: Siddhesh Poyarekar, libc-alpha; +Cc: carlos On 12/31/2016 04:40 PM, Siddhesh Poyarekar wrote: > +#ifndef _TUNABLES_H_ > +#define _TUNABLES_H_ > + > +# if !HAVE_TUNABLES > +static inline void > +__always_inline > +__tunables_init (char **unused __attribute_unused) > +{ > + return; > +} You can drop the return statement. Maybe replace it with a comment indicating that this initalization function is optimized out if tunables are not enabled? > +# else > + > +# include <stddef.h> > +# include "dl-tunable-types.h" > + > +/* A tunable. */ I think the header inclusion guard does not usually trigger such indentation. Apart from that, I think the series is ready commit. I will do some installation-tree testing after that. Thanks, Florian ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2016-12-31 16:49 ` [PATCH 1/4] Add framework for tunables Florian Weimer @ 2016-12-31 17:58 ` Siddhesh Poyarekar 2016-12-31 19:43 ` Florian Weimer 0 siblings, 1 reply; 27+ messages in thread From: Siddhesh Poyarekar @ 2016-12-31 17:58 UTC (permalink / raw) To: Florian Weimer, libc-alpha; +Cc: carlos On Saturday 31 December 2016 10:19 PM, Florian Weimer wrote: > Apart from that, I think the series is ready commit. I will do some > installation-tree testing after that. Thanks, I'll push them shortly. Siddhesh ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2016-12-31 17:58 ` Siddhesh Poyarekar @ 2016-12-31 19:43 ` Florian Weimer 2016-12-31 19:50 ` Siddhesh Poyarekar 0 siblings, 1 reply; 27+ messages in thread From: Florian Weimer @ 2016-12-31 19:43 UTC (permalink / raw) To: siddhesh, libc-alpha; +Cc: carlos On 12/31/2016 06:58 PM, Siddhesh Poyarekar wrote: > On Saturday 31 December 2016 10:19 PM, Florian Weimer wrote: >> Apart from that, I think the series is ready commit. I will do some >> installation-tree testing after that. > > Thanks, I'll push them shortly. I missed the __attribute_unused syntax error. :( I'll fix that shortly. I've also realized that the suggested csu/ move did not happen, so building with stack protector enabled could be broken. Thanks, Florian ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2016-12-31 19:43 ` Florian Weimer @ 2016-12-31 19:50 ` Siddhesh Poyarekar 2016-12-31 19:52 ` Florian Weimer 0 siblings, 1 reply; 27+ messages in thread From: Siddhesh Poyarekar @ 2016-12-31 19:50 UTC (permalink / raw) To: Florian Weimer, libc-alpha; +Cc: carlos On Sunday 01 January 2017 01:13 AM, Florian Weimer wrote: > I missed the __attribute_unused syntax error. :( Ugh, sorry, that was a stupid mistake. > I'll fix that shortly. Thanks. > I've also realized that the suggested csu/ move > did not happen, so building with stack protector enabled could be broken. I saw your comment about it but did not link it to an actual suggestion. Did you suggest moving tunables to csu or specific bits of it? Siddhesh ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2016-12-31 19:50 ` Siddhesh Poyarekar @ 2016-12-31 19:52 ` Florian Weimer 2017-01-01 13:40 ` Nix 0 siblings, 1 reply; 27+ messages in thread From: Florian Weimer @ 2016-12-31 19:52 UTC (permalink / raw) To: siddhesh, libc-alpha; +Cc: carlos On 12/31/2016 08:49 PM, Siddhesh Poyarekar wrote: >> I've also realized that the suggested csu/ move >> did not happen, so building with stack protector enabled could be broken. > > I saw your comment about it but did not link it to an actual suggestion. Never mind, it appears the existing magic in elf/Makefile takes care of the details. Thanks, Florian ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2016-12-31 19:52 ` Florian Weimer @ 2017-01-01 13:40 ` Nix 0 siblings, 0 replies; 27+ messages in thread From: Nix @ 2017-01-01 13:40 UTC (permalink / raw) To: Florian Weimer; +Cc: siddhesh, libc-alpha, carlos On 31 Dec 2016, Florian Weimer told this: > On 12/31/2016 08:49 PM, Siddhesh Poyarekar wrote: > >>> I've also realized that the suggested csu/ move >>> did not happen, so building with stack protector enabled could be broken. >> >> I saw your comment about it but did not link it to an actual suggestion. > > Never mind, it appears the existing magic in elf/Makefile takes care of the details. Yeah, you never need to worry about dynamic-linker stack-protection interactions in any directory because of that magic. It's static libc init that poses problems for csu/, hence the hardwired disabling of the stack-protector in csu/ for that case. (Though, as you noted in a review, that also unfortunately catches any tests in that directory). -- NULL && (void) ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCHv6 0/4] glibc tunables @ 2016-11-16 8:35 Siddhesh Poyarekar 2016-11-16 8:35 ` [PATCH 1/4] Add framework for tunables Siddhesh Poyarekar 0 siblings, 1 reply; 27+ messages in thread From: Siddhesh Poyarekar @ 2016-11-16 8:35 UTC (permalink / raw) To: libc-alpha; +Cc: carlos, adhemerval.zanella Hi, Here is iteration 6 of the tunables patchset, now with two patches palmed off to Adhemerval to worry about! Here are changes from the previous patchset: - I discovered some implicit includes that broke aarch64 - shame on me for missing them. - I realized that __tunables_init used strncmp and strlen. They're built into ld.so too, but they are likely tunables targets and I don't want them to break unintentionally in future, so off they go - The patchset depends on Adhemerval's __access_noerrno patch[1] fixed up with my comments on the patch. The branch siddhesh/tunables has these patches and also Adhemerval's fixed up patch. If you're watching this, please also review Adhemerval's __access_noerrno patch and help get that through. Siddhesh [1] https://sourceware.org/ml/libc-alpha/2016-11/msg00416.html Siddhesh Poyarekar (4): Add framework for tunables Initialize tunable list with the GLIBC_TUNABLES environment variable Enhance --enable-tunables to select tunables frontend at build time User manual documentation for tunables INSTALL | 18 ++ Makeconfig | 16 ++ README.tunables | 84 ++++++ config.h.in | 3 + config.make.in | 1 + configure | 17 ++ configure.ac | 10 + csu/init-first.c | 2 - csu/libc-start.c | 12 + elf/Makefile | 7 + elf/Versions | 3 + elf/dl-support.c | 2 + elf/dl-sysdep.c | 8 + elf/dl-tunable-types.h | 46 +++ elf/dl-tunables.c | 447 +++++++++++++++++++++++++++++ elf/dl-tunables.h | 82 ++++++ elf/dl-tunables.list | 69 +++++ elf/rtld.c | 2 + malloc/Makefile | 6 + malloc/arena.c | 54 ++++ malloc/tst-malloc-usable-static-tunables.c | 1 + malloc/tst-malloc-usable-static.c | 1 + malloc/tst-malloc-usable-tunables.c | 1 + manual/Makefile | 3 +- manual/install.texi | 21 ++ manual/probes.texi | 2 +- manual/tunables.texi | 185 ++++++++++++ scripts/gen-tunables.awk | 157 ++++++++++ sysdeps/mach/hurd/dl-sysdep.c | 8 + 29 files changed, 1264 insertions(+), 4 deletions(-) create mode 100644 README.tunables create mode 100644 elf/dl-tunable-types.h create mode 100644 elf/dl-tunables.c create mode 100644 elf/dl-tunables.h create mode 100644 elf/dl-tunables.list create mode 100644 malloc/tst-malloc-usable-static-tunables.c create mode 100644 malloc/tst-malloc-usable-static.c create mode 100644 malloc/tst-malloc-usable-tunables.c create mode 100644 manual/tunables.texi create mode 100644 scripts/gen-tunables.awk -- 2.7.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/4] Add framework for tunables 2016-11-16 8:35 [PATCHv6 0/4] glibc tunables Siddhesh Poyarekar @ 2016-11-16 8:35 ` Siddhesh Poyarekar 2016-12-27 11:08 ` Florian Weimer 0 siblings, 1 reply; 27+ messages in thread From: Siddhesh Poyarekar @ 2016-11-16 8:35 UTC (permalink / raw) To: libc-alpha; +Cc: carlos, adhemerval.zanella The tunables framework allows us to uniformly manage and expose global variables inside glibc as switches to users. tunables/README has instructions for glibc developers to add new tunables. Tunables support can be enabled by passing the --enable-tunables configure flag to the configure script. This patch only adds a framework and does not pose any limitations on how tunable values are read from the user. It also adds environment variables used in malloc behaviour tweaking to the tunables framework as a PoC of the compatibility interface. * manual/install.texi: Add --enable-tunables option. * INSTALL: Regenerate. * README.tunables: New file. * Makeconfig (CPPFLAGS): Define TOP_NAMESPACE. (before-compile): Generate dl-tunable-list.h early. * config.h.in: Add HAVE_TUNABLES. * config.make.in: Add have-tunables. * configure.ac: Add --enable-tunables option. * configure: Regenerate. * csu/init-first.c (__libc_init_first): Move __libc_init_secure earlier... * csu/libc-start.c (LIBC_START_MAIN):... to here. [HAVE_TUNABLES]: Include dl-tunables.h. (LIBC_START_MAIN) [!SHARED]: Initialize tunables for static binaries. * elf/Makefile (dl-routines): Add dl-tunables. * elf/Versions (ld): Add __tunable_set_val to GLIBC_PRIVATE namespace. * elf/dl-support (_dl_nondynamic_init): Unset MALLOC_CHECK_ only when !HAVE_TUNABLES. * elf/rtld.c (process_envvars): Likewise. * elf/dl-sysdep.c [HAVE_TUNABLES]: Include dl-tunables.h (_dl_sysdep_start): Call __tunables_init. * elf/dl-tunable-types.h: New file. * elf/dl-tunables.c: New file. * elf/dl-tunables.h: New file. * elf/dl-tunables.list: New file. * malloc/tst-malloc-usable-static.c: New test case. * malloc/Makefile (tests-static): Add it. * malloc/arena.c [HAVE_TUNABLES]: Include dl-tunables.h. Define TUNABLE_NAMESPACE. (DL_TUNABLE_CALLBACK (set_mallopt_check)): New function. (DL_TUNABLE_CALLBACK_FNDECL): New macro. Use it to define callback functions. (ptmalloc_init): Set tunable values. * scripts/gen-tunables.awk: New file. * sysdeps/mach/hurd/dl-sysdep.c [HAVE_TUNABLES]: Include dl-tunables.h. (_dl_sysdep_start): Call __tunables_init. --- INSTALL | 5 + Makeconfig | 16 ++ README.tunables | 84 +++++++++++ config.h.in | 3 + config.make.in | 1 + configure | 16 ++ configure.ac | 10 ++ csu/init-first.c | 2 - csu/libc-start.c | 12 ++ elf/Makefile | 5 + elf/Versions | 3 + elf/dl-support.c | 2 + elf/dl-sysdep.c | 8 + elf/dl-tunable-types.h | 46 ++++++ elf/dl-tunables.c | 310 ++++++++++++++++++++++++++++++++++++++ elf/dl-tunables.h | 78 ++++++++++ elf/dl-tunables.list | 69 +++++++++ elf/rtld.c | 2 + malloc/Makefile | 2 + malloc/arena.c | 54 +++++++ malloc/tst-malloc-usable-static.c | 1 + manual/install.texi | 5 + scripts/gen-tunables.awk | 157 +++++++++++++++++++ sysdeps/mach/hurd/dl-sysdep.c | 8 + 24 files changed, 897 insertions(+), 2 deletions(-) create mode 100644 README.tunables create mode 100644 elf/dl-tunable-types.h create mode 100644 elf/dl-tunables.c create mode 100644 elf/dl-tunables.h create mode 100644 elf/dl-tunables.list create mode 100644 malloc/tst-malloc-usable-static.c create mode 100644 scripts/gen-tunables.awk diff --git a/INSTALL b/INSTALL index b5acedc..ccacccc 100644 --- a/INSTALL +++ b/INSTALL @@ -158,6 +158,11 @@ will be used, and CFLAGS sets optimization options for the compiler. By default for x86_64, the GNU C Library is built with the vector math library. Use this option to disable the vector math library. +'--enable-tunables' + Tunables support allows additional library parameters to be + customized at runtime. This is an experimental feature and affects + startup time and is thus disabled by default. + '--build=BUILD-SYSTEM' '--host=HOST-SYSTEM' These options are for cross-compiling. If you specify both options diff --git a/Makeconfig b/Makeconfig index a785860..d0eb832 100644 --- a/Makeconfig +++ b/Makeconfig @@ -883,6 +883,11 @@ CPPFLAGS = $(config-extra-cppflags) $(CPPUNDEFS) $(CPPFLAGS-config) \ $(foreach lib,$(libof-$(basename $(@F))) \ $(libof-$(<F)) $(libof-$(@F)),$(CPPFLAGS-$(lib))) \ $(CPPFLAGS-$(<F)) $(CPPFLAGS-$(@F)) $(CPPFLAGS-$(basename $(@F))) + +ifeq (yes,$(have-tunables)) +CPPFLAGS += -DTOP_NAMESPACE=glibc +endif + override CFLAGS = -std=gnu11 -fgnu89-inline $(config-extra-cflags) \ $(filter-out %frame-pointer,$(+cflags)) $(+gccwarn-c) \ $(sysdep-CFLAGS) $(CFLAGS-$(suffix $@)) $(CFLAGS-$(<F)) \ @@ -1057,6 +1062,17 @@ $(common-objpfx)libc-modules.stmp: $(..)scripts/gen-libc-modules.awk \ endif +# Build the tunables list header early since it could be used by any module in +# glibc. +ifeq (yes,$(have-tunables)) +before-compile += $(common-objpfx)dl-tunable-list.h + +$(common-objpfx)dl-tunable-list.h: $(..)scripts/gen-tunables.awk \ + $(..)elf/dl-tunables.list + $(AWK) -f $^ > $@.tmp + mv $@.tmp $@ +endif + common-generated += libc-modules.h libc-modules.stmp # The name under which the run-time dynamic linker is installed. diff --git a/README.tunables b/README.tunables new file mode 100644 index 0000000..3d9c86a --- /dev/null +++ b/README.tunables @@ -0,0 +1,84 @@ + TUNABLE FRAMEWORK + ================= + +Tunables is a feature in the GNU C Library that allows application authors and +distribution maintainers to alter the runtime library behaviour to match their +workload. + +The tunable framework allows modules within glibc to register variables that +may be tweaked through an environment variable or an API call. It aims to +enforce a strict namespace rule to bring consistency to naming of these tunable +environment variables across the project. This document is a guide for glibc +developers to add tunables to the framework. + +ADDING A NEW TUNABLE +-------------------- + +The TOP_NAMESPACE is defined by default as 'glibc' and it may be overridden in +distributions for specific tunables if they want to add their own tunables. +Downstream implementations are discouraged from using the 'glibc' namespace for +tunables they don't already have consensus to push upstream. + +There are two steps to adding a tunable: + +1. Add a tunable ID: + +Modules that wish to use the tunables interface must define the +TUNABLE_NAMESPACE macro. Following this, for each tunable you want to +add, make an entry in elf/dl-tunables.list. The format of the file is as +follows: + +TOP_NAMESPACE { + NAMESPACE1 { + TUNABLE1 { + # tunable attributes, one per line + } + # A tunable with default attributes, i.e. string variable. + TUNABLE2 + TUNABLE3 { + # its attributes + } + } + NAMESPACE2 { + ... + } +} + +The list of allowed attributes are: + +- type: Data type. Defaults to STRING. Allowed types are: + INT_32, SIZE_T and STRING. + +- minval: Optional minimum acceptable value. For a string type + this is the minimum length of the value. + +- maxval: Optional maximum acceptable value. For a string type + this is the maximum length of the value. + +- env_alias: An alias environment variable + +- is_secure: Specify whether the tunable should be read for setuid + binaries. True allows the tunable to be read for + setuid binaries while false disables it. Note that + even if this is set as true and the value is read, it + may not be used if it does not validate against the + acceptable values or is not considered safe by the + module. + +2. Call either the TUNABLE_SET_VALUE and pass into it the tunable name and a + pointer to the variable that should be set with the tunable value. + If additional work needs to be done after setting the value, use the + TUNABLE_SET_VALUE_WITH_CALLBACK instead and additionally pass a pointer to + the function that should be called if the tunable value has been set. + +FUTURE WORK +----------- + +The framework currently only allows a one-time initialization of variables +through environment variables and in some cases, modification of variables via +an API call. A future goals for this project include: + +- Setting system-wide and user-wide defaults for tunables through some + mechanism like a configuration file. + +- Allow tweaking of some tunables at runtime diff --git a/config.h.in b/config.h.in index 33757bd..fd93bb3 100644 --- a/config.h.in +++ b/config.h.in @@ -246,4 +246,7 @@ /* PowerPC32 uses fctidz for floating point to long long conversions. */ #define HAVE_PPC_FCTIDZ 0 +/* Build glibc with tunables support. */ +#define HAVE_TUNABLES 0 + #endif diff --git a/config.make.in b/config.make.in index 04a8b3e..6d0b607 100644 --- a/config.make.in +++ b/config.make.in @@ -93,6 +93,7 @@ use-nscd = @use_nscd@ build-hardcoded-path-in-tests= @hardcoded_path_in_tests@ build-pt-chown = @build_pt_chown@ enable-lock-elision = @enable_lock_elision@ +have-tunables = @have_tunables@ # Build tools. CC = @CC@ diff --git a/configure b/configure index d9f8c06..cbd5fb3 100755 --- a/configure +++ b/configure @@ -662,6 +662,7 @@ multi_arch base_machine add_on_subdirs add_ons +have_tunables build_pt_chown build_nscd link_obsolete_rpc @@ -776,6 +777,7 @@ enable_systemtap enable_build_nscd enable_nscd enable_pt_chown +enable_tunables enable_mathvec with_cpu ' @@ -1443,6 +1445,7 @@ Optional Features: --disable-build-nscd disable building and installing the nscd daemon --disable-nscd library functions will not contact the nscd daemon --enable-pt_chown Enable building and installing pt_chown + --enable-tunables Enable tunables support --enable-mathvec Enable building and installing mathvec [default depends on architecture] @@ -3649,6 +3652,19 @@ if test "$build_pt_chown" = yes; then fi +# Check whether --enable-tunables was given. +if test "${enable_tunables+set}" = set; then : + enableval=$enable_tunables; have_tunables=$enableval +else + have_tunables=no +fi + + +if test "$have_tunables" = yes; then + $as_echo "#define HAVE_TUNABLES 1" >>confdefs.h + +fi + # The abi-tags file uses a fairly simplistic model for name recognition that # can't distinguish i486-pc-linux-gnu fully from i486-pc-gnu. So we mutate a # $host_os of `gnu*' here to be `gnu-gnu*' just so that it can tell. diff --git a/configure.ac b/configure.ac index de0a40f..3b7a0db 100644 --- a/configure.ac +++ b/configure.ac @@ -395,6 +395,16 @@ if test "$build_pt_chown" = yes; then AC_DEFINE(HAVE_PT_CHOWN) fi +AC_ARG_ENABLE([tunables], + [AS_HELP_STRING([--enable-tunables], + [Enable tunables support])], + [have_tunables=$enableval], + [have_tunables=no]) +AC_SUBST(have_tunables) +if test "$have_tunables" = yes; then + AC_DEFINE(HAVE_TUNABLES) +fi + # The abi-tags file uses a fairly simplistic model for name recognition that # can't distinguish i486-pc-linux-gnu fully from i486-pc-gnu. So we mutate a # $host_os of `gnu*' here to be `gnu-gnu*' just so that it can tell. diff --git a/csu/init-first.c b/csu/init-first.c index 77c6e1c..465f25b 100644 --- a/csu/init-first.c +++ b/csu/init-first.c @@ -72,8 +72,6 @@ _init (int argc, char **argv, char **envp) __environ = envp; #ifndef SHARED - __libc_init_secure (); - /* First the initialization which normally would be done by the dynamic linker. */ _dl_non_dynamic_init (); diff --git a/csu/libc-start.c b/csu/libc-start.c index 99c040a..4e5cafa 100644 --- a/csu/libc-start.c +++ b/csu/libc-start.c @@ -21,6 +21,11 @@ #include <unistd.h> #include <ldsodefs.h> #include <exit-thread.h> +#include <libc-internal.h> + +#if HAVE_TUNABLES +# include <elf/dl-tunables.h> +#endif extern void __libc_init_first (int argc, char **argv, char **envp); @@ -184,6 +189,13 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), } # endif + /* Initialize very early so that tunables can use it. */ + __libc_init_secure (); + +#if HAVE_TUNABLES + __tunables_init (__environ); +#endif + /* Perform IREL{,A} relocations. */ apply_irel (); diff --git a/elf/Makefile b/elf/Makefile index 82c7e05..477f8fc 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -35,6 +35,11 @@ dl-routines = $(addprefix dl-,load lookup object reloc deps hwcaps \ ifeq (yes,$(use-ldconfig)) dl-routines += dl-cache endif + +ifeq (yes,$(have-tunables)) +dl-routines += dl-tunables +endif + all-dl-routines = $(dl-routines) $(sysdep-dl-routines) # But they are absent from the shared libc, because that code is in ld.so. elide-routines.os = $(all-dl-routines) dl-support enbl-secure dl-origin \ diff --git a/elf/Versions b/elf/Versions index 23deda9..1734697 100644 --- a/elf/Versions +++ b/elf/Versions @@ -64,5 +64,8 @@ ld { # Pointer protection. __pointer_chk_guard; + + # Set value of a tunable. + __tunable_set_val; } } diff --git a/elf/dl-support.c b/elf/dl-support.c index c30194c..d350d6d 100644 --- a/elf/dl-support.c +++ b/elf/dl-support.c @@ -354,8 +354,10 @@ _dl_non_dynamic_init (void) cp = (const char *) __rawmemchr (cp, '\0') + 1; } +#if !HAVE_TUNABLES if (__access ("/etc/suid-debug", F_OK) != 0) __unsetenv ("MALLOC_CHECK_"); +#endif } #ifdef DL_PLATFORM_INIT diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c index eaa7155..68f30f2 100644 --- a/elf/dl-sysdep.c +++ b/elf/dl-sysdep.c @@ -44,6 +44,10 @@ #include <hp-timing.h> #include <tls.h> +#if HAVE_TUNABLES +# include <dl-tunables.h> +#endif + extern char **_environ attribute_hidden; extern char _end[] attribute_hidden; @@ -219,6 +223,10 @@ _dl_sysdep_start (void **start_argptr, } #endif +#if HAVE_TUNABLES + __tunables_init (_environ); +#endif + #ifdef DL_SYSDEP_INIT DL_SYSDEP_INIT; #endif diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h new file mode 100644 index 0000000..d1591b6 --- /dev/null +++ b/elf/dl-tunable-types.h @@ -0,0 +1,46 @@ +/* Tunable type information. + + Copyright (C) 2016 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/>. */ + +#ifndef _TUNABLE_TYPES_H_ +# define _TUNABLE_TYPES_H_ +#include <stddef.h> + +typedef void (*tunable_callback_t) (void *); + +typedef enum +{ + TUNABLE_TYPE_INT_32, + TUNABLE_TYPE_SIZE_T, + TUNABLE_TYPE_STRING +} tunable_type_code_t; + +typedef struct +{ + tunable_type_code_t type_code; + int64_t min; + int64_t max; +} tunable_type_t; + +typedef union +{ + int64_t numval; + const char *strval; +} tunable_val_t; + +#endif diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c new file mode 100644 index 0000000..91340b6 --- /dev/null +++ b/elf/dl-tunables.c @@ -0,0 +1,310 @@ +/* The tunable framework. See the README to know how to use the tunable in + a glibc module. + + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <stdint.h> +#include <stdbool.h> +#include <unistd.h> +#include <stdlib.h> +#include <libc-internal.h> +#include <sysdep.h> +#include <fcntl.h> + +#define TUNABLES_INTERNAL 1 +#include "dl-tunables.h" + +/* Compare environment names, bounded by the name hardcoded in glibc. */ +static bool +is_name (const char *orig, const char *envname) +{ + for (;*orig != '\0' && *envname != '\0'; envname++, orig++) + if (*orig != *envname) + break; + + /* The ENVNAME is immediately followed by a value. */ + if (*orig == '\0' && *envname == '=') + return true; + else + return false; +} + +static char ** +get_next_env (char **envp, char **name, size_t *namelen, char **val) +{ + while (envp != NULL && *envp != NULL) + { + char *envline = *envp; + int len = 0; + + while (envline[len] != '\0' && envline[len] != '=') + len++; + + /* Just the name and no value, go to the next one. */ + if (envline[len] == '\0') + continue; + + *name = envline; + *namelen = len; + *val = &envline[len + 1]; + + return ++envp; + } + + return NULL; +} + +static int +tunables_unsetenv (char **ep, const char *name) +{ + while (*ep != NULL) + { + size_t cnt = 0; + + while ((*ep)[cnt] == name[cnt] && name[cnt] != '\0') + ++cnt; + + if (name[cnt] == '\0' && (*ep)[cnt] == '=') + { + /* Found it. Remove this pointer by moving later ones to + the front. */ + char **dp = ep; + + do + dp[0] = dp[1]; + while (*dp++); + /* Continue the loop in case NAME appears again. */ + } + else + ++ep; + } + + return 0; +} + +/* A stripped down strtoul-like implementation for very early use. It does not + set errno if the result is outside bounds because it gets called before + errno may have been set up. */ +static unsigned long int +tunables_strtoul (const char *nptr) +{ + unsigned long int result = 0; + long int sign = 1; + unsigned max_digit; + + while (*nptr == ' ' || *nptr == '\t') + ++nptr; + + if (*nptr == '-') + { + sign = -1; + ++nptr; + } + else if (*nptr == '+') + ++nptr; + + if (*nptr < '0' || *nptr > '9') + return 0UL; + + int base = 10; + max_digit = 9; + if (*nptr == '0') + { + if (nptr[1] == 'x' || nptr[1] == 'X') + { + base = 16; + nptr += 2; + } + else + { + base = 8; + max_digit = 7; + } + } + + while (1) + { + unsigned long int digval; + if (*nptr >= '0' && *nptr <= '0' + max_digit) + digval = *nptr - '0'; + else if (base == 16) + { + if (*nptr >= 'a' && *nptr <= 'f') + digval = *nptr - 'a' + 10; + else if (*nptr >= 'A' && *nptr <= 'F') + digval = *nptr - 'A' + 10; + else + break; + } + else + break; + + if (result > ULONG_MAX / base + || (result == ULONG_MAX / base && digval > ULONG_MAX % base)) + return ULONG_MAX; + result *= base; + result += digval; + ++nptr; + } + + return result * sign; +} + +/* If the value does not validate, don't bother initializing the internal type + and also take care to clear the recorded string value in STRVAL. */ +#define RETURN_IF_INVALID_RANGE(__cur, __val) \ +({ \ + __typeof ((__cur)->type.min) min = (__cur)->type.min; \ + __typeof ((__cur)->type.max) max = (__cur)->type.max; \ + if (min != max && ((__val) < min || (__val) > max)) \ + return; \ +}) + +/* 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) +{ + switch (cur->type.type_code) + { + case TUNABLE_TYPE_INT_32: + { + int32_t val = (int32_t) tunables_strtoul (strval); + RETURN_IF_INVALID_RANGE (cur, val); + cur->val.numval = (int64_t) val; + cur->strval = strval; + break; + } + case TUNABLE_TYPE_SIZE_T: + { + size_t val = (size_t) tunables_strtoul (strval); + RETURN_IF_INVALID_RANGE (cur, val); + cur->val.numval = (int64_t) val; + cur->strval = strval; + break; + } + case TUNABLE_TYPE_STRING: + { + cur->val.strval = cur->strval = strval; + break; + } + default: + __builtin_unreachable (); + } +} + +/* Disable a tunable if it is set. */ +static void +disable_tunable (tunable_id_t id, char **envp) +{ + const char *env_alias = tunable_list[id].env_alias; + + if (env_alias) + tunables_unsetenv (envp, tunable_list[id].env_alias); +} + +/* Disable the glibc.malloc.check tunable in SETUID/SETGID programs unless + the system administrator overrides it by creating 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 inline void +__always_inline +maybe_disable_malloc_check (void) +{ + if (__libc_enable_secure && __access_noerrno ("/etc/suid-debug", F_OK) != 0) + disable_tunable (TUNABLE_ENUM_NAME(glibc, malloc, check), __environ); +} + +/* 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. */ +void +__tunables_init (char **envp) +{ + char *envname = NULL; + char *envval = NULL; + size_t len = 0; + + maybe_disable_malloc_check (); + + while ((envp = get_next_env (envp, &envname, &len, &envval)) != NULL) + { + for (int i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) + { + tunable_t *cur = &tunable_list[i]; + + /* Skip over tunables that have either been set already or should be + skipped. */ + if (cur->strval != NULL || cur->env_alias == NULL + || (__libc_enable_secure && !cur->is_secure)) + continue; + + const char *name = cur->env_alias; + + /* We have a match. Initialize and move on to the next line. */ + if (is_name (name, envname)) + { + tunable_initialize (cur, envval); + break; + } + } + } +} + +/* Set the tunable value. This is called by the module that the tunable exists + in. */ +void +__tunable_set_val (tunable_id_t id, void *valp, tunable_callback_t callback) +{ + tunable_t *cur = &tunable_list[id]; + + /* Don't do anything if our tunable was not set during initialization or if + it failed validation. */ + if (cur->strval == NULL) + return; + + if (valp == NULL) + goto cb; + + switch (cur->type.type_code) + { + case TUNABLE_TYPE_INT_32: + { + *((int32_t *) valp) = (int32_t) cur->val.numval; + break; + } + case TUNABLE_TYPE_SIZE_T: + { + *((size_t *) valp) = (size_t) cur->val.numval; + break; + } + case TUNABLE_TYPE_STRING: + { + *((const char **)valp) = cur->val.strval; + break; + } + default: + __builtin_unreachable (); + } + +cb: + if (callback) + callback (&cur->val); +} diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h new file mode 100644 index 0000000..10a644b --- /dev/null +++ b/elf/dl-tunables.h @@ -0,0 +1,78 @@ +/* The tunable framework. See the README to know how to use the tunable in + a glibc module. + + Copyright (C) 2016 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/>. */ + +#ifndef _TUNABLES_H_ +# define _TUNABLES_H_ +# include <stddef.h> +# include "dl-tunable-types.h" + +/* A tunable. */ +struct _tunable +{ + const char *name; /* Internal name of the tunable. */ + tunable_type_t type; /* Data type of the tunable. */ + tunable_val_t val; /* The value. */ + const char *strval; /* The string containing the value, + points into envp. */ + bool is_secure; /* Whether the tunable must be read + even for setuid binaries. 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; /* The compatibility environment + variable name. */ +}; + +typedef struct _tunable tunable_t; + +/* Full name for a tunable is top_ns.tunable_ns.id. */ +#define TUNABLE_NAME_S(top,ns,id) #top "." #ns "." #id + +# define TUNABLE_ENUM_NAME(top,ns,id) TUNABLE_ENUM_NAME1 (top,ns,id) +# define TUNABLE_ENUM_NAME1(top,ns,id) top ## _ ## ns ## _ ## id + +#include "dl-tunable-list.h" + +extern void __tunables_init (char **); +extern void __tunable_set_val (tunable_id_t, void *, tunable_callback_t); + +/* Check if the tunable has been set to a non-default value and if it is, copy + it over into __VAL. */ +# define TUNABLE_SET_VAL(__id,__val) \ +({ \ + __tunable_set_val \ + (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val), \ + NULL); \ +}) + +/* Same as TUNABLE_SET_VAL, but also set the callback function to __CB and call + it. */ +# define TUNABLE_SET_VAL_WITH_CALLBACK(__id,__val,__cb) \ +({ \ + __tunable_set_val \ + (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val), \ + DL_TUNABLE_CALLBACK (__cb)); \ +}) + +/* Namespace sanity for callback functions. Use this macro to keep the + namespace of the modules clean. */ +#define DL_TUNABLE_CALLBACK(__name) _dl_tunable_ ## __name +#endif diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list new file mode 100644 index 0000000..11504c4 --- /dev/null +++ b/elf/dl-tunables.list @@ -0,0 +1,69 @@ +# Copyright (C) 2016 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/>. + +# Allowed attributes for tunables: +# +# type: Defaults to STRING +# minval: Optional minimum acceptable value +# maxval: Optional maximum acceptable value +# env_alias: An alias environment variable +# is_secure: Specify whether the environment variable should be read for +# setuid binaries. + +glibc { + malloc { + check { + type: INT_32 + minval: 0 + maxval: 3 + env_alias: MALLOC_CHECK_ + is_secure: true + } + top_pad { + type: SIZE_T + env_alias: MALLOC_TOP_PAD_ + } + perturb { + type: INT_32 + minval: 0 + maxval: 0xff + env_alias: MALLOC_PERTURB_ + } + mmap_threshold { + type: SIZE_T + env_alias: MALLOC_MMAP_THRESHOLD_ + } + trim_threshold { + type: SIZE_T + env_alias: MALLOC_TRIM_THRESHOLD_ + } + mmap_max { + type: INT_32 + env_alias: MALLOC_MMAP_MAX_ + } + arena_max { + type: SIZE_T + env_alias: MALLOC_ARENA_MAX + minval: 1 + } + arena_test { + type: SIZE_T + env_alias: MALLOC_ARENA_TEST + minval: 1 + } + } +} diff --git a/elf/rtld.c b/elf/rtld.c index 647661c..eb211a2 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -2527,11 +2527,13 @@ process_envvars (enum mode *modep) } while (*nextp != '\0'); +#if !HAVE_TUNABLES if (__access ("/etc/suid-debug", F_OK) != 0) { unsetenv ("MALLOC_CHECK_"); GLRO(dl_debug_mask) = 0; } +#endif if (mode != normal) _exit (5); diff --git a/malloc/Makefile b/malloc/Makefile index b8efcd6..4e4104e 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -37,6 +37,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ tests-static := \ tst-interpose-static-nothread \ tst-interpose-static-thread \ + tst-malloc-usable-static tests += $(tests-static) test-srcs = tst-mtrace @@ -158,6 +159,7 @@ endif tst-mcheck-ENV = MALLOC_CHECK_=3 tst-malloc-usable-ENV = MALLOC_CHECK_=3 +tst-malloc-usable-static-ENV = $(tst-malloc-usable-ENV) # Uncomment this for test releases. For public releases it is too expensive. #CPPFLAGS-malloc.o += -DMALLOC_DEBUG=1 diff --git a/malloc/arena.c b/malloc/arena.c index eed4247..c2791b2 100644 --- a/malloc/arena.c +++ b/malloc/arena.c @@ -19,6 +19,11 @@ #include <stdbool.h> +#if HAVE_TUNABLES +# define TUNABLE_NAMESPACE malloc +# include <elf/dl-tunables.h> +#endif + /* Compile-time constants. */ #define HEAP_MIN_SIZE (32 * 1024) @@ -204,6 +209,34 @@ __malloc_fork_unlock_child (void) __libc_lock_init (list_lock); } +#if HAVE_TUNABLES +static inline int do_set_mallopt_check (int32_t value); +void +DL_TUNABLE_CALLBACK (set_mallopt_check) (void *valp) +{ + int32_t value = *(int32_t *) valp; + do_set_mallopt_check (value); + if (check_action != 0) + __malloc_check_init (); +} + +# define DL_TUNABLE_CALLBACK_FNDECL(__name, __type) \ +static inline int do_ ## __name (__type value); \ +void \ +DL_TUNABLE_CALLBACK (__name) (void *valp) \ +{ \ + __type value = *(__type *) valp; \ + do_ ## __name (value); \ +} + +DL_TUNABLE_CALLBACK_FNDECL (set_mmap_threshold, size_t) +DL_TUNABLE_CALLBACK_FNDECL (set_mmaps_max, int32_t) +DL_TUNABLE_CALLBACK_FNDECL (set_top_pad, size_t) +DL_TUNABLE_CALLBACK_FNDECL (set_perturb_byte, int32_t) +DL_TUNABLE_CALLBACK_FNDECL (set_trim_threshold, size_t) +DL_TUNABLE_CALLBACK_FNDECL (set_arena_max, size_t) +DL_TUNABLE_CALLBACK_FNDECL (set_arena_test, size_t) +#else /* Initialization routine. */ #include <string.h> extern char **_environ; @@ -238,6 +271,7 @@ next_env_entry (char ***position) return result; } +#endif #ifdef SHARED @@ -272,6 +306,24 @@ ptmalloc_init (void) #endif thread_arena = &main_arena; + +#if HAVE_TUNABLES + /* Ensure initialization/consolidation and do it under a lock so that a + thread attempting to use the arena in parallel waits on us till we + finish. */ + __libc_lock_lock (main_arena.mutex); + malloc_consolidate (&main_arena); + + TUNABLE_SET_VAL_WITH_CALLBACK (check, NULL, set_mallopt_check); + TUNABLE_SET_VAL_WITH_CALLBACK (top_pad, NULL, set_top_pad); + TUNABLE_SET_VAL_WITH_CALLBACK (perturb, NULL, set_perturb_byte); + TUNABLE_SET_VAL_WITH_CALLBACK (mmap_threshold, NULL, set_mmap_threshold); + TUNABLE_SET_VAL_WITH_CALLBACK (trim_threshold, NULL, set_trim_threshold); + TUNABLE_SET_VAL_WITH_CALLBACK (mmap_max, NULL, set_mmaps_max); + TUNABLE_SET_VAL_WITH_CALLBACK (arena_max, NULL, set_arena_max); + TUNABLE_SET_VAL_WITH_CALLBACK (arena_test, NULL, set_arena_test); + __libc_lock_unlock (main_arena.mutex); +#else const char *s = NULL; if (__glibc_likely (_environ != NULL)) { @@ -340,6 +392,8 @@ ptmalloc_init (void) if (check_action != 0) __malloc_check_init (); } +#endif + #if HAVE_MALLOC_INIT_HOOK void (*hook) (void) = atomic_forced_read (__malloc_initialize_hook); if (hook != NULL) diff --git a/malloc/tst-malloc-usable-static.c b/malloc/tst-malloc-usable-static.c new file mode 100644 index 0000000..8907db0 --- /dev/null +++ b/malloc/tst-malloc-usable-static.c @@ -0,0 +1 @@ +#include <malloc/tst-malloc-usable.c> diff --git a/manual/install.texi b/manual/install.texi index de1c203..ffc166d 100644 --- a/manual/install.texi +++ b/manual/install.texi @@ -189,6 +189,11 @@ configure with @option{--disable-werror}. By default for x86_64, @theglibc{} is built with the vector math library. Use this option to disable the vector math library. +@item --enable-tunables +Tunables support allows additional library parameters to be customized at +runtime. This is an experimental feature and affects startup time and is thus +disabled by default. + @item --build=@var{build-system} @itemx --host=@var{host-system} These options are for cross-compiling. If you specify both options and diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk new file mode 100644 index 0000000..b65b5a4 --- /dev/null +++ b/scripts/gen-tunables.awk @@ -0,0 +1,157 @@ +# Generate dl-tunable-list.h from dl-tunables.list + +BEGIN { + tunable="" + ns="" + top_ns="" +} + +# Skip over blank lines and comments. +/^#/ { + next +} + +/^[ \t]*$/ { + next +} + +# Beginning of either a top namespace, tunable namespace or a tunable, decided +# on the current value of TUNABLE, NS or TOP_NS. +$2 == "{" { + if (top_ns == "") { + top_ns = $1 + } + else if (ns == "") { + ns = $1 + } + else if (tunable == "") { + tunable = $1 + } + else { + printf ("Unexpected occurrence of '{': %s:%d\n", FILENAME, FNR) + exit 1 + } + + next +} + +# End of either a top namespace, tunable namespace or a tunable. +$1 == "}" { + if (tunable != "") { + # Tunables definition ended, now fill in default attributes. + if (!types[top_ns][ns][tunable]) { + types[top_ns][ns][tunable] = "STRING" + } + if (!minvals[top_ns][ns][tunable]) { + minvals[top_ns][ns][tunable] = "0" + } + if (!maxvals[top_ns][ns][tunable]) { + maxvals[top_ns][ns][tunable] = "0" + } + if (!env_alias[top_ns][ns][tunable]) { + env_alias[top_ns][ns][tunable] = "NULL" + } + if (!is_secure[top_ns][ns][tunable]) { + is_secure[top_ns][ns][tunable] = "false" + } + + tunable = "" + } + else if (ns != "") { + ns = "" + } + else if (top_ns != "") { + top_ns = "" + } + else { + printf ("syntax error: extra }: %s:%d\n", FILENAME, FNR) + exit 1 + } + next +} + +# Everything else, which could either be a tunable without any attributes or a +# tunable attribute. +{ + if (ns == "") { + printf("Line %d: Invalid tunable outside a namespace: %s\n", NR, $0) + exit 1 + } + + if (tunable == "") { + # We encountered a tunable without any attributes, so note it with a + # default. + types[top_ns][ns][$1] = "STRING" + next + } + + # Otherwise, we have encountered a tunable attribute. + split($0, arr, ":") + attr = gensub(/^[ \t]+|[ \t]+$/, "", "g", arr[1]) + val = gensub(/^[ \t]+|[ \t]+$/, "", "g", arr[2]) + + if (attr == "type") { + types[top_ns][ns][tunable] = val + } + else if (attr == "minval") { + minvals[top_ns][ns][tunable] = val + } + else if (attr == "maxval") { + maxvals[top_ns][ns][tunable] = val + } + else if (attr == "env_alias") { + env_alias[top_ns][ns][tunable] = sprintf("\"%s\"", val) + } + else if (attr == "is_secure") { + if (val == "true" || val == "false") { + is_secure[top_ns][ns][tunable] = val + } + else { + printf("Line %d: Invalid value (%s) for is_secure: %s, ", NR, val, + $0) + print("Allowed values are 'true' or 'false'") + exit 1 + } + } +} + +END { + if (ns != "") { + print "Unterminated namespace. Is a closing brace missing?" + exit 1 + } + + print "/* AUTOGENERATED by gen-tunables.awk. */" + print "#ifndef _TUNABLES_H_" + print "# error \"Do not include this file directly.\"" + print "# error \"Include tunables.h instead.\"" + print "#endif" + + # Now, the enum names + print "\ntypedef enum" + print "{" + for (t in types) { + for (n in types[t]) { + for (m in types[t][n]) { + printf (" TUNABLE_ENUM_NAME(%s, %s, %s),\n", t, n, m); + } + } + } + print "} tunable_id_t;\n" + + # Finally, the tunable list. + print "\n#ifdef TUNABLES_INTERNAL" + print "static tunable_t tunable_list[] = {" + for (t in types) { + for (n in types[t]) { + for (m in types[t][n]) { + printf (" {TUNABLE_NAME_S(%s, %s, %s)", t, n, m) + printf (", {TUNABLE_TYPE_%s, %s, %s}, {.numval = 0}, NULL, %s, %s},\n", + types[t][n][m], minvals[t][n][m], maxvals[t][n][m], + is_secure[t][n][m], env_alias[t][n][m]); + } + } + } + print "};" + print "#endif" +} diff --git a/sysdeps/mach/hurd/dl-sysdep.c b/sysdeps/mach/hurd/dl-sysdep.c index d730f82..40b5be5 100644 --- a/sysdeps/mach/hurd/dl-sysdep.c +++ b/sysdeps/mach/hurd/dl-sysdep.c @@ -44,6 +44,10 @@ #include <dl-machine.h> #include <dl-procinfo.h> +#if HAVE_TUNABLES +# include <dl-tunables.h> +#endif + extern void __mach_init (void); extern int _dl_argc; @@ -143,6 +147,10 @@ _dl_sysdep_start (void **start_argptr, __libc_enable_secure = _dl_hurd_data->flags & EXEC_SECURE; +#if HAVE_TUNABLES + __tunables_init (_environ); +#endif + if (_dl_hurd_data->flags & EXEC_STACK_ARGS && _dl_hurd_data->user_entry == 0) _dl_hurd_data->user_entry = (vm_address_t) ENTRY_POINT; -- 2.7.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2016-11-16 8:35 ` [PATCH 1/4] Add framework for tunables Siddhesh Poyarekar @ 2016-12-27 11:08 ` Florian Weimer 2016-12-27 20:59 ` Siddhesh Poyarekar 2016-12-30 13:28 ` Joseph Myers 0 siblings, 2 replies; 27+ messages in thread From: Florian Weimer @ 2016-12-27 11:08 UTC (permalink / raw) To: Siddhesh Poyarekar, libc-alpha; +Cc: carlos, adhemerval.zanella On 11/16/2016 09:35 AM, Siddhesh Poyarekar wrote: > diff --git a/README.tunables b/README.tunables > +The tunable framework allows modules within glibc to register variables that > +may be tweaked through an environment variable or an API call. It aims to This seems outdated, change with an API call is no longer supported. > +enforce a strict namespace rule to bring consistency to naming of these tunable > +environment variables across the project. This document is a guide for glibc > +developers to add tunables to the framework. > + > +ADDING A NEW TUNABLE > +-------------------- > + > +The TOP_NAMESPACE is defined by default as 'glibc' and it may be overridden in “The TOP_NAMESPACE macro”? It's not clear based on this description if distributions are supposed to change the default definition for glibc as a whole (I don't think so), or where they are supposed to override the macro. > diff --git a/csu/libc-start.c b/csu/libc-start.c > index 99c040a..4e5cafa 100644 > --- a/csu/libc-start.c > +++ b/csu/libc-start.c > @@ -21,6 +21,11 @@ > #include <unistd.h> > #include <ldsodefs.h> > #include <exit-thread.h> > +#include <libc-internal.h> > + > +#if HAVE_TUNABLES > +# include <elf/dl-tunables.h> > +#endif I'd prefer if you could cut down the number of HAVE_TUNABLES conditionals. > + /* Initialize very early so that tunables can use it. */ > + __libc_init_secure (); > + > +#if HAVE_TUNABLES > + __tunables_init (__environ); > +#endif For !HAVE_TUNABLES, __tunables_init could be defined an empty inline function, so that no code is generated for it. > diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h > +#ifndef _TUNABLE_TYPES_H_ > +# define _TUNABLE_TYPES_H_ No indentation here. > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c > new file mode 100644 > index 0000000..91340b6 > --- /dev/null > +++ b/elf/dl-tunables.c > @@ -0,0 +1,310 @@ > +/* The tunable framework. See the README to know how to use the tunable in Should be “README.tunables”. Parts of this file should probably go into csu/ because the code runs so early during library startup (see my other comments). > +static int > +tunables_unsetenv (char **ep, const char *name) This is copied from elf/dl-environ.c, it seems. Is it possible to avoid creating this copy? Calling either form of unsetenv is problematic because it may make the auxiliary vector unreachable on !LIBC_START_MAIN_AUXVEC_ARG platforms (which seems to include the majority, so perhaps this is a non-issue). > +/* If the value does not validate, don't bother initializing the internal type > + and also take care to clear the recorded string value in STRVAL. */ > +#define RETURN_IF_INVALID_RANGE(__cur, __val) \ > +({ \ > + __typeof ((__cur)->type.min) min = (__cur)->type.min; \ > + __typeof ((__cur)->type.max) max = (__cur)->type.max; \ > + if (min != max && ((__val) < min || (__val) > max)) \ > + return; \ > +}) Is it really necessary to turn this into a macro instead of an (inline) function? The return statement could very well be located in the caller, IMHO. > +/* Disable a tunable if it is set. */ > +static void > +disable_tunable (tunable_id_t id, char **envp) > +{ > + const char *env_alias = tunable_list[id].env_alias; > + > + if (env_alias) > + tunables_unsetenv (envp, tunable_list[id].env_alias); > +} env_alias != NULL. > + default: > + __builtin_unreachable (); Should this rather be __bultin_trap? > diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h > +/* A tunable. */ > +struct _tunable > +{ > + const char *env_alias; /* The compatibility environment > + variable name. */ Maybe store the variable name as well, as to > +/* Same as TUNABLE_SET_VAL, but also set the callback function to __CB and call > + it. */ > +# define TUNABLE_SET_VAL_WITH_CALLBACK(__id,__val,__cb) \ Comment seems misleading, the callback function isn't really set. > diff --git a/elf/rtld.c b/elf/rtld.c > index 647661c..eb211a2 100644 > --- a/elf/rtld.c > +++ b/elf/rtld.c > @@ -2527,11 +2527,13 @@ process_envvars (enum mode *modep) > } > while (*nextp != '\0'); > > +#if !HAVE_TUNABLES > if (__access ("/etc/suid-debug", F_OK) != 0) > { > unsetenv ("MALLOC_CHECK_"); > GLRO(dl_debug_mask) = 0; > } > +#endif I believe this introduces a security vulnerability. You must keep the setting of dl_debug_mask. Thanks, Florian ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2016-12-27 11:08 ` Florian Weimer @ 2016-12-27 20:59 ` Siddhesh Poyarekar 2016-12-27 21:12 ` Florian Weimer 2016-12-30 13:28 ` Joseph Myers 1 sibling, 1 reply; 27+ messages in thread From: Siddhesh Poyarekar @ 2016-12-27 20:59 UTC (permalink / raw) To: Florian Weimer, libc-alpha; +Cc: carlos, adhemerval.zanella On Tuesday 27 December 2016 04:38 PM, Florian Weimer wrote: > On 11/16/2016 09:35 AM, Siddhesh Poyarekar wrote: >> diff --git a/README.tunables b/README.tunables > >> +The tunable framework allows modules within glibc to register >> variables that >> +may be tweaked through an environment variable or an API call. It >> aims to > > This seems outdated, change with an API call is no longer supported. Yeah, that's an old relic. Fixed >> +enforce a strict namespace rule to bring consistency to naming of >> these tunable >> +environment variables across the project. This document is a guide >> for glibc >> +developers to add tunables to the framework. >> + >> +ADDING A NEW TUNABLE >> +-------------------- >> + >> +The TOP_NAMESPACE is defined by default as 'glibc' and it may be >> overridden in > > “The TOP_NAMESPACE macro”? Fixed. > > It's not clear based on this description if distributions are supposed > to change the default definition for glibc as a whole (I don't think > so), or where they are supposed to override the macro. > I'll redo the description a bit. >> diff --git a/csu/libc-start.c b/csu/libc-start.c >> index 99c040a..4e5cafa 100644 >> --- a/csu/libc-start.c >> +++ b/csu/libc-start.c >> @@ -21,6 +21,11 @@ >> #include <unistd.h> >> #include <ldsodefs.h> >> #include <exit-thread.h> >> +#include <libc-internal.h> >> + >> +#if HAVE_TUNABLES >> +# include <elf/dl-tunables.h> >> +#endif > > I'd prefer if you could cut down the number of HAVE_TUNABLES conditionals. > >> + /* Initialize very early so that tunables can use it. */ >> + __libc_init_secure (); >> + >> +#if HAVE_TUNABLES >> + __tunables_init (__environ); >> +#endif > > For !HAVE_TUNABLES, __tunables_init could be defined an empty inline > function, so that no code is generated for it. > OK. >> diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h > >> +#ifndef _TUNABLE_TYPES_H_ >> +# define _TUNABLE_TYPES_H_ > > No indentation here. > >> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c >> new file mode 100644 >> index 0000000..91340b6 >> --- /dev/null >> +++ b/elf/dl-tunables.c >> @@ -0,0 +1,310 @@ >> +/* The tunable framework. See the README to know how to use the >> tunable in > > Should be “README.tunables”. OK. > Parts of this file should probably go into csu/ because the code runs so > early during library startup (see my other comments). OK, I'll need to figure out how to split things up. >> +static int >> +tunables_unsetenv (char **ep, const char *name) > > This is copied from elf/dl-environ.c, it seems. Is it possible to avoid > creating this copy? I'm going to eventually drop the dl-environ.c copy by moving the LD_* variables to tunables. > > Calling either form of unsetenv is problematic because it may make the > auxiliary vector unreachable on !LIBC_START_MAIN_AUXVEC_ARG platforms > (which seems to include the majority, so perhaps this is a non-issue). I'm not sure what the problem is here. Either way, it doesn't seem like something the patch introduces, is it? >> +/* If the value does not validate, don't bother initializing the >> internal type >> + and also take care to clear the recorded string value in STRVAL. */ >> +#define RETURN_IF_INVALID_RANGE(__cur, __val) \ >> +({ \ >> + __typeof ((__cur)->type.min) min = >> (__cur)->type.min; \ >> + __typeof ((__cur)->type.max) max = >> (__cur)->type.max; \ >> + if (min != max && ((__val) < min || (__val) > max)) \ >> + return; \ >> +}) > > Is it really necessary to turn this into a macro instead of an (inline) > function? The return statement could very well be located in the > caller, IMHO. I rewrote the function to explicitly check for bounds based on type. > >> +/* Disable a tunable if it is set. */ >> +static void >> +disable_tunable (tunable_id_t id, char **envp) >> +{ >> + const char *env_alias = tunable_list[id].env_alias; >> + >> + if (env_alias) >> + tunables_unsetenv (envp, tunable_list[id].env_alias); >> +} > > env_alias != NULL. > >> + default: >> + __builtin_unreachable (); > > Should this rather be __bultin_trap? OK. > >> diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h > >> +/* A tunable. */ >> +struct _tunable >> +{ > >> + const char *env_alias; /* The compatibility environment >> + variable name. */ > > Maybe store the variable name as well, as to I don't understand this, can you please elaborate? > >> +/* Same as TUNABLE_SET_VAL, but also set the callback function to >> __CB and call >> + it. */ >> +# define TUNABLE_SET_VAL_WITH_CALLBACK(__id,__val,__cb) \ > > Comment seems misleading, the callback function isn't really set. It used to be until I changed that. I'll fix the comment. > >> diff --git a/elf/rtld.c b/elf/rtld.c >> index 647661c..eb211a2 100644 >> --- a/elf/rtld.c >> +++ b/elf/rtld.c >> @@ -2527,11 +2527,13 @@ process_envvars (enum mode *modep) >> } >> while (*nextp != '\0'); >> >> +#if !HAVE_TUNABLES >> if (__access ("/etc/suid-debug", F_OK) != 0) >> { >> unsetenv ("MALLOC_CHECK_"); >> GLRO(dl_debug_mask) = 0; >> } >> +#endif > > I believe this introduces a security vulnerability. You must keep the > setting of dl_debug_mask. I'll add it back in #ifdef SHARED in disable_tunable. Siddhesh ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2016-12-27 20:59 ` Siddhesh Poyarekar @ 2016-12-27 21:12 ` Florian Weimer 2016-12-27 21:25 ` Siddhesh Poyarekar 0 siblings, 1 reply; 27+ messages in thread From: Florian Weimer @ 2016-12-27 21:12 UTC (permalink / raw) To: siddhesh, libc-alpha; +Cc: carlos, adhemerval.zanella On 12/27/2016 09:59 PM, Siddhesh Poyarekar wrote: >>> +static int >>> +tunables_unsetenv (char **ep, const char *name) >> >> This is copied from elf/dl-environ.c, it seems. Is it possible to avoid >> creating this copy? > > I'm going to eventually drop the dl-environ.c copy by moving the LD_* > variables to tunables. This is fine. Perhaps add a comment. >> Calling either form of unsetenv is problematic because it may make the >> auxiliary vector unreachable on !LIBC_START_MAIN_AUXVEC_ARG platforms >> (which seems to include the majority, so perhaps this is a non-issue). > > I'm not sure what the problem is here. If auxv isn't passed explicitly, the vector starts after the last NULL pointer in the envp array. If you put in additional NULL pointers there to remove element, that's not going to work anymore. > Either way, it doesn't seem like > something the patch introduces, is it? Probably not, but I don't have a complete picture in my head of the startup code. >>> diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h >> >>> +/* A tunable. */ >>> +struct _tunable >>> +{ >> >>> + const char *env_alias; /* The compatibility environment >>> + variable name. */ >> >> Maybe store the variable name as well, as to > > I don't understand this, can you please elaborate? I meant to write “variable name length”, so that it's possible to optimize the comparison somewhat. But it's likely better to keep the complexity down for now, so please disregard this suggestion. >> I believe this introduces a security vulnerability. You must keep the >> setting of dl_debug_mask. > > I'll add it back in #ifdef SHARED in disable_tunable. Are you sure this is sufficient? You need to make sure that the value is zeroed out again after it has been populated from the environment and before it is used in any way by the dynamic linker. Thanks, Florian ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2016-12-27 21:12 ` Florian Weimer @ 2016-12-27 21:25 ` Siddhesh Poyarekar 0 siblings, 0 replies; 27+ messages in thread From: Siddhesh Poyarekar @ 2016-12-27 21:25 UTC (permalink / raw) To: Florian Weimer, libc-alpha; +Cc: carlos, adhemerval.zanella On Wednesday 28 December 2016 02:41 AM, Florian Weimer wrote: > Are you sure this is sufficient? You need to make sure that the value > is zeroed out again after it has been populated from the environment and > before it is used in any way by the dynamic linker. Right, it isn't. I guess I'll be better off only avoiding the unsetenv in rtld.c for now when !HAVE_TUNABLES and clearing dl_debug_mask all the time. Siddhesh ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2016-12-27 11:08 ` Florian Weimer 2016-12-27 20:59 ` Siddhesh Poyarekar @ 2016-12-30 13:28 ` Joseph Myers 2016-12-31 13:48 ` Siddhesh Poyarekar 1 sibling, 1 reply; 27+ messages in thread From: Joseph Myers @ 2016-12-30 13:28 UTC (permalink / raw) To: Florian Weimer; +Cc: Siddhesh Poyarekar, libc-alpha, carlos, adhemerval.zanella On Tue, 27 Dec 2016, Florian Weimer wrote: > > + default: > > + __builtin_unreachable (); > > Should this rather be __bultin_trap? Note that __bultin_trap generates a call to abort on some platforms. It can only be used in places where such a call is OK; we've had problems with the isolate-erroneous-paths optimization generating such calls. So make sure anything using __bultin_trap builds OK for SH with build-many-glibcs.py, for example. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] Add framework for tunables 2016-12-30 13:28 ` Joseph Myers @ 2016-12-31 13:48 ` Siddhesh Poyarekar 0 siblings, 0 replies; 27+ messages in thread From: Siddhesh Poyarekar @ 2016-12-31 13:48 UTC (permalink / raw) To: Joseph Myers, Florian Weimer; +Cc: libc-alpha, carlos, adhemerval.zanella On Friday 30 December 2016 06:57 PM, Joseph Myers wrote: > Note that __bultin_trap generates a call to abort on some platforms. It > can only be used in places where such a call is OK; we've had problems > with the isolate-erroneous-paths optimization generating such calls. So > make sure anything using __bultin_trap builds OK for SH with > build-many-glibcs.py, for example. Oh, then I think I'll stick to __builtin_unreachable for now and then maybe post an addon patch to use __builtin_trap once I have verified all target builds. I'm setting up the tree for build-many-glibcs right now. Siddhesh ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2017-02-08 8:48 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-30 4:09 [PATCH v7 0/4] glibc tunables Siddhesh Poyarekar 2016-12-30 4:09 ` [PATCH 1/4] Add framework for tunables Siddhesh Poyarekar 2017-02-06 22:36 ` Andreas Schwab 2017-02-07 6:49 ` Siddhesh Poyarekar 2017-02-07 16:06 ` Andreas Schwab 2017-02-08 8:48 ` Siddhesh Poyarekar 2017-02-06 22:59 ` Andreas Schwab 2016-12-30 4:09 ` [PATCH 3/4] Enhance --enable-tunables to select tunables frontend at build time Siddhesh Poyarekar 2016-12-30 14:55 ` Joseph Myers 2016-12-30 4:09 ` [PATCH 4/4] User manual documentation for tunables Siddhesh Poyarekar 2016-12-30 9:50 ` Florian Weimer 2016-12-31 12:40 ` Siddhesh Poyarekar 2016-12-30 4:09 ` [PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable Siddhesh Poyarekar 2016-12-30 9:50 ` Florian Weimer -- strict thread matches above, loose matches on Subject: below -- 2016-12-31 15:41 [PATCH v8 0/4] glibc tunables Siddhesh Poyarekar [not found] ` <1483198831-2232-2-git-send-email-siddhesh@sourceware.org> 2016-12-31 16:49 ` [PATCH 1/4] Add framework for tunables Florian Weimer 2016-12-31 17:58 ` Siddhesh Poyarekar 2016-12-31 19:43 ` Florian Weimer 2016-12-31 19:50 ` Siddhesh Poyarekar 2016-12-31 19:52 ` Florian Weimer 2017-01-01 13:40 ` Nix 2016-11-16 8:35 [PATCHv6 0/4] glibc tunables Siddhesh Poyarekar 2016-11-16 8:35 ` [PATCH 1/4] Add framework for tunables Siddhesh Poyarekar 2016-12-27 11:08 ` Florian Weimer 2016-12-27 20:59 ` Siddhesh Poyarekar 2016-12-27 21:12 ` Florian Weimer 2016-12-27 21:25 ` Siddhesh Poyarekar 2016-12-30 13:28 ` Joseph Myers 2016-12-31 13:48 ` Siddhesh Poyarekar
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).