From patchwork Wed Jun 21 13:26:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 21166 Received: (qmail 107215 invoked by alias); 21 Jun 2017 13:26:09 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 107197 invoked by uid 89); 21 Jun 2017 13:26:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.8 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-oi0-f66.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=9tf3mlLvl/JXpsjTSI18YFgAhNIuZasbOYxqOzHQwMk=; b=WFwodmgxsDh3bpnFw/qabCn32HkTxuux3z11jB9u2YY3qN6L3nyFodaa7eXuQAOny9 d8cwZySMaDjtGiz1AX2UvbwhbOedjDeG3IukN74nUSXXcrptbBr4w5sTGQGc6c5TAVip HnrN+ZUqgxqomnPu5QDc1U4n5gTVxFAispQtGgE0UtKDlGnvHu32zrFDYbWxmDawJLdr GpwReIO//+mQgyFFTeCKUEzT4oIwSmr5gdA165Vlb87Ba2PdrmmX8gcgF9yvq5nGOesF 5jjHgl86lea1uPg2X9SGwvMpe2/0XhUQ4+TR48+rvZMHUrR2LE3Xmsn0TAsJtea+AZeS 700A== X-Gm-Message-State: AKS2vOybB6m8jrybvHdI3rLqlx5PId9Af/lUv7votP/9qmhAUMfkmKOb owA72TgzxkfdSdRhwxPkhm00grreEQ== X-Received: by 10.202.104.144 with SMTP id o16mr18005819oik.158.1498051564721; Wed, 21 Jun 2017 06:26:04 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <86ed2014-1600-55da-b8ae-35405a36cdf8@gotplt.org> References: <20170615131042.GA28885@gmail.com> <63707191-601d-9374-8cad-74f15d51f917@linaro.org> <658e5dbb-d93e-9ee8-8eaa-52d750e6d977@gotplt.org> <86ed2014-1600-55da-b8ae-35405a36cdf8@gotplt.org> From: "H.J. Lu" Date: Wed, 21 Jun 2017 06:26:03 -0700 Message-ID: Subject: Re: [PATCH] tunables: Add IFUNC selection and cache sizes To: Siddhesh Poyarekar Cc: Adhemerval Zanella , GNU C Library On Wed, Jun 21, 2017 at 5:44 AM, Siddhesh Poyarekar wrote: > On Wednesday 21 June 2017 05:55 PM, H.J. Lu wrote: >>> Why do you need these to be inherited by children of setuid processes? >>> If not, then you should remove the SXID_IGNORE and let the default (i.e. >>> SXID_ERASE) prevail so that setuid processes and their children do not >>> see these tunables. >> >> I want to be conservative. The default ones should be good for 99.9% >> of applications. > > The conservative (in terms of security) should be the default, i.e. > disallow envvars to be read across setxid boundaries. If an escalation > of privileges is required, there should be a specifically stated reason > to do so. The current envvars need a thorough analysis too to figure > out if they need to be read across setxid boundaries. Done. >>> Also you've specified these as size_t but the values seem to get stored >>> in long int. Please fix them. >> >> tunables supports: >> >> TUNABLE_TYPE_INT_32, >> TUNABLE_TYPE_UINT_64, >> TUNABLE_TYPE_SIZE_T, >> TUNABLE_TYPE_STRING >> >> There is no long and on x86, long == size_t: >> >> [hjl@gnu-tools-1 tmp]$ cat f.c >> #include >> >> int x [(sizeof (size_t) == sizeof (long)) ? 1 : -1]; >> [hjl@gnu-tools-1 tmp]$ gcc -S f.c -m64 >> [hjl@gnu-tools-1 tmp]$ gcc -S f.c -m32 >> [hjl@gnu-tools-1 tmp]$ gcc -S f.c -mx32 >> [hjl@gnu-tools-1 tmp]$ > > Sure, but one is signed and the other isn't, which messes with > validation of the tunable. If long is what you need, you could add > ssize_t, but I don't see why the sizes should be negative. It seems > like instead of making the tunables signed, you should be making the > caches unsigned. Done. >> On x86, there are CPU features, like SSE, AVX, ... and ARCH features, >> like slow bsf. X86 tunables allow users to disable AVX and slow bsf. >> They don't change any CPU names. > > Ah OK then you're right, all of those should be hwcaps too and go into > x86 - I thought you were suggesting something like the mtune options, > i.e. corei7. I'll add the glibc.tune.cpu myself. > The supported x86 platforms are listed in sysdeps/x86/dl-procinfo.c. x86 tunables will influence setting of dl_platform, which shouldn't set arbitrarily. >>> Since #name is going to be a constant stirng, you could just use the >>> is_name from dl-tunables.c. Just pull it out into dl-tunables.h as a >>> static inline. >> >> memcmp is used elsewhere in ld.so and it is much faster than >> is_name in dl-tunables.c. I prefer to keep memcmp here. > > Fair enough. > I am testing this patch. OK for master if there are no regressions? From a724bae730245fe77bdd7ad280f24fbc00debe82 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Wed, 21 Jun 2017 05:38:03 -0700 Subject: [PATCH] x86: Rename glibc.tune.ifunc to glibc.tune.hwcaps Rename glibc.tune.ifunc to glibc.tune.hwcaps and move it to sysdeps/x86/dl-tunables.list since it is x86 specicifc. Also change type of data_cache_size, data_cache_size and non_temporal_threshold to unsigned long int to match size_t. Remove usage DEFAULT_STRLEN from cpu-tunables.c. * elf/dl-tunables.list (glibc.tune.ifunc): Removed. * sysdeps/x86/dl-tunables.list (glibc.tune.hwcaps): New. Remove security_level on all fields. * manual/tunables.texi: Replace ifunc with hwcaps. * sysdeps/x86/cpu-features.c (TUNABLE_CALLBACK (set_ifunc)): Renamed to .. (TUNABLE_CALLBACK (set_hwcaps)): This. (init_cpu_features): Updated. * sysdeps/x86/cpu-features.h (cpu_features): Change type of data_cache_size, data_cache_size and non_temporal_threshold to unsigned long int. * sysdeps/x86/cpu-tunables.c (DEFAULT_STRLEN): Removed. (TUNABLE_CALLBACK (set_ifunc)): Renamed to ... (TUNABLE_CALLBACK (set_hwcaps)): This. Update comments. Don't use DEFAULT_STRLEN. --- elf/dl-tunables.list | 4 ---- manual/tunables.texi | 8 ++++---- sysdeps/x86/cpu-features.c | 4 ++-- sysdeps/x86/cpu-features.h | 6 +++--- sysdeps/x86/cpu-tunables.c | 28 +++++++++++++--------------- sysdeps/x86/dl-tunables.list | 6 +++--- 6 files changed, 25 insertions(+), 31 deletions(-) diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list index b8b0ce5..df4f962 100644 --- a/elf/dl-tunables.list +++ b/elf/dl-tunables.list @@ -83,9 +83,5 @@ glibc { env_alias: LD_HWCAP_MASK default: HWCAP_IMPORTANT } - ifunc { - type: STRING - security_level: SXID_IGNORE - } } } diff --git a/manual/tunables.texi b/manual/tunables.texi index 3263f94..689e894 100644 --- a/manual/tunables.texi +++ b/manual/tunables.texi @@ -198,8 +198,8 @@ is 8 times the number of cores online. @cindex hardware capability tunables @cindex hwcap tunables @cindex tunables, hwcap -@cindex ifunc tunables -@cindex tunables, ifunc +@cindex hwcaps tunables +@cindex tunables, hwcaps @cindex data_cache_size tunables @cindex tunables, data_cache_size @cindex shared_cache_size tunables @@ -222,8 +222,8 @@ extensions available in the processor at runtime for some architectures. The capabilities at runtime, thus disabling use of those extensions. @end deftp -@deftp Tunable glibc.tune.ifunc -The @code{glibc.tune.ifunc=-xxx,yyy,-zzz...} tunable allows the user to +@deftp Tunable glibc.tune.hwcaps +The @code{glibc.tune.hwcaps=-xxx,yyy,-zzz...} tunable allows the user to enable CPU/ARCH feature @code{yyy}, disable CPU/ARCH feature @code{xxx} and @code{zzz} where the feature name is case-sensitive and has to match the ones in @code{sysdeps/x86/cpu-features.h}. diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c index 76f053a..1d087ea 100644 --- a/sysdeps/x86/cpu-features.c +++ b/sysdeps/x86/cpu-features.c @@ -25,7 +25,7 @@ # include /* Get STDOUT_FILENO for _dl_printf. */ # include -extern void TUNABLE_CALLBACK (set_ifunc) (tunable_val_t *) +extern void TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *) attribute_hidden; #endif @@ -322,7 +322,7 @@ no_cpuid: cpu_features->kind = kind; #if HAVE_TUNABLES - TUNABLE_GET (ifunc, tunable_val_t *, TUNABLE_CALLBACK (set_ifunc)); + TUNABLE_GET (hwcaps, tunable_val_t *, TUNABLE_CALLBACK (set_hwcaps)); cpu_features->non_temporal_threshold = TUNABLE_GET (x86_non_temporal_threshold, long int, NULL); cpu_features->data_cache_size diff --git a/sysdeps/x86/cpu-features.h b/sysdeps/x86/cpu-features.h index fef5e18..3ed67f5 100644 --- a/sysdeps/x86/cpu-features.h +++ b/sysdeps/x86/cpu-features.h @@ -217,12 +217,12 @@ struct cpu_features unsigned int feature[FEATURE_INDEX_MAX]; /* Data cache size for use in memory and string routines, typically L1 size. */ - long int data_cache_size; + unsigned long int data_cache_size; /* Shared cache size for use in memory and string routines, typically L2 or L3 size. */ - long int shared_cache_size; + unsigned long int shared_cache_size; /* Threshold to use non temporal store. */ - long int non_temporal_threshold; + unsigned long int non_temporal_threshold; }; /* Used from outside of glibc to get access to the CPU features diff --git a/sysdeps/x86/cpu-tunables.c b/sysdeps/x86/cpu-tunables.c index 9258fb4..d6f16a1 100644 --- a/sysdeps/x86/cpu-tunables.c +++ b/sysdeps/x86/cpu-tunables.c @@ -31,16 +31,12 @@ # if defined USE_MULTIARCH && !defined SHARED # ifdef __x86_64__ # define DEFAULT_MEMCMP __memcmp_sse2 -# define DEFAULT_STRLEN __strlen_sse2 # else # define DEFAULT_MEMCMP __memcmp_ia32 -# define DEFAULT_STRLEN strlen # endif extern __typeof (memcmp) DEFAULT_MEMCMP; -extern __typeof (strlen) DEFAULT_STRLEN; # else # define DEFAULT_MEMCMP memcmp -# define DEFAULT_STRLEN strlen # endif # define CHECK_GLIBC_IFUNC_CPU_OFF(f, cpu_features, name, len) \ @@ -112,22 +108,25 @@ extern __typeof (strlen) DEFAULT_STRLEN; attribute_hidden void -TUNABLE_CALLBACK (set_ifunc) (tunable_val_t *valp) +TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp) { /* The current IFUNC selection is based on microbenchmarks in glibc. It should give the best performance for most workloads. But other choices may have better performance for a particular workload or on the hardware which wasn't available when the selection was made. - The environment variable, GLIBC_IFUNC=-xxx,yyy,-zzz...., can be - used to enable CPU/ARCH feature yyy, disable CPU/ARCH feature yyy - and zzz, where the feature name is case-sensitive and has to match - the ones in cpu-features.h. It can be used by glibc developers to - tune for a new processor or override the IFUNC selection to improve - performance for a particular workload. + The environment variable: + + GLIBC_TUNABLES=glibc.tune.hwcaps=-xxx,yyy,-zzz,.... + + can be used to enable CPU/ARCH feature yyy, disable CPU/ARCH feature + yyy and zzz, where the feature name is case-sensitive and has to + match the ones in cpu-features.h. It can be used by glibc developers + to tune for a new processor or override the IFUNC selection to + improve performance for a particular workload. Since all CPU/ARCH features are hardware optimizations without security implication, except for Prefer_MAP_32BIT_EXEC, which can - only be disabled, we check GLIBC_IFUNC for programs, including + only be disabled, we check glibc.tune.hwcaps for programs, including set*id ones. NOTE: the IFUNC selection may change over time. Please check all @@ -135,7 +134,6 @@ TUNABLE_CALLBACK (set_ifunc) (tunable_val_t *valp) const char *p = valp->strval; struct cpu_features *cpu_features = &GLRO(dl_x86_cpu_features); - const char *end = p + DEFAULT_STRLEN (p); size_t len; do @@ -145,7 +143,7 @@ TUNABLE_CALLBACK (set_ifunc) (tunable_val_t *valp) size_t nl; for (c = p; *c != ','; c++) - if (c >= end) + if (*c == '\0') break; len = c - p; @@ -325,6 +323,6 @@ TUNABLE_CALLBACK (set_ifunc) (tunable_val_t *valp) } p += len + 1; } - while (p < end); + while (*p != '\0'); } #endif diff --git a/sysdeps/x86/dl-tunables.list b/sysdeps/x86/dl-tunables.list index 50c130a..99a9cc4 100644 --- a/sysdeps/x86/dl-tunables.list +++ b/sysdeps/x86/dl-tunables.list @@ -18,17 +18,17 @@ glibc { tune { + hwcaps { + type: STRING + } x86_non_temporal_threshold { type: SIZE_T - security_level: SXID_IGNORE } x86_data_cache_size { type: SIZE_T - security_level: SXID_IGNORE } x86_shared_cache_size { type: SIZE_T - security_level: SXID_IGNORE } } } -- 2.9.4