Message ID | 387267b5cd50f268056db8c89e68fac800959c15.1610471272.git.szabolcs.nagy@arm.com |
---|---|
State | New |
Headers | show |
Series | fix ifunc with static pie [BZ #27072] | expand |
On 12/01/2021 14:22, Szabolcs Nagy via Libc-alpha wrote: > With static pie linking pointers in the tunables list need > RELATIVE relocs since the absolute address is not known at link > time. We want to avoid relocations so the static pie self > relocation can be done after tunables are initialized. > > This is a simple fix that embeds the tunable strings into the > tunable list instead of using pointers. It is possible to have > a more compact representation of tunables with some additional > complexity in the generator and tunable parser logic. Such > optimization will be useful if the list of tunables grows. > > There is still an issue that tunables_strdup allocates and the > failure handling code path is sufficiently complex that it can > easily have RELATIVE relocations. It is possible to avoid the > early allocation and only change environment variables in a > setuid exe after relocations are processed. But that is a > bigger change and early failure is fatal anyway so it is not > as critical to fix right away. The _dl_fatal_printf is used quite earlier in the loader, does still represent a potential failure? If so I think it would be good to open a bug report to track it. LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > elf/dl-tunable-types.h | 4 ++-- > elf/dl-tunables.c | 2 +- > scripts/gen-tunables.awk | 12 +++++++++++- > 3 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h > index 05d4958e1c..3fcc0806f5 100644 > --- a/elf/dl-tunable-types.h > +++ b/elf/dl-tunable-types.h > @@ -59,7 +59,7 @@ typedef enum > /* A tunable. */ > struct _tunable > { > - const char *name; /* Internal name of the tunable. */ > + const char name[TUNABLE_NAME_MAX]; /* Internal name of the tunable. */ > tunable_type_t type; /* Data type of the tunable. */ > tunable_val_t val; /* The value. */ > bool initialized; /* Flag to indicate that the tunable is > @@ -75,7 +75,7 @@ struct _tunable > target module if the value is > considered unsafe. */ > /* Compatibility elements. */ > - const char *env_alias; /* The compatibility environment > + const char env_alias[TUNABLE_ALIAS_MAX]; /* The compatibility environment > variable name. */ > }; > Ok. > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c > index 9b4d737fb8..3845b2c04e 100644 > --- a/elf/dl-tunables.c > +++ b/elf/dl-tunables.c > @@ -350,7 +350,7 @@ __tunables_init (char **envp) > > /* Skip over tunables that have either been set already or should be > skipped. */ > - if (cur->initialized || cur->env_alias == NULL) > + if (cur->initialized || cur->env_alias[0] == '\0') > continue; > > const char *name = cur->env_alias; Ok. > diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk > index cda12ef62e..fa63e86d1a 100644 > --- a/scripts/gen-tunables.awk > +++ b/scripts/gen-tunables.awk > @@ -12,6 +12,8 @@ BEGIN { > tunable="" > ns="" > top_ns="" > + max_name_len=0 > + max_alias_len=0 > } > > # Skip over blank lines and comments. > @@ -57,11 +59,14 @@ $1 == "}" { > maxvals[top_ns,ns,tunable] = max_of[types[top_ns,ns,tunable]] > } > if (!env_alias[top_ns,ns,tunable]) { > - env_alias[top_ns,ns,tunable] = "NULL" > + env_alias[top_ns,ns,tunable] = "{0}" > } > if (!security_level[top_ns,ns,tunable]) { > security_level[top_ns,ns,tunable] = "SXID_ERASE" > } > + len = length(top_ns"."ns"."tunable) > + if (len > max_name_len) > + max_name_len = len > > tunable = "" > } > @@ -109,6 +114,9 @@ $1 == "}" { > } > else if (attr == "env_alias") { > env_alias[top_ns,ns,tunable] = sprintf("\"%s\"", val) > + len = length(val) > + if (len > max_alias_len) > + max_alias_len = len > } > else if (attr == "security_level") { > if (val == "SXID_ERASE" || val == "SXID_IGNORE" || val == "NONE") { > @@ -158,6 +166,8 @@ END { > > print "\n#ifdef TUNABLES_INTERNAL" > # Internal definitions. > + print "# define TUNABLE_NAME_MAX " (max_name_len + 1) > + print "# define TUNABLE_ALIAS_MAX " (max_alias_len + 1) > print "# include \"dl-tunable-types.h\"" > # Finally, the tunable list. > print "static tunable_t tunable_list[] attribute_relro = {" > Ok.
diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h index 05d4958e1c..3fcc0806f5 100644 --- a/elf/dl-tunable-types.h +++ b/elf/dl-tunable-types.h @@ -59,7 +59,7 @@ typedef enum /* A tunable. */ struct _tunable { - const char *name; /* Internal name of the tunable. */ + const char name[TUNABLE_NAME_MAX]; /* Internal name of the tunable. */ tunable_type_t type; /* Data type of the tunable. */ tunable_val_t val; /* The value. */ bool initialized; /* Flag to indicate that the tunable is @@ -75,7 +75,7 @@ struct _tunable target module if the value is considered unsafe. */ /* Compatibility elements. */ - const char *env_alias; /* The compatibility environment + const char env_alias[TUNABLE_ALIAS_MAX]; /* The compatibility environment variable name. */ }; diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index 9b4d737fb8..3845b2c04e 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -350,7 +350,7 @@ __tunables_init (char **envp) /* Skip over tunables that have either been set already or should be skipped. */ - if (cur->initialized || cur->env_alias == NULL) + if (cur->initialized || cur->env_alias[0] == '\0') continue; const char *name = cur->env_alias; diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk index cda12ef62e..fa63e86d1a 100644 --- a/scripts/gen-tunables.awk +++ b/scripts/gen-tunables.awk @@ -12,6 +12,8 @@ BEGIN { tunable="" ns="" top_ns="" + max_name_len=0 + max_alias_len=0 } # Skip over blank lines and comments. @@ -57,11 +59,14 @@ $1 == "}" { maxvals[top_ns,ns,tunable] = max_of[types[top_ns,ns,tunable]] } if (!env_alias[top_ns,ns,tunable]) { - env_alias[top_ns,ns,tunable] = "NULL" + env_alias[top_ns,ns,tunable] = "{0}" } if (!security_level[top_ns,ns,tunable]) { security_level[top_ns,ns,tunable] = "SXID_ERASE" } + len = length(top_ns"."ns"."tunable) + if (len > max_name_len) + max_name_len = len tunable = "" } @@ -109,6 +114,9 @@ $1 == "}" { } else if (attr == "env_alias") { env_alias[top_ns,ns,tunable] = sprintf("\"%s\"", val) + len = length(val) + if (len > max_alias_len) + max_alias_len = len } else if (attr == "security_level") { if (val == "SXID_ERASE" || val == "SXID_IGNORE" || val == "NONE") { @@ -158,6 +166,8 @@ END { print "\n#ifdef TUNABLES_INTERNAL" # Internal definitions. + print "# define TUNABLE_NAME_MAX " (max_name_len + 1) + print "# define TUNABLE_ALIAS_MAX " (max_alias_len + 1) print "# include \"dl-tunable-types.h\"" # Finally, the tunable list. print "static tunable_t tunable_list[] attribute_relro = {"