| Message ID | 20240625091719.2084892-1-stli@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | elf/rtld: Fix auxiliary vector for enable_secure | expand |
On 25/06/24 06:17, Stefan Liebler wrote: > Starting with commit > 59974938fe1f4add843f5325f78e2a7ccd8db853 > elf/rtld: Count skipped environment variables for enable_secure > > The new testcase elf/tst-tunables-enable_secure-env segfaults on s390 (31bit). > There _start parses the auxiliary vector for some additional checks. > > Therefore it skips over the zeros after the environment variables ... > 0x7fffac20: 0x7fffbd17 0x7fffbd32 0x7fffbd69 0x00000000 > ------------------------------------------------^^^last environment variable > > ... and then it parses the auxiliary vector and stops at AT_NULL. > 0x7fffac30: 0x00000000 0x00000021 0x00000000 0x00000000 > --------------------------------^^^AT_SYSINFO_EHDR--------------^^^AT_NULL > ----------------^^^newp-----------------------------------------^^^oldp > Afterwards it tries to access AT_PHDR which points to somewhere and segfaults. > > Due to not incorporating the skip_env variable in the computation of oldp > when shuffling down the auxv in rtld.c, it just copies one entry with AT_NULL > and value 0x00000021 and stops the loop. In reality we have skipped > GLIBC_TUNABLES environment variable (=> skip_env=1). Thus we should copy from > here: > 0x7fffac40: 0x00000021 0x7ffff000 0x00000010 0x007fffff > ----------------^^^fixed-oldp > > This patch fixes the computation of oldp when shuffling down auxiliary vector. > It also adds some checks in the testcase. Those checks also fail on > s390x (64bit) and x86_64 without the fix. > --- > elf/rtld.c | 2 +- > elf/tst-tunables-enable_secure-env.c | 19 +++++++++++++++++++ > 2 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/elf/rtld.c b/elf/rtld.c > index e9525ea987..883c651d48 100644 > --- a/elf/rtld.c > +++ b/elf/rtld.c > @@ -1325,7 +1325,7 @@ _dl_start_args_adjust (int skip_args, int skip_env) > > /* Shuffle auxv down. */ > ElfW(auxv_t) ax; > - char *oldp = (char *) (p + 1); > + char *oldp = (char *) (p + 1 + skip_env); > char *newp = (char *) (sp + 1); > do > { I don't think this is correct, running as the testcase it does show the expected value: $ GLIBC_TUNABLES=glibc.rtld.enable_secure=1 \ elf/ld-linux-x86-64.so.2 --library-path ... elf/tst-tunables-enable_secure-env --direct AT_EXECFD: 0 AT_EXECFN: [...]/elf/tst-tunables-enable_secure-env AT_PHDR: 0x7e7f608b4040 AT_PHENT: 56 AT_PHNUM: 13 AT_PAGESZ: 4096 AT_BASE: 0x0 AT_FLAGS: 0x0 AT_ENTRY: 0x7e7f608b6760 AT_NOTELF: 0 AT_UID: 1000 AT_EUID: 1000 AT_GID: 1000 AT_EGID: 1000 AT_PLATFORM: x86_64 AT_HWCAP: 2 AT_CLKTCK: 100 AT_FPUCW: 0 AT_DCACHEBSIZE: 0x0 AT_ICACHEBSIZE: 0x0 AT_UCACHEBSIZE: 0x0 AT_IGNOREPPC(null) AT_SECURE: 0 AT_BASE_PLATFORM: (null) AT_SYSINFO: 0x0 AT_SYSINFO_EHDR: 0x0 AT_RANDOM: 0x7ffe90102d29 AT_HWCAP2: 0x2 AT_HWCAP3: 0x0 AT_HWCAP4: 0x0 AT_MINSIGSTKSZ: 3376 AT_L1I_CACHESIZE: 0 AT_L1I_CACHEGEOMETRY: 0x0 AT_L1D_CACHESIZE: 0 AT_L1D_CACHEGEOMETRY: 0x0 AT_L2_CACHESIZE: 0 AT_L2_CACHEGEOMETRY: 0x0 AT_L3_CACHESIZE: 0 AT_L3_CACHEGEOMETRY: 0x0 However if I add environment that should be filtered: $ GLIBC_TUNABLES=glibc.rtld.enable_secure=1 \ LD_SHOW_AUXV=1 \ elf/ld-linux-x86-64.so.2 --library-path ... elf/tst-tunables-enable_secure-env --direct AT_EXECFD: 0 AT_EXECFN: (null) AT_PHDR: 0x0 AT_PHENT: 0 AT_PHNUM: 0 AT_PAGESZ: 0 AT_BASE: 0x0 AT_FLAGS: 0x0 AT_ENTRY: 0x0 AT_NOTELF: 0 AT_UID: 0 AT_EUID: 0 AT_GID: 0 AT_EGID: 0 AT_PLATFORM: (null) AT_HWCAP: 2 AT_CLKTCK: 0 AT_FPUCW: 0 AT_DCACHEBSIZE: 0x0 AT_ICACHEBSIZE: 0x0 AT_UCACHEBSIZE: 0x0 AT_IGNOREPPC(null) AT_SECURE: 0 AT_BASE_PLATFORM: (null) AT_SYSINFO: 0x0 AT_SYSINFO_EHDR: 0x0 AT_RANDOM: 0x0 AT_HWCAP2: 0x2 AT_HWCAP3: 0x0 AT_HWCAP4: 0x0 AT_MINSIGSTKSZ: 0 AT_L1I_CACHESIZE: 0 AT_L1I_CACHEGEOMETRY: 0x0 AT_L1D_CACHESIZE: 0 AT_L1D_CACHEGEOMETRY: 0x0 AT_L2_CACHESIZE: 0 AT_L2_CACHEGEOMETRY: 0x0 AT_L3_CACHESIZE: 0 AT_L3_CACHEGEOMETRY: 0x0 > diff --git a/elf/tst-tunables-enable_secure-env.c b/elf/tst-tunables-enable_secure-env.c > index 24e846f299..c78e113c21 100644 > --- a/elf/tst-tunables-enable_secure-env.c > +++ b/elf/tst-tunables-enable_secure-env.c > @@ -17,8 +17,12 @@ > License along with the GNU C Library; if not, see > <https://www.gnu.org/licenses/>. */ > > +#include <stdlib.h> > #include <support/capture_subprocess.h> > #include <support/check.h> > +#ifdef __linux__ > + #include <sys/auxv.h> > +#endif > > static int > do_test (int argc, char *argv[]) > @@ -26,6 +30,21 @@ do_test (int argc, char *argv[]) > /* Ensure that no assertions are hit when a dynamically linked application > runs. This test requires that GLIBC_TUNABLES=glibc.rtld.enable_secure=1 > is set. */ > + > + /* The environment variable GLIBC_TUNABLES is skipped for secure > + execution. */ > + TEST_VERIFY (getenv ("GLIBC_TUNABLES") == NULL); > + > +#ifdef __linux__ > + /* On linux, the auxiliary vector is located after the environment variables. > + Check that some of them are available as those are also shuffled down by > + ld.so. */ > + TEST_VERIFY (getauxval (AT_PHDR) != 0); > + TEST_VERIFY (getauxval (AT_PHENT) != 0); > + TEST_VERIFY (getauxval (AT_PHNUM) != 0); > + TEST_VERIFY (getauxval (AT_ENTRY) != 0); > +#endif > + > return 0; > } >
On 27.06.24 22:40, Adhemerval Zanella Netto wrote: > > > On 25/06/24 06:17, Stefan Liebler wrote: >> Starting with commit >> 59974938fe1f4add843f5325f78e2a7ccd8db853 >> elf/rtld: Count skipped environment variables for enable_secure >> >> The new testcase elf/tst-tunables-enable_secure-env segfaults on s390 (31bit). >> There _start parses the auxiliary vector for some additional checks. >> >> Therefore it skips over the zeros after the environment variables ... >> 0x7fffac20: 0x7fffbd17 0x7fffbd32 0x7fffbd69 0x00000000 >> ------------------------------------------------^^^last environment variable >> >> ... and then it parses the auxiliary vector and stops at AT_NULL. >> 0x7fffac30: 0x00000000 0x00000021 0x00000000 0x00000000 >> --------------------------------^^^AT_SYSINFO_EHDR--------------^^^AT_NULL >> ----------------^^^newp-----------------------------------------^^^oldp >> Afterwards it tries to access AT_PHDR which points to somewhere and segfaults. >> >> Due to not incorporating the skip_env variable in the computation of oldp >> when shuffling down the auxv in rtld.c, it just copies one entry with AT_NULL >> and value 0x00000021 and stops the loop. In reality we have skipped >> GLIBC_TUNABLES environment variable (=> skip_env=1). Thus we should copy from >> here: >> 0x7fffac40: 0x00000021 0x7ffff000 0x00000010 0x007fffff >> ----------------^^^fixed-oldp >> >> This patch fixes the computation of oldp when shuffling down auxiliary vector. >> It also adds some checks in the testcase. Those checks also fail on >> s390x (64bit) and x86_64 without the fix. >> --- >> elf/rtld.c | 2 +- >> elf/tst-tunables-enable_secure-env.c | 19 +++++++++++++++++++ >> 2 files changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/elf/rtld.c b/elf/rtld.c >> index e9525ea987..883c651d48 100644 >> --- a/elf/rtld.c >> +++ b/elf/rtld.c >> @@ -1325,7 +1325,7 @@ _dl_start_args_adjust (int skip_args, int skip_env) >> >> /* Shuffle auxv down. */ >> ElfW(auxv_t) ax; >> - char *oldp = (char *) (p + 1); >> + char *oldp = (char *) (p + 1 + skip_env); >> char *newp = (char *) (sp + 1); >> do >> { > > I don't think this is correct, running as the testcase it does show the expected value: > LD_SHOW_AUXV=1 can't be used to show this issue. In process_envvars(), process_envvars_secure() is called and e.g. GLIBC_TUNABLES variable is removed with unsetenv(). The skipped/removed variables are counted in skip_env. Note that unsetenv() itself removes the pointer on the stack by moving the later ones to the front. Then in process_envvars_default(), _dl_show_auxv() is called if LD_SHOW_AUXV=1 is set. At this time, the auxv is fine. Lateron, in _dl_start_args_adjust(), the arguments, environment-variables and the auxiliary-vector are shuffled down as "elf/ld-linux-x86-64.so.2" "--library-path" "..." arguments are removed. argv/envp are shuffled down until the NULL-pointers are found on stack: /* Shuffle argv down. */ do *++sp = *++p; while (*p != NULL); ... /* Shuffle envp down. */ do *++sp = *++p; while (*p != NULL); Then auxv is shuffled down. The code assumes that there is one NULL-pointer between envp and auxv. /* Shuffle auxv down. */ ElfW(auxv_t) ax; char *oldp = (char *) (p + 1); char *newp = (char *) (sp + 1); do { memcpy (&ax, oldp, sizeof (ax)); memcpy (newp, &ax, sizeof (ax)); oldp += sizeof (ax); newp += sizeof (ax); } while (ax.a_type != AT_NULL); But GLIBC_TUNABLES was removed with unsetenv() and thus there are skip_env=1 additional pointers between envp and auxv. With skip_env==1, oldp points to the original NULL-pointer between envp and auxv at startup of ld.so. Thus only one auxv-entry is in place - the AT_NULL-one. (Also for skip_env >1, oldp points to a NULL-pointer) If you want, add "_dl_show_auxv ();" after the call of "_dl_start_args_adjust (_dl_argv - orig_argv, skip_env);" But you won't get an output! The called executable (elf/tst-tunables-enable_secure-env) does not get any auxv-entries via getauxval(). Okay, only the special handled ones: AT_HWCAP/AT_HWCAP2. > $ GLIBC_TUNABLES=glibc.rtld.enable_secure=1 \ > elf/ld-linux-x86-64.so.2 --library-path ... elf/tst-tunables-enable_secure-env --direct > AT_EXECFD: 0 > AT_EXECFN: [...]/elf/tst-tunables-enable_secure-env > AT_PHDR: 0x7e7f608b4040 > AT_PHENT: 56 > AT_PHNUM: 13 > AT_PAGESZ: 4096 > AT_BASE: 0x0 > AT_FLAGS: 0x0 > AT_ENTRY: 0x7e7f608b6760 > AT_NOTELF: 0 > AT_UID: 1000 > AT_EUID: 1000 > AT_GID: 1000 > AT_EGID: 1000 > AT_PLATFORM: x86_64 > AT_HWCAP: 2 > AT_CLKTCK: 100 > AT_FPUCW: 0 > AT_DCACHEBSIZE: 0x0 > AT_ICACHEBSIZE: 0x0 > AT_UCACHEBSIZE: 0x0 > AT_IGNOREPPC(null) > AT_SECURE: 0 > AT_BASE_PLATFORM: (null) > AT_SYSINFO: 0x0 > AT_SYSINFO_EHDR: 0x0 > AT_RANDOM: 0x7ffe90102d29 > AT_HWCAP2: 0x2 > AT_HWCAP3: 0x0 > AT_HWCAP4: 0x0 > AT_MINSIGSTKSZ: 3376 > AT_L1I_CACHESIZE: 0 > AT_L1I_CACHEGEOMETRY: 0x0 > AT_L1D_CACHESIZE: 0 > AT_L1D_CACHEGEOMETRY: 0x0 > AT_L2_CACHESIZE: 0 > AT_L2_CACHEGEOMETRY: 0x0 > AT_L3_CACHESIZE: 0 > AT_L3_CACHEGEOMETRY: 0x0 > > However if I add environment that should be filtered: > > > $ GLIBC_TUNABLES=glibc.rtld.enable_secure=1 \ > LD_SHOW_AUXV=1 \ > elf/ld-linux-x86-64.so.2 --library-path ... elf/tst-tunables-enable_secure-env --direct > > AT_EXECFD: 0 > AT_EXECFN: (null) > AT_PHDR: 0x0 > AT_PHENT: 0 > AT_PHNUM: 0 > AT_PAGESZ: 0 > AT_BASE: 0x0 > AT_FLAGS: 0x0 > AT_ENTRY: 0x0 > AT_NOTELF: 0 > AT_UID: 0 > AT_EUID: 0 > AT_GID: 0 > AT_EGID: 0 > AT_PLATFORM: (null) > AT_HWCAP: 2 > AT_CLKTCK: 0 > AT_FPUCW: 0 > AT_DCACHEBSIZE: 0x0 > AT_ICACHEBSIZE: 0x0 > AT_UCACHEBSIZE: 0x0 > AT_IGNOREPPC(null) > AT_SECURE: 0 > AT_BASE_PLATFORM: (null) > AT_SYSINFO: 0x0 > AT_SYSINFO_EHDR: 0x0 > AT_RANDOM: 0x0 > AT_HWCAP2: 0x2 > AT_HWCAP3: 0x0 > AT_HWCAP4: 0x0 > AT_MINSIGSTKSZ: 0 > AT_L1I_CACHESIZE: 0 > AT_L1I_CACHEGEOMETRY: 0x0 > AT_L1D_CACHESIZE: 0 > AT_L1D_CACHEGEOMETRY: 0x0 > AT_L2_CACHESIZE: 0 > AT_L2_CACHEGEOMETRY: 0x0 > AT_L3_CACHESIZE: 0 > AT_L3_CACHEGEOMETRY: 0x0 > >> diff --git a/elf/tst-tunables-enable_secure-env.c b/elf/tst-tunables-enable_secure-env.c >> index 24e846f299..c78e113c21 100644 >> --- a/elf/tst-tunables-enable_secure-env.c >> +++ b/elf/tst-tunables-enable_secure-env.c >> @@ -17,8 +17,12 @@ >> License along with the GNU C Library; if not, see >> <https://www.gnu.org/licenses/>. */ >> >> +#include <stdlib.h> >> #include <support/capture_subprocess.h> >> #include <support/check.h> >> +#ifdef __linux__ >> + #include <sys/auxv.h> >> +#endif >> >> static int >> do_test (int argc, char *argv[]) >> @@ -26,6 +30,21 @@ do_test (int argc, char *argv[]) >> /* Ensure that no assertions are hit when a dynamically linked application >> runs. This test requires that GLIBC_TUNABLES=glibc.rtld.enable_secure=1 >> is set. */ >> + >> + /* The environment variable GLIBC_TUNABLES is skipped for secure >> + execution. */ >> + TEST_VERIFY (getenv ("GLIBC_TUNABLES") == NULL); >> + >> +#ifdef __linux__ >> + /* On linux, the auxiliary vector is located after the environment variables. >> + Check that some of them are available as those are also shuffled down by >> + ld.so. */ >> + TEST_VERIFY (getauxval (AT_PHDR) != 0); >> + TEST_VERIFY (getauxval (AT_PHENT) != 0); >> + TEST_VERIFY (getauxval (AT_PHNUM) != 0); >> + TEST_VERIFY (getauxval (AT_ENTRY) != 0); >> +#endif >> + >> return 0; >> } >>
On 28/06/24 10:01, Stefan Liebler wrote: > On 27.06.24 22:40, Adhemerval Zanella Netto wrote: >> >> >> On 25/06/24 06:17, Stefan Liebler wrote: >>> Starting with commit >>> 59974938fe1f4add843f5325f78e2a7ccd8db853 >>> elf/rtld: Count skipped environment variables for enable_secure >>> >>> The new testcase elf/tst-tunables-enable_secure-env segfaults on s390 (31bit). >>> There _start parses the auxiliary vector for some additional checks. >>> >>> Therefore it skips over the zeros after the environment variables ... >>> 0x7fffac20: 0x7fffbd17 0x7fffbd32 0x7fffbd69 0x00000000 >>> ------------------------------------------------^^^last environment variable >>> >>> ... and then it parses the auxiliary vector and stops at AT_NULL. >>> 0x7fffac30: 0x00000000 0x00000021 0x00000000 0x00000000 >>> --------------------------------^^^AT_SYSINFO_EHDR--------------^^^AT_NULL >>> ----------------^^^newp-----------------------------------------^^^oldp >>> Afterwards it tries to access AT_PHDR which points to somewhere and segfaults. >>> >>> Due to not incorporating the skip_env variable in the computation of oldp >>> when shuffling down the auxv in rtld.c, it just copies one entry with AT_NULL >>> and value 0x00000021 and stops the loop. In reality we have skipped >>> GLIBC_TUNABLES environment variable (=> skip_env=1). Thus we should copy from >>> here: >>> 0x7fffac40: 0x00000021 0x7ffff000 0x00000010 0x007fffff >>> ----------------^^^fixed-oldp >>> >>> This patch fixes the computation of oldp when shuffling down auxiliary vector. >>> It also adds some checks in the testcase. Those checks also fail on >>> s390x (64bit) and x86_64 without the fix. >>> --- >>> elf/rtld.c | 2 +- >>> elf/tst-tunables-enable_secure-env.c | 19 +++++++++++++++++++ >>> 2 files changed, 20 insertions(+), 1 deletion(-) >>> >>> diff --git a/elf/rtld.c b/elf/rtld.c >>> index e9525ea987..883c651d48 100644 >>> --- a/elf/rtld.c >>> +++ b/elf/rtld.c >>> @@ -1325,7 +1325,7 @@ _dl_start_args_adjust (int skip_args, int skip_env) >>> >>> /* Shuffle auxv down. */ >>> ElfW(auxv_t) ax; >>> - char *oldp = (char *) (p + 1); >>> + char *oldp = (char *) (p + 1 + skip_env); >>> char *newp = (char *) (sp + 1); >>> do >>> { >> >> I don't think this is correct, running as the testcase it does show the expected value: >> > LD_SHOW_AUXV=1 can't be used to show this issue. > > In process_envvars(), process_envvars_secure() is called and e.g. > GLIBC_TUNABLES variable is removed with unsetenv(). The skipped/removed > variables are counted in skip_env. Note that unsetenv() itself removes > the pointer on the stack by moving the later ones to the front. > Then in process_envvars_default(), _dl_show_auxv() is called if > LD_SHOW_AUXV=1 is set. At this time, the auxv is fine. > > Lateron, in _dl_start_args_adjust(), the arguments, > environment-variables and the auxiliary-vector are shuffled down as > "elf/ld-linux-x86-64.so.2" "--library-path" "..." arguments are removed. > > argv/envp are shuffled down until the NULL-pointers are found on stack: > /* Shuffle argv down. */ > do > *++sp = *++p; > while (*p != NULL); > ... > /* Shuffle envp down. */ > do > *++sp = *++p; > while (*p != NULL); > > Then auxv is shuffled down. The code assumes that there is one > NULL-pointer between envp and auxv. > /* Shuffle auxv down. */ > ElfW(auxv_t) ax; > char *oldp = (char *) (p + 1); > char *newp = (char *) (sp + 1); > do > { > memcpy (&ax, oldp, sizeof (ax)); > memcpy (newp, &ax, sizeof (ax)); > oldp += sizeof (ax); > newp += sizeof (ax); > } > while (ax.a_type != AT_NULL); > > But GLIBC_TUNABLES was removed with unsetenv() and thus there are > skip_env=1 additional pointers between envp and auxv. > > With skip_env==1, oldp points to the original NULL-pointer between envp > and auxv at startup of ld.so. Thus only one auxv-entry is in place - the > AT_NULL-one. (Also for skip_env >1, oldp points to a NULL-pointer) > > If you want, add "_dl_show_auxv ();" after the call of > "_dl_start_args_adjust (_dl_argv - orig_argv, skip_env);" But you won't > get an output! Sorry if I was not clearn, I was not relying on the _dl_show_auxv but rather I changed tst-tunables-enable_secure-env to loop over a list of AT_ entries with getauxval. I used LD_SHOW_AUXV=1 as an example, but I does fail if you set any filtered variable from sysdeps/generic/unsecvars.h: GLIBC_TUNABLES=glibc.rtld.enable_secure=1 \ LD_BIND_NOT=1 \ elf/ld-linux-x86-64.so.2 --library-path ... elf/tst-tunables-enable_secure-env --direct AT_EXECFD: 0 AT_EXECFN: (null) AT_PHDR: 0x0 AT_PHENT: 0 AT_PHNUM: 0 AT_PAGESZ: 0 AT_BASE: 0x0 AT_FLAGS: 0x0 AT_ENTRY: 0x0 AT_NOTELF: 0 AT_UID: 0 AT_EUID: 0 AT_GID: 0 AT_EGID: 0 AT_PLATFORM: (null) AT_HWCAP: 2 AT_CLKTCK: 0 AT_FPUCW: 0 AT_DCACHEBSIZE: 0x0 AT_ICACHEBSIZE: 0x0 AT_UCACHEBSIZE: 0x0 AT_IGNOREPPC(null) AT_SECURE: 0 AT_BASE_PLATFORM: (null) AT_SYSINFO: 0x0 AT_SYSINFO_EHDR: 0x0 AT_RANDOM: 0x0 AT_HWCAP2: 0x2 AT_HWCAP3: 0x0 AT_HWCAP4: 0x0 AT_MINSIGSTKSZ: 0 AT_L1I_CACHESIZE: 0 AT_L1I_CACHEGEOMETRY: 0x0 AT_L1D_CACHESIZE: 0 AT_L1D_CACHEGEOMETRY: 0x0 AT_L2_CACHESIZE: 0 AT_L2_CACHEGEOMETRY: 0x0 AT_L3_CACHESIZE: 0 AT_L3_CACHEGEOMETRY: 0x0 > > The called executable (elf/tst-tunables-enable_secure-env) does not get > any auxv-entries via getauxval(). Okay, only the special handled ones: > AT_HWCAP/AT_HWCAP2. Well, it does if only set GLIBC_TUNABLES=glibc.rtld.enable_secure=1, but the auxv will only contain AT_HWCAP if you set something from the filtered environments variables. I am not sure if this is really a consistent behavior. > >> $ GLIBC_TUNABLES=glibc.rtld.enable_secure=1 \ >> elf/ld-linux-x86-64.so.2 --library-path ... elf/tst-tunables-enable_secure-env --direct >> AT_EXECFD: 0 >> AT_EXECFN: [...]/elf/tst-tunables-enable_secure-env >> AT_PHDR: 0x7e7f608b4040 >> AT_PHENT: 56 >> AT_PHNUM: 13 >> AT_PAGESZ: 4096 >> AT_BASE: 0x0 >> AT_FLAGS: 0x0 >> AT_ENTRY: 0x7e7f608b6760 >> AT_NOTELF: 0 >> AT_UID: 1000 >> AT_EUID: 1000 >> AT_GID: 1000 >> AT_EGID: 1000 >> AT_PLATFORM: x86_64 >> AT_HWCAP: 2 >> AT_CLKTCK: 100 >> AT_FPUCW: 0 >> AT_DCACHEBSIZE: 0x0 >> AT_ICACHEBSIZE: 0x0 >> AT_UCACHEBSIZE: 0x0 >> AT_IGNOREPPC(null) >> AT_SECURE: 0 >> AT_BASE_PLATFORM: (null) >> AT_SYSINFO: 0x0 >> AT_SYSINFO_EHDR: 0x0 >> AT_RANDOM: 0x7ffe90102d29 >> AT_HWCAP2: 0x2 >> AT_HWCAP3: 0x0 >> AT_HWCAP4: 0x0 >> AT_MINSIGSTKSZ: 3376 >> AT_L1I_CACHESIZE: 0 >> AT_L1I_CACHEGEOMETRY: 0x0 >> AT_L1D_CACHESIZE: 0 >> AT_L1D_CACHEGEOMETRY: 0x0 >> AT_L2_CACHESIZE: 0 >> AT_L2_CACHEGEOMETRY: 0x0 >> AT_L3_CACHESIZE: 0 >> AT_L3_CACHEGEOMETRY: 0x0 >> >> However if I add environment that should be filtered: >> >> >> $ GLIBC_TUNABLES=glibc.rtld.enable_secure=1 \ >> LD_SHOW_AUXV=1 \ >> elf/ld-linux-x86-64.so.2 --library-path ... elf/tst-tunables-enable_secure-env --direct >> >> AT_EXECFD: 0 >> AT_EXECFN: (null) >> AT_PHDR: 0x0 >> AT_PHENT: 0 >> AT_PHNUM: 0 >> AT_PAGESZ: 0 >> AT_BASE: 0x0 >> AT_FLAGS: 0x0 >> AT_ENTRY: 0x0 >> AT_NOTELF: 0 >> AT_UID: 0 >> AT_EUID: 0 >> AT_GID: 0 >> AT_EGID: 0 >> AT_PLATFORM: (null) >> AT_HWCAP: 2 >> AT_CLKTCK: 0 >> AT_FPUCW: 0 >> AT_DCACHEBSIZE: 0x0 >> AT_ICACHEBSIZE: 0x0 >> AT_UCACHEBSIZE: 0x0 >> AT_IGNOREPPC(null) >> AT_SECURE: 0 >> AT_BASE_PLATFORM: (null) >> AT_SYSINFO: 0x0 >> AT_SYSINFO_EHDR: 0x0 >> AT_RANDOM: 0x0 >> AT_HWCAP2: 0x2 >> AT_HWCAP3: 0x0 >> AT_HWCAP4: 0x0 >> AT_MINSIGSTKSZ: 0 >> AT_L1I_CACHESIZE: 0 >> AT_L1I_CACHEGEOMETRY: 0x0 >> AT_L1D_CACHESIZE: 0 >> AT_L1D_CACHEGEOMETRY: 0x0 >> AT_L2_CACHESIZE: 0 >> AT_L2_CACHEGEOMETRY: 0x0 >> AT_L3_CACHESIZE: 0 >> AT_L3_CACHEGEOMETRY: 0x0 >> >>> diff --git a/elf/tst-tunables-enable_secure-env.c b/elf/tst-tunables-enable_secure-env.c >>> index 24e846f299..c78e113c21 100644 >>> --- a/elf/tst-tunables-enable_secure-env.c >>> +++ b/elf/tst-tunables-enable_secure-env.c >>> @@ -17,8 +17,12 @@ >>> License along with the GNU C Library; if not, see >>> <https://www.gnu.org/licenses/>. */ >>> >>> +#include <stdlib.h> >>> #include <support/capture_subprocess.h> >>> #include <support/check.h> >>> +#ifdef __linux__ >>> + #include <sys/auxv.h> >>> +#endif >>> >>> static int >>> do_test (int argc, char *argv[]) >>> @@ -26,6 +30,21 @@ do_test (int argc, char *argv[]) >>> /* Ensure that no assertions are hit when a dynamically linked application >>> runs. This test requires that GLIBC_TUNABLES=glibc.rtld.enable_secure=1 >>> is set. */ >>> + >>> + /* The environment variable GLIBC_TUNABLES is skipped for secure >>> + execution. */ >>> + TEST_VERIFY (getenv ("GLIBC_TUNABLES") == NULL); >>> + >>> +#ifdef __linux__ >>> + /* On linux, the auxiliary vector is located after the environment variables. >>> + Check that some of them are available as those are also shuffled down by >>> + ld.so. */ >>> + TEST_VERIFY (getauxval (AT_PHDR) != 0); >>> + TEST_VERIFY (getauxval (AT_PHENT) != 0); >>> + TEST_VERIFY (getauxval (AT_PHNUM) != 0); >>> + TEST_VERIFY (getauxval (AT_ENTRY) != 0); >>> +#endif >>> + >>> return 0; >>> } >>> >
On 28.06.24 15:22, Adhemerval Zanella Netto wrote: > > > On 28/06/24 10:01, Stefan Liebler wrote: >> On 27.06.24 22:40, Adhemerval Zanella Netto wrote: >>> >>> >>> On 25/06/24 06:17, Stefan Liebler wrote: >>>> Starting with commit >>>> 59974938fe1f4add843f5325f78e2a7ccd8db853 >>>> elf/rtld: Count skipped environment variables for enable_secure >>>> >>>> The new testcase elf/tst-tunables-enable_secure-env segfaults on s390 (31bit). >>>> There _start parses the auxiliary vector for some additional checks. >>>> >>>> Therefore it skips over the zeros after the environment variables ... >>>> 0x7fffac20: 0x7fffbd17 0x7fffbd32 0x7fffbd69 0x00000000 >>>> ------------------------------------------------^^^last environment variable >>>> >>>> ... and then it parses the auxiliary vector and stops at AT_NULL. >>>> 0x7fffac30: 0x00000000 0x00000021 0x00000000 0x00000000 >>>> --------------------------------^^^AT_SYSINFO_EHDR--------------^^^AT_NULL >>>> ----------------^^^newp-----------------------------------------^^^oldp >>>> Afterwards it tries to access AT_PHDR which points to somewhere and segfaults. >>>> >>>> Due to not incorporating the skip_env variable in the computation of oldp >>>> when shuffling down the auxv in rtld.c, it just copies one entry with AT_NULL >>>> and value 0x00000021 and stops the loop. In reality we have skipped >>>> GLIBC_TUNABLES environment variable (=> skip_env=1). Thus we should copy from >>>> here: >>>> 0x7fffac40: 0x00000021 0x7ffff000 0x00000010 0x007fffff >>>> ----------------^^^fixed-oldp >>>> >>>> This patch fixes the computation of oldp when shuffling down auxiliary vector. >>>> It also adds some checks in the testcase. Those checks also fail on >>>> s390x (64bit) and x86_64 without the fix. >>>> --- >>>> elf/rtld.c | 2 +- >>>> elf/tst-tunables-enable_secure-env.c | 19 +++++++++++++++++++ >>>> 2 files changed, 20 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/elf/rtld.c b/elf/rtld.c >>>> index e9525ea987..883c651d48 100644 >>>> --- a/elf/rtld.c >>>> +++ b/elf/rtld.c >>>> @@ -1325,7 +1325,7 @@ _dl_start_args_adjust (int skip_args, int skip_env) >>>> >>>> /* Shuffle auxv down. */ >>>> ElfW(auxv_t) ax; >>>> - char *oldp = (char *) (p + 1); >>>> + char *oldp = (char *) (p + 1 + skip_env); >>>> char *newp = (char *) (sp + 1); >>>> do >>>> { >>> >>> I don't think this is correct, running as the testcase it does show the expected value: >>> >> LD_SHOW_AUXV=1 can't be used to show this issue. >> >> In process_envvars(), process_envvars_secure() is called and e.g. >> GLIBC_TUNABLES variable is removed with unsetenv(). The skipped/removed >> variables are counted in skip_env. Note that unsetenv() itself removes >> the pointer on the stack by moving the later ones to the front. >> Then in process_envvars_default(), _dl_show_auxv() is called if >> LD_SHOW_AUXV=1 is set. At this time, the auxv is fine. >> >> Lateron, in _dl_start_args_adjust(), the arguments, >> environment-variables and the auxiliary-vector are shuffled down as >> "elf/ld-linux-x86-64.so.2" "--library-path" "..." arguments are removed. >> >> argv/envp are shuffled down until the NULL-pointers are found on stack: >> /* Shuffle argv down. */ >> do >> *++sp = *++p; >> while (*p != NULL); >> ... >> /* Shuffle envp down. */ >> do >> *++sp = *++p; >> while (*p != NULL); >> >> Then auxv is shuffled down. The code assumes that there is one >> NULL-pointer between envp and auxv. >> /* Shuffle auxv down. */ >> ElfW(auxv_t) ax; >> char *oldp = (char *) (p + 1); >> char *newp = (char *) (sp + 1); >> do >> { >> memcpy (&ax, oldp, sizeof (ax)); >> memcpy (newp, &ax, sizeof (ax)); >> oldp += sizeof (ax); >> newp += sizeof (ax); >> } >> while (ax.a_type != AT_NULL); >> >> But GLIBC_TUNABLES was removed with unsetenv() and thus there are >> skip_env=1 additional pointers between envp and auxv. >> >> With skip_env==1, oldp points to the original NULL-pointer between envp >> and auxv at startup of ld.so. Thus only one auxv-entry is in place - the >> AT_NULL-one. (Also for skip_env >1, oldp points to a NULL-pointer) >> >> If you want, add "_dl_show_auxv ();" after the call of >> "_dl_start_args_adjust (_dl_argv - orig_argv, skip_env);" But you won't >> get an output! > > Sorry if I was not clearn, I was not relying on the _dl_show_auxv but rather > I changed tst-tunables-enable_secure-env to loop over a list of AT_ entries > with getauxval. I used LD_SHOW_AUXV=1 as an example, but I does fail if you > set any filtered variable from sysdeps/generic/unsecvars.h: > > GLIBC_TUNABLES=glibc.rtld.enable_secure=1 \ > LD_BIND_NOT=1 \ > elf/ld-linux-x86-64.so.2 --library-path ... elf/tst-tunables-enable_secure-env --direct > AT_EXECFD: 0 > AT_EXECFN: (null) > AT_PHDR: 0x0 > AT_PHENT: 0 > AT_PHNUM: 0 > AT_PAGESZ: 0 > AT_BASE: 0x0 > AT_FLAGS: 0x0 > AT_ENTRY: 0x0 > AT_NOTELF: 0 > AT_UID: 0 > AT_EUID: 0 > AT_GID: 0 > AT_EGID: 0 > AT_PLATFORM: (null) > AT_HWCAP: 2 > AT_CLKTCK: 0 > AT_FPUCW: 0 > AT_DCACHEBSIZE: 0x0 > AT_ICACHEBSIZE: 0x0 > AT_UCACHEBSIZE: 0x0 > AT_IGNOREPPC(null) > AT_SECURE: 0 > AT_BASE_PLATFORM: (null) > AT_SYSINFO: 0x0 > AT_SYSINFO_EHDR: 0x0 > AT_RANDOM: 0x0 > AT_HWCAP2: 0x2 > AT_HWCAP3: 0x0 > AT_HWCAP4: 0x0 > AT_MINSIGSTKSZ: 0 > AT_L1I_CACHESIZE: 0 > AT_L1I_CACHEGEOMETRY: 0x0 > AT_L1D_CACHESIZE: 0 > AT_L1D_CACHEGEOMETRY: 0x0 > AT_L2_CACHESIZE: 0 > AT_L2_CACHEGEOMETRY: 0x0 > AT_L3_CACHESIZE: 0 > AT_L3_CACHEGEOMETRY: 0x0 > >> >> The called executable (elf/tst-tunables-enable_secure-env) does not get >> any auxv-entries via getauxval(). Okay, only the special handled ones: >> AT_HWCAP/AT_HWCAP2. > > Well, it does if only set GLIBC_TUNABLES=glibc.rtld.enable_secure=1, but > the auxv will only contain AT_HWCAP if you set something from the filtered > environments variables. I am not sure if this is really a consistent > behavior. > I have no idea why it works for you if only GLIBC_TUNABLES=glibc.rtld.enable_secure=1 is set. GLIBC_TUNABLES itself is also filtered and produces an additional NULL pointer. For me on s390x/s390/x86_64, I don't get any auxv on current master as it points to an auxv-entry with type==AT_NULL (see my patch-summary or my previous reply). With the proposed adjustment of newp, the whole auxv is moved to the updated GLRO(dl_auxv). Can you please recheck in your environment? gdb --args elf/ld-linux-x86-64.so.2 --library-path ... elf/tst-tunables-enable_secure-env --direct (gdb) set environment GLIBC_TUNABLES glibc.rtld.enable_secure=1 (gdb) b _dl_start_args_adjust and check the memory around auxv after line: void **auxv = (void **) GLRO(dl_auxv) - skip_args - skip_env; (gdb) x/52xg (sp-1) sp-1 should be last environment-pointer. sp should be the null-pointer between environment and the shuffled down auxv. auxv/newp should point to the shuffled down auxv. With my proposed fix, oldp points to the first auxv-entry. In my case on s390x/x86_64: type=0x21=33=AT_SYSINFO_EHDR >> >>> $ GLIBC_TUNABLES=glibc.rtld.enable_secure=1 \ >>> elf/ld-linux-x86-64.so.2 --library-path ... elf/tst-tunables-enable_secure-env --direct >>> AT_EXECFD: 0 >>> AT_EXECFN: [...]/elf/tst-tunables-enable_secure-env >>> AT_PHDR: 0x7e7f608b4040 >>> AT_PHENT: 56 >>> AT_PHNUM: 13 >>> AT_PAGESZ: 4096 >>> AT_BASE: 0x0 >>> AT_FLAGS: 0x0 >>> AT_ENTRY: 0x7e7f608b6760 >>> AT_NOTELF: 0 >>> AT_UID: 1000 >>> AT_EUID: 1000 >>> AT_GID: 1000 >>> AT_EGID: 1000 >>> AT_PLATFORM: x86_64 >>> AT_HWCAP: 2 >>> AT_CLKTCK: 100 >>> AT_FPUCW: 0 >>> AT_DCACHEBSIZE: 0x0 >>> AT_ICACHEBSIZE: 0x0 >>> AT_UCACHEBSIZE: 0x0 >>> AT_IGNOREPPC(null) >>> AT_SECURE: 0 >>> AT_BASE_PLATFORM: (null) >>> AT_SYSINFO: 0x0 >>> AT_SYSINFO_EHDR: 0x0 >>> AT_RANDOM: 0x7ffe90102d29 >>> AT_HWCAP2: 0x2 >>> AT_HWCAP3: 0x0 >>> AT_HWCAP4: 0x0 >>> AT_MINSIGSTKSZ: 3376 >>> AT_L1I_CACHESIZE: 0 >>> AT_L1I_CACHEGEOMETRY: 0x0 >>> AT_L1D_CACHESIZE: 0 >>> AT_L1D_CACHEGEOMETRY: 0x0 >>> AT_L2_CACHESIZE: 0 >>> AT_L2_CACHEGEOMETRY: 0x0 >>> AT_L3_CACHESIZE: 0 >>> AT_L3_CACHEGEOMETRY: 0x0 >>> >>> However if I add environment that should be filtered: >>> >>> >>> $ GLIBC_TUNABLES=glibc.rtld.enable_secure=1 \ >>> LD_SHOW_AUXV=1 \ >>> elf/ld-linux-x86-64.so.2 --library-path ... elf/tst-tunables-enable_secure-env --direct >>> >>> AT_EXECFD: 0 >>> AT_EXECFN: (null) >>> AT_PHDR: 0x0 >>> AT_PHENT: 0 >>> AT_PHNUM: 0 >>> AT_PAGESZ: 0 >>> AT_BASE: 0x0 >>> AT_FLAGS: 0x0 >>> AT_ENTRY: 0x0 >>> AT_NOTELF: 0 >>> AT_UID: 0 >>> AT_EUID: 0 >>> AT_GID: 0 >>> AT_EGID: 0 >>> AT_PLATFORM: (null) >>> AT_HWCAP: 2 >>> AT_CLKTCK: 0 >>> AT_FPUCW: 0 >>> AT_DCACHEBSIZE: 0x0 >>> AT_ICACHEBSIZE: 0x0 >>> AT_UCACHEBSIZE: 0x0 >>> AT_IGNOREPPC(null) >>> AT_SECURE: 0 >>> AT_BASE_PLATFORM: (null) >>> AT_SYSINFO: 0x0 >>> AT_SYSINFO_EHDR: 0x0 >>> AT_RANDOM: 0x0 >>> AT_HWCAP2: 0x2 >>> AT_HWCAP3: 0x0 >>> AT_HWCAP4: 0x0 >>> AT_MINSIGSTKSZ: 0 >>> AT_L1I_CACHESIZE: 0 >>> AT_L1I_CACHEGEOMETRY: 0x0 >>> AT_L1D_CACHESIZE: 0 >>> AT_L1D_CACHEGEOMETRY: 0x0 >>> AT_L2_CACHESIZE: 0 >>> AT_L2_CACHEGEOMETRY: 0x0 >>> AT_L3_CACHESIZE: 0 >>> AT_L3_CACHEGEOMETRY: 0x0 >>> >>>> diff --git a/elf/tst-tunables-enable_secure-env.c b/elf/tst-tunables-enable_secure-env.c >>>> index 24e846f299..c78e113c21 100644 >>>> --- a/elf/tst-tunables-enable_secure-env.c >>>> +++ b/elf/tst-tunables-enable_secure-env.c >>>> @@ -17,8 +17,12 @@ >>>> License along with the GNU C Library; if not, see >>>> <https://www.gnu.org/licenses/>. */ >>>> >>>> +#include <stdlib.h> >>>> #include <support/capture_subprocess.h> >>>> #include <support/check.h> >>>> +#ifdef __linux__ >>>> + #include <sys/auxv.h> >>>> +#endif >>>> >>>> static int >>>> do_test (int argc, char *argv[]) >>>> @@ -26,6 +30,21 @@ do_test (int argc, char *argv[]) >>>> /* Ensure that no assertions are hit when a dynamically linked application >>>> runs. This test requires that GLIBC_TUNABLES=glibc.rtld.enable_secure=1 >>>> is set. */ >>>> + >>>> + /* The environment variable GLIBC_TUNABLES is skipped for secure >>>> + execution. */ >>>> + TEST_VERIFY (getenv ("GLIBC_TUNABLES") == NULL); >>>> + >>>> +#ifdef __linux__ >>>> + /* On linux, the auxiliary vector is located after the environment variables. >>>> + Check that some of them are available as those are also shuffled down by >>>> + ld.so. */ >>>> + TEST_VERIFY (getauxval (AT_PHDR) != 0); >>>> + TEST_VERIFY (getauxval (AT_PHENT) != 0); >>>> + TEST_VERIFY (getauxval (AT_PHNUM) != 0); >>>> + TEST_VERIFY (getauxval (AT_ENTRY) != 0); >>>> +#endif >>>> + >>>> return 0; >>>> } >>>> >>
On 01/07/24 10:54, Stefan Liebler wrote: > On 28.06.24 15:22, Adhemerval Zanella Netto wrote: >> >> >> On 28/06/24 10:01, Stefan Liebler wrote: >>> On 27.06.24 22:40, Adhemerval Zanella Netto wrote: >>>> >>>> >>>> On 25/06/24 06:17, Stefan Liebler wrote: >>>>> Starting with commit >>>>> 59974938fe1f4add843f5325f78e2a7ccd8db853 >>>>> elf/rtld: Count skipped environment variables for enable_secure >>>>> >>>>> The new testcase elf/tst-tunables-enable_secure-env segfaults on s390 (31bit). >>>>> There _start parses the auxiliary vector for some additional checks. >>>>> >>>>> Therefore it skips over the zeros after the environment variables ... >>>>> 0x7fffac20: 0x7fffbd17 0x7fffbd32 0x7fffbd69 0x00000000 >>>>> ------------------------------------------------^^^last environment variable >>>>> >>>>> ... and then it parses the auxiliary vector and stops at AT_NULL. >>>>> 0x7fffac30: 0x00000000 0x00000021 0x00000000 0x00000000 >>>>> --------------------------------^^^AT_SYSINFO_EHDR--------------^^^AT_NULL >>>>> ----------------^^^newp-----------------------------------------^^^oldp >>>>> Afterwards it tries to access AT_PHDR which points to somewhere and segfaults. >>>>> >>>>> Due to not incorporating the skip_env variable in the computation of oldp >>>>> when shuffling down the auxv in rtld.c, it just copies one entry with AT_NULL >>>>> and value 0x00000021 and stops the loop. In reality we have skipped >>>>> GLIBC_TUNABLES environment variable (=> skip_env=1). Thus we should copy from >>>>> here: >>>>> 0x7fffac40: 0x00000021 0x7ffff000 0x00000010 0x007fffff >>>>> ----------------^^^fixed-oldp >>>>> >>>>> This patch fixes the computation of oldp when shuffling down auxiliary vector. >>>>> It also adds some checks in the testcase. Those checks also fail on >>>>> s390x (64bit) and x86_64 without the fix. >>>>> --- >>>>> elf/rtld.c | 2 +- >>>>> elf/tst-tunables-enable_secure-env.c | 19 +++++++++++++++++++ >>>>> 2 files changed, 20 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/elf/rtld.c b/elf/rtld.c >>>>> index e9525ea987..883c651d48 100644 >>>>> --- a/elf/rtld.c >>>>> +++ b/elf/rtld.c >>>>> @@ -1325,7 +1325,7 @@ _dl_start_args_adjust (int skip_args, int skip_env) >>>>> >>>>> /* Shuffle auxv down. */ >>>>> ElfW(auxv_t) ax; >>>>> - char *oldp = (char *) (p + 1); >>>>> + char *oldp = (char *) (p + 1 + skip_env); >>>>> char *newp = (char *) (sp + 1); >>>>> do >>>>> { >>>> >>>> I don't think this is correct, running as the testcase it does show the expected value: >>>> >>> LD_SHOW_AUXV=1 can't be used to show this issue. >>> >>> In process_envvars(), process_envvars_secure() is called and e.g. >>> GLIBC_TUNABLES variable is removed with unsetenv(). The skipped/removed >>> variables are counted in skip_env. Note that unsetenv() itself removes >>> the pointer on the stack by moving the later ones to the front. >>> Then in process_envvars_default(), _dl_show_auxv() is called if >>> LD_SHOW_AUXV=1 is set. At this time, the auxv is fine. >>> >>> Lateron, in _dl_start_args_adjust(), the arguments, >>> environment-variables and the auxiliary-vector are shuffled down as >>> "elf/ld-linux-x86-64.so.2" "--library-path" "..." arguments are removed. >>> >>> argv/envp are shuffled down until the NULL-pointers are found on stack: >>> /* Shuffle argv down. */ >>> do >>> *++sp = *++p; >>> while (*p != NULL); >>> ... >>> /* Shuffle envp down. */ >>> do >>> *++sp = *++p; >>> while (*p != NULL); >>> >>> Then auxv is shuffled down. The code assumes that there is one >>> NULL-pointer between envp and auxv. >>> /* Shuffle auxv down. */ >>> ElfW(auxv_t) ax; >>> char *oldp = (char *) (p + 1); >>> char *newp = (char *) (sp + 1); >>> do >>> { >>> memcpy (&ax, oldp, sizeof (ax)); >>> memcpy (newp, &ax, sizeof (ax)); >>> oldp += sizeof (ax); >>> newp += sizeof (ax); >>> } >>> while (ax.a_type != AT_NULL); >>> >>> But GLIBC_TUNABLES was removed with unsetenv() and thus there are >>> skip_env=1 additional pointers between envp and auxv. >>> >>> With skip_env==1, oldp points to the original NULL-pointer between envp >>> and auxv at startup of ld.so. Thus only one auxv-entry is in place - the >>> AT_NULL-one. (Also for skip_env >1, oldp points to a NULL-pointer) >>> >>> If you want, add "_dl_show_auxv ();" after the call of >>> "_dl_start_args_adjust (_dl_argv - orig_argv, skip_env);" But you won't >>> get an output! >> >> Sorry if I was not clearn, I was not relying on the _dl_show_auxv but rather >> I changed tst-tunables-enable_secure-env to loop over a list of AT_ entries >> with getauxval. I used LD_SHOW_AUXV=1 as an example, but I does fail if you >> set any filtered variable from sysdeps/generic/unsecvars.h: >> >> GLIBC_TUNABLES=glibc.rtld.enable_secure=1 \ >> LD_BIND_NOT=1 \ >> elf/ld-linux-x86-64.so.2 --library-path ... elf/tst-tunables-enable_secure-env --direct >> AT_EXECFD: 0 >> AT_EXECFN: (null) >> AT_PHDR: 0x0 >> AT_PHENT: 0 >> AT_PHNUM: 0 >> AT_PAGESZ: 0 >> AT_BASE: 0x0 >> AT_FLAGS: 0x0 >> AT_ENTRY: 0x0 >> AT_NOTELF: 0 >> AT_UID: 0 >> AT_EUID: 0 >> AT_GID: 0 >> AT_EGID: 0 >> AT_PLATFORM: (null) >> AT_HWCAP: 2 >> AT_CLKTCK: 0 >> AT_FPUCW: 0 >> AT_DCACHEBSIZE: 0x0 >> AT_ICACHEBSIZE: 0x0 >> AT_UCACHEBSIZE: 0x0 >> AT_IGNOREPPC(null) >> AT_SECURE: 0 >> AT_BASE_PLATFORM: (null) >> AT_SYSINFO: 0x0 >> AT_SYSINFO_EHDR: 0x0 >> AT_RANDOM: 0x0 >> AT_HWCAP2: 0x2 >> AT_HWCAP3: 0x0 >> AT_HWCAP4: 0x0 >> AT_MINSIGSTKSZ: 0 >> AT_L1I_CACHESIZE: 0 >> AT_L1I_CACHEGEOMETRY: 0x0 >> AT_L1D_CACHESIZE: 0 >> AT_L1D_CACHEGEOMETRY: 0x0 >> AT_L2_CACHESIZE: 0 >> AT_L2_CACHEGEOMETRY: 0x0 >> AT_L3_CACHESIZE: 0 >> AT_L3_CACHEGEOMETRY: 0x0 >> >>> >>> The called executable (elf/tst-tunables-enable_secure-env) does not get >>> any auxv-entries via getauxval(). Okay, only the special handled ones: >>> AT_HWCAP/AT_HWCAP2. >> >> Well, it does if only set GLIBC_TUNABLES=glibc.rtld.enable_secure=1, but >> the auxv will only contain AT_HWCAP if you set something from the filtered >> environments variables. I am not sure if this is really a consistent >> behavior. >> > I have no idea why it works for you if only > GLIBC_TUNABLES=glibc.rtld.enable_secure=1 is set. GLIBC_TUNABLES itself > is also filtered and produces an additional NULL pointer. > > For me on s390x/s390/x86_64, I don't get any auxv on current master as > it points to an auxv-entry with type==AT_NULL (see my patch-summary or > my previous reply). > > With the proposed adjustment of newp, the whole auxv is moved to the > updated GLRO(dl_auxv). > > Can you please recheck in your environment? > gdb --args elf/ld-linux-x86-64.so.2 --library-path ... > elf/tst-tunables-enable_secure-env --direct > (gdb) set environment GLIBC_TUNABLES glibc.rtld.enable_secure=1 > (gdb) b _dl_start_args_adjust > and check the memory around auxv after line: > void **auxv = (void **) GLRO(dl_auxv) - skip_args - skip_env; > > (gdb) x/52xg (sp-1) > sp-1 should be last environment-pointer. > sp should be the null-pointer between environment and the shuffled down > auxv. > auxv/newp should point to the shuffled down auxv. > > With my proposed fix, oldp points to the first auxv-entry. In my case on > s390x/x86_64: type=0x21=33=AT_SYSINFO_EHDR It seems that I did something wrong with my environment setup last time, retesting everything shows no issue. Sorry about the noise. However, I do think we should check for the auxv values over program re-execution, instead of just checking for non values (so we can be sure that it was not changed to something bogus). What do you think to change the test to something to: diff --git a/elf/Makefile b/elf/Makefile index 24ad5221c2..a3475f3fb5 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -1224,7 +1224,6 @@ tests-special += \ $(objpfx)tst-trace3.out \ $(objpfx)tst-trace4.out \ $(objpfx)tst-trace5.out \ - $(objpfx)tst-tunables-enable_secure-env.out \ $(objpfx)tst-unused-dep-cmp.out \ $(objpfx)tst-unused-dep.out \ # tests-special @@ -2252,13 +2251,7 @@ $(objpfx)tst-unused-dep-cmp.out: $(objpfx)tst-unused-dep.out cmp $< /dev/null > $@; \ $(evaluate-test) -$(objpfx)tst-tunables-enable_secure-env.out: $(objpfx)tst-tunables-enable_secure-env - $(test-wrapper-env) \ - GLIBC_TUNABLES=glibc.rtld.enable_secure=1 \ - $(rtld-prefix) \ - $< > $@; \ - $(evaluate-test) - +tst-tunables-enable_secure-env-ARGS = -- $(host-test-program-cmd) $(objpfx)tst-audit11.out: $(objpfx)tst-auditmod11.so $(objpfx)tst-audit11mod1.so tst-audit11-ENV = LD_AUDIT=$(objpfx)tst-auditmod11.so diff --git a/elf/tst-tunables-enable_secure-env.c b/elf/tst-tunables-enable_secure-env.c index c78e113c21..f01debceb0 100644 --- a/elf/tst-tunables-enable_secure-env.c +++ b/elf/tst-tunables-enable_secure-env.c @@ -17,6 +17,10 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ +#include <array_length.h> +#include <errno.h> +#include <getopt.h> +#include <intprops.h> #include <stdlib.h> #include <support/capture_subprocess.h> #include <support/check.h> @@ -24,25 +28,114 @@ #include <sys/auxv.h> #endif -static int -do_test (int argc, char *argv[]) +/* Nonzero if the program gets called via `exec'. */ +#define CMDLINE_OPTIONS \ + { "restart", no_argument, &restart, 1 }, +static int restart; + +/* Hold the four initial argument used to respawn the process, plus the extra + '--direct', '--restart', auxiliary vector values, and final NULL. */ +static char *spargs[9]; + +static void +check_auxv (unsigned long type, char *argv) +{ + char *endptr; + errno = 0; + unsigned long int varg = strtol (argv, &endptr, 10); + TEST_VERIFY_EXIT (errno == 0); + TEST_VERIFY_EXIT (*endptr == '\0'); + unsigned long int v = getauxval (type); + TEST_COMPARE (varg, v); +} + +/* Called on process re-execution. */ +_Noreturn static void +handle_restart (int argc, char *argv[]) { - /* Ensure that no assertions are hit when a dynamically linked application - runs. This test requires that GLIBC_TUNABLES=glibc.rtld.enable_secure=1 - is set. */ + TEST_COMPARE (argc, 4); - /* The environment variable GLIBC_TUNABLES is skipped for secure - execution. */ TEST_VERIFY (getenv ("GLIBC_TUNABLES") == NULL); + TEST_VERIFY (getenv ("LD_BIND_NOW") == NULL); + + check_auxv (AT_PHENT, argv[0]); + check_auxv (AT_PHNUM, argv[1]); + check_auxv (AT_PAGESZ, argv[2]); + check_auxv (AT_HWCAP, argv[3]); + + exit (EXIT_SUCCESS); +} +static int +do_test (int argc, char *argv[]) +{ #ifdef __linux__ - /* On linux, the auxiliary vector is located after the environment variables. - Check that some of them are available as those are also shuffled down by - ld.so. */ - TEST_VERIFY (getauxval (AT_PHDR) != 0); - TEST_VERIFY (getauxval (AT_PHENT) != 0); - TEST_VERIFY (getauxval (AT_PHNUM) != 0); - TEST_VERIFY (getauxval (AT_ENTRY) != 0); + /* We must have either: + + - four parameter if called initially: + + path for ld.so [optional] + + "--library-path" [optional] + + the library path [optional] + + the application name + + - either parameters left if called through re-execution. + + auxiliary vector value 1 + + auxiliary vector value 2 + + auxiliary vector value 3 + + auxiliary vector value 4 + */ + if (restart) + handle_restart (argc - 1, &argv[1]); + + TEST_VERIFY_EXIT (argc == 2 || argc == 5); + + struct + { + unsigned long int type; + char str[INT_BUFSIZE_BOUND (unsigned long)]; + } auxvals[] = + { + /* Check some auxiliary values that should be constant over process + re-execution. */ + { AT_PHENT }, + { AT_PHNUM }, + { AT_PAGESZ }, + { AT_HWCAP }, + }; + for (int i = 0; i < array_length (auxvals); i++) + { + unsigned long int v = getauxval (auxvals[i].type); + snprintf (auxvals[i].str, sizeof auxvals[i].str, "%ld", v); + } + + { + int i; + for (i = 0; i < argc - 1; i++) + spargs[i] = argv[i + 1]; + spargs[i++] = (char *) "--direct"; + spargs[i++] = (char *) "--restart"; + for (int j = 0; j < array_length (auxvals); j++) + spargs[i++] = auxvals[j].str; + spargs[i] = NULL; + } + + { + char *envs[] = + { + /* Add some environment variable that should be filtered out. */ + (char *) "GLIBC_TUNABLES=glibc.rtld.enable_secure=1", + (char* ) "LD_BIND_NOW=0", + NULL, + }; + struct support_capture_subprocess result + = support_capture_subprogram (spargs[0], spargs, envs); + support_capture_subprocess_check (&result, + "tst-tunables-enable_secure-env", + 0, + sc_allow_none); + support_capture_subprocess_free (&result); + } + #endif return 0;
On 01.07.24 21:07, Adhemerval Zanella Netto wrote: > > > On 01/07/24 10:54, Stefan Liebler wrote: >> On 28.06.24 15:22, Adhemerval Zanella Netto wrote: >>> >>> >>> On 28/06/24 10:01, Stefan Liebler wrote: >>>> On 27.06.24 22:40, Adhemerval Zanella Netto wrote: >>>>> >>>>> >>>>> On 25/06/24 06:17, Stefan Liebler wrote: >>>>>> Starting with commit >>>>>> 59974938fe1f4add843f5325f78e2a7ccd8db853 >>>>>> elf/rtld: Count skipped environment variables for enable_secure >>>>>> >>>>>> The new testcase elf/tst-tunables-enable_secure-env segfaults on s390 (31bit). >>>>>> There _start parses the auxiliary vector for some additional checks. >>>>>> >>>>>> Therefore it skips over the zeros after the environment variables ... >>>>>> 0x7fffac20: 0x7fffbd17 0x7fffbd32 0x7fffbd69 0x00000000 >>>>>> ------------------------------------------------^^^last environment variable >>>>>> >>>>>> ... and then it parses the auxiliary vector and stops at AT_NULL. >>>>>> 0x7fffac30: 0x00000000 0x00000021 0x00000000 0x00000000 >>>>>> --------------------------------^^^AT_SYSINFO_EHDR--------------^^^AT_NULL >>>>>> ----------------^^^newp-----------------------------------------^^^oldp >>>>>> Afterwards it tries to access AT_PHDR which points to somewhere and segfaults. >>>>>> >>>>>> Due to not incorporating the skip_env variable in the computation of oldp >>>>>> when shuffling down the auxv in rtld.c, it just copies one entry with AT_NULL >>>>>> and value 0x00000021 and stops the loop. In reality we have skipped >>>>>> GLIBC_TUNABLES environment variable (=> skip_env=1). Thus we should copy from >>>>>> here: >>>>>> 0x7fffac40: 0x00000021 0x7ffff000 0x00000010 0x007fffff >>>>>> ----------------^^^fixed-oldp >>>>>> >>>>>> This patch fixes the computation of oldp when shuffling down auxiliary vector. >>>>>> It also adds some checks in the testcase. Those checks also fail on >>>>>> s390x (64bit) and x86_64 without the fix. >>>>>> --- >>>>>> elf/rtld.c | 2 +- >>>>>> elf/tst-tunables-enable_secure-env.c | 19 +++++++++++++++++++ >>>>>> 2 files changed, 20 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/elf/rtld.c b/elf/rtld.c >>>>>> index e9525ea987..883c651d48 100644 >>>>>> --- a/elf/rtld.c >>>>>> +++ b/elf/rtld.c >>>>>> @@ -1325,7 +1325,7 @@ _dl_start_args_adjust (int skip_args, int skip_env) >>>>>> >>>>>> /* Shuffle auxv down. */ >>>>>> ElfW(auxv_t) ax; >>>>>> - char *oldp = (char *) (p + 1); >>>>>> + char *oldp = (char *) (p + 1 + skip_env); >>>>>> char *newp = (char *) (sp + 1); >>>>>> do >>>>>> { >>>>> >>>>> I don't think this is correct, running as the testcase it does show the expected value: >>>>> >>>> LD_SHOW_AUXV=1 can't be used to show this issue. >>>> >>>> In process_envvars(), process_envvars_secure() is called and e.g. >>>> GLIBC_TUNABLES variable is removed with unsetenv(). The skipped/removed >>>> variables are counted in skip_env. Note that unsetenv() itself removes >>>> the pointer on the stack by moving the later ones to the front. >>>> Then in process_envvars_default(), _dl_show_auxv() is called if >>>> LD_SHOW_AUXV=1 is set. At this time, the auxv is fine. >>>> >>>> Lateron, in _dl_start_args_adjust(), the arguments, >>>> environment-variables and the auxiliary-vector are shuffled down as >>>> "elf/ld-linux-x86-64.so.2" "--library-path" "..." arguments are removed. >>>> >>>> argv/envp are shuffled down until the NULL-pointers are found on stack: >>>> /* Shuffle argv down. */ >>>> do >>>> *++sp = *++p; >>>> while (*p != NULL); >>>> ... >>>> /* Shuffle envp down. */ >>>> do >>>> *++sp = *++p; >>>> while (*p != NULL); >>>> >>>> Then auxv is shuffled down. The code assumes that there is one >>>> NULL-pointer between envp and auxv. >>>> /* Shuffle auxv down. */ >>>> ElfW(auxv_t) ax; >>>> char *oldp = (char *) (p + 1); >>>> char *newp = (char *) (sp + 1); >>>> do >>>> { >>>> memcpy (&ax, oldp, sizeof (ax)); >>>> memcpy (newp, &ax, sizeof (ax)); >>>> oldp += sizeof (ax); >>>> newp += sizeof (ax); >>>> } >>>> while (ax.a_type != AT_NULL); >>>> >>>> But GLIBC_TUNABLES was removed with unsetenv() and thus there are >>>> skip_env=1 additional pointers between envp and auxv. >>>> >>>> With skip_env==1, oldp points to the original NULL-pointer between envp >>>> and auxv at startup of ld.so. Thus only one auxv-entry is in place - the >>>> AT_NULL-one. (Also for skip_env >1, oldp points to a NULL-pointer) >>>> >>>> If you want, add "_dl_show_auxv ();" after the call of >>>> "_dl_start_args_adjust (_dl_argv - orig_argv, skip_env);" But you won't >>>> get an output! >>> >>> Sorry if I was not clearn, I was not relying on the _dl_show_auxv but rather >>> I changed tst-tunables-enable_secure-env to loop over a list of AT_ entries >>> with getauxval. I used LD_SHOW_AUXV=1 as an example, but I does fail if you >>> set any filtered variable from sysdeps/generic/unsecvars.h: >>> >>> GLIBC_TUNABLES=glibc.rtld.enable_secure=1 \ >>> LD_BIND_NOT=1 \ >>> elf/ld-linux-x86-64.so.2 --library-path ... elf/tst-tunables-enable_secure-env --direct >>> AT_EXECFD: 0 >>> AT_EXECFN: (null) >>> AT_PHDR: 0x0 >>> AT_PHENT: 0 >>> AT_PHNUM: 0 >>> AT_PAGESZ: 0 >>> AT_BASE: 0x0 >>> AT_FLAGS: 0x0 >>> AT_ENTRY: 0x0 >>> AT_NOTELF: 0 >>> AT_UID: 0 >>> AT_EUID: 0 >>> AT_GID: 0 >>> AT_EGID: 0 >>> AT_PLATFORM: (null) >>> AT_HWCAP: 2 >>> AT_CLKTCK: 0 >>> AT_FPUCW: 0 >>> AT_DCACHEBSIZE: 0x0 >>> AT_ICACHEBSIZE: 0x0 >>> AT_UCACHEBSIZE: 0x0 >>> AT_IGNOREPPC(null) >>> AT_SECURE: 0 >>> AT_BASE_PLATFORM: (null) >>> AT_SYSINFO: 0x0 >>> AT_SYSINFO_EHDR: 0x0 >>> AT_RANDOM: 0x0 >>> AT_HWCAP2: 0x2 >>> AT_HWCAP3: 0x0 >>> AT_HWCAP4: 0x0 >>> AT_MINSIGSTKSZ: 0 >>> AT_L1I_CACHESIZE: 0 >>> AT_L1I_CACHEGEOMETRY: 0x0 >>> AT_L1D_CACHESIZE: 0 >>> AT_L1D_CACHEGEOMETRY: 0x0 >>> AT_L2_CACHESIZE: 0 >>> AT_L2_CACHEGEOMETRY: 0x0 >>> AT_L3_CACHESIZE: 0 >>> AT_L3_CACHEGEOMETRY: 0x0 >>> >>>> >>>> The called executable (elf/tst-tunables-enable_secure-env) does not get >>>> any auxv-entries via getauxval(). Okay, only the special handled ones: >>>> AT_HWCAP/AT_HWCAP2. >>> >>> Well, it does if only set GLIBC_TUNABLES=glibc.rtld.enable_secure=1, but >>> the auxv will only contain AT_HWCAP if you set something from the filtered >>> environments variables. I am not sure if this is really a consistent >>> behavior. >>> >> I have no idea why it works for you if only >> GLIBC_TUNABLES=glibc.rtld.enable_secure=1 is set. GLIBC_TUNABLES itself >> is also filtered and produces an additional NULL pointer. >> >> For me on s390x/s390/x86_64, I don't get any auxv on current master as >> it points to an auxv-entry with type==AT_NULL (see my patch-summary or >> my previous reply). >> >> With the proposed adjustment of newp, the whole auxv is moved to the >> updated GLRO(dl_auxv). >> >> Can you please recheck in your environment? >> gdb --args elf/ld-linux-x86-64.so.2 --library-path ... >> elf/tst-tunables-enable_secure-env --direct >> (gdb) set environment GLIBC_TUNABLES glibc.rtld.enable_secure=1 >> (gdb) b _dl_start_args_adjust >> and check the memory around auxv after line: >> void **auxv = (void **) GLRO(dl_auxv) - skip_args - skip_env; >> >> (gdb) x/52xg (sp-1) >> sp-1 should be last environment-pointer. >> sp should be the null-pointer between environment and the shuffled down >> auxv. >> auxv/newp should point to the shuffled down auxv. >> >> With my proposed fix, oldp points to the first auxv-entry. In my case on >> s390x/x86_64: type=0x21=33=AT_SYSINFO_EHDR > > It seems that I did something wrong with my environment setup last time, > retesting everything shows no issue. Sorry about the noise. Okay. No problem. > > However, I do think we should check for the auxv values over program > re-execution, instead of just checking for non values (so we can be > sure that it was not changed to something bogus). What do you think to > change the test to something to: Sure, this is the better solution. But I've adjusted your diff a bit: - spargs was too small - check_auxv can also check errno for getauxval - ensure that the test is also restarting on non-linux without auxv. I've send a V2 with Adhemerval as co-author here: "[PATCH v2] elf/rtld: Fix auxiliary vector for enable_secure" https://sourceware.org/pipermail/libc-alpha/2024-July/157934.html And for completeness here is the diff on top of Adhemervals adjustments: diff --git a/elf/tst-tunables-enable_secure-env.c b/elf/tst-tunables-enable_secure-env.c index f01debceb0..01f121efc3 100644 --- a/elf/tst-tunables-enable_secure-env.c +++ b/elf/tst-tunables-enable_secure-env.c @@ -25,7 +25,10 @@ #include <support/capture_subprocess.h> #include <support/check.h> #ifdef __linux__ - #include <sys/auxv.h> +# define HAVE_AUXV 1 +# include <sys/auxv.h> +#else +# define HAVE_AUXV 0 #endif /* Nonzero if the program gets called via `exec'. */ @@ -35,8 +38,9 @@ static int restart; /* Hold the four initial argument used to respawn the process, plus the extra '--direct', '--restart', auxiliary vector values, and final NULL. */ -static char *spargs[9]; +static char *spargs[11]; +#if HAVE_AUXV static void check_auxv (unsigned long type, char *argv) { @@ -45,23 +49,27 @@ check_auxv (unsigned long type, char *argv) unsigned long int varg = strtol (argv, &endptr, 10); TEST_VERIFY_EXIT (errno == 0); TEST_VERIFY_EXIT (*endptr == '\0'); + errno = 0; unsigned long int v = getauxval (type); + TEST_COMPARE (errno, 0); TEST_COMPARE (varg, v); } +#endif /* Called on process re-execution. */ _Noreturn static void handle_restart (int argc, char *argv[]) { - TEST_COMPARE (argc, 4); - TEST_VERIFY (getenv ("GLIBC_TUNABLES") == NULL); TEST_VERIFY (getenv ("LD_BIND_NOW") == NULL); +#if HAVE_AUXV + TEST_VERIFY_EXIT (argc == 4); check_auxv (AT_PHENT, argv[0]); check_auxv (AT_PHNUM, argv[1]); check_auxv (AT_PAGESZ, argv[2]); check_auxv (AT_HWCAP, argv[3]); +#endif exit (EXIT_SUCCESS); } @@ -69,7 +77,6 @@ handle_restart (int argc, char *argv[]) static int do_test (int argc, char *argv[]) { -#ifdef __linux__ /* We must have either: - four parameter if called initially: @@ -89,6 +96,7 @@ do_test (int argc, char *argv[]) TEST_VERIFY_EXIT (argc == 2 || argc == 5); +#if HAVE_AUXV struct { unsigned long int type; @@ -105,8 +113,9 @@ do_test (int argc, char *argv[]) for (int i = 0; i < array_length (auxvals); i++) { unsigned long int v = getauxval (auxvals[i].type); - snprintf (auxvals[i].str, sizeof auxvals[i].str, "%ld", v); + snprintf (auxvals[i].str, sizeof auxvals[i].str, "%lu", v); } +#endif { int i; @@ -114,8 +123,10 @@ do_test (int argc, char *argv[]) spargs[i] = argv[i + 1]; spargs[i++] = (char *) "--direct"; spargs[i++] = (char *) "--restart"; +#if HAVE_AUXV for (int j = 0; j < array_length (auxvals); j++) spargs[i++] = auxvals[j].str; +#endif spargs[i] = NULL; } @@ -136,8 +147,6 @@ do_test (int argc, char *argv[]) support_capture_subprocess_free (&result); } -#endif - return 0; } > > diff --git a/elf/Makefile b/elf/Makefile > index 24ad5221c2..a3475f3fb5 100644 > --- a/elf/Makefile > +++ b/elf/Makefile > @@ -1224,7 +1224,6 @@ tests-special += \ > $(objpfx)tst-trace3.out \ > $(objpfx)tst-trace4.out \ > $(objpfx)tst-trace5.out \ > - $(objpfx)tst-tunables-enable_secure-env.out \ > $(objpfx)tst-unused-dep-cmp.out \ > $(objpfx)tst-unused-dep.out \ > # tests-special > @@ -2252,13 +2251,7 @@ $(objpfx)tst-unused-dep-cmp.out: $(objpfx)tst-unused-dep.out > cmp $< /dev/null > $@; \ > $(evaluate-test) > > -$(objpfx)tst-tunables-enable_secure-env.out: $(objpfx)tst-tunables-enable_secure-env > - $(test-wrapper-env) \ > - GLIBC_TUNABLES=glibc.rtld.enable_secure=1 \ > - $(rtld-prefix) \ > - $< > $@; \ > - $(evaluate-test) > - > +tst-tunables-enable_secure-env-ARGS = -- $(host-test-program-cmd) > > $(objpfx)tst-audit11.out: $(objpfx)tst-auditmod11.so $(objpfx)tst-audit11mod1.so > tst-audit11-ENV = LD_AUDIT=$(objpfx)tst-auditmod11.so > diff --git a/elf/tst-tunables-enable_secure-env.c b/elf/tst-tunables-enable_secure-env.c > index c78e113c21..f01debceb0 100644 > --- a/elf/tst-tunables-enable_secure-env.c > +++ b/elf/tst-tunables-enable_secure-env.c > @@ -17,6 +17,10 @@ > License along with the GNU C Library; if not, see > <https://www.gnu.org/licenses/>. */ > > +#include <array_length.h> > +#include <errno.h> > +#include <getopt.h> > +#include <intprops.h> > #include <stdlib.h> > #include <support/capture_subprocess.h> > #include <support/check.h> > @@ -24,25 +28,114 @@ > #include <sys/auxv.h> > #endif > > -static int > -do_test (int argc, char *argv[]) > +/* Nonzero if the program gets called via `exec'. */ > +#define CMDLINE_OPTIONS \ > + { "restart", no_argument, &restart, 1 }, > +static int restart; > + > +/* Hold the four initial argument used to respawn the process, plus the extra > + '--direct', '--restart', auxiliary vector values, and final NULL. */ > +static char *spargs[9]; > + > +static void > +check_auxv (unsigned long type, char *argv) > +{ > + char *endptr; > + errno = 0; > + unsigned long int varg = strtol (argv, &endptr, 10); > + TEST_VERIFY_EXIT (errno == 0); > + TEST_VERIFY_EXIT (*endptr == '\0'); > + unsigned long int v = getauxval (type); > + TEST_COMPARE (varg, v); > +} > + > +/* Called on process re-execution. */ > +_Noreturn static void > +handle_restart (int argc, char *argv[]) > { > - /* Ensure that no assertions are hit when a dynamically linked application > - runs. This test requires that GLIBC_TUNABLES=glibc.rtld.enable_secure=1 > - is set. */ > + TEST_COMPARE (argc, 4); > > - /* The environment variable GLIBC_TUNABLES is skipped for secure > - execution. */ > TEST_VERIFY (getenv ("GLIBC_TUNABLES") == NULL); > + TEST_VERIFY (getenv ("LD_BIND_NOW") == NULL); > + > + check_auxv (AT_PHENT, argv[0]); > + check_auxv (AT_PHNUM, argv[1]); > + check_auxv (AT_PAGESZ, argv[2]); > + check_auxv (AT_HWCAP, argv[3]); > + > + exit (EXIT_SUCCESS); > +} > > +static int > +do_test (int argc, char *argv[]) > +{ > #ifdef __linux__ > - /* On linux, the auxiliary vector is located after the environment variables. > - Check that some of them are available as those are also shuffled down by > - ld.so. */ > - TEST_VERIFY (getauxval (AT_PHDR) != 0); > - TEST_VERIFY (getauxval (AT_PHENT) != 0); > - TEST_VERIFY (getauxval (AT_PHNUM) != 0); > - TEST_VERIFY (getauxval (AT_ENTRY) != 0); > + /* We must have either: > + > + - four parameter if called initially: > + + path for ld.so [optional] > + + "--library-path" [optional] > + + the library path [optional] > + + the application name > + > + - either parameters left if called through re-execution. > + + auxiliary vector value 1 > + + auxiliary vector value 2 > + + auxiliary vector value 3 > + + auxiliary vector value 4 > + */ > + if (restart) > + handle_restart (argc - 1, &argv[1]); > + > + TEST_VERIFY_EXIT (argc == 2 || argc == 5); > + > + struct > + { > + unsigned long int type; > + char str[INT_BUFSIZE_BOUND (unsigned long)]; > + } auxvals[] = > + { > + /* Check some auxiliary values that should be constant over process > + re-execution. */ > + { AT_PHENT }, > + { AT_PHNUM }, > + { AT_PAGESZ }, > + { AT_HWCAP }, > + }; > + for (int i = 0; i < array_length (auxvals); i++) > + { > + unsigned long int v = getauxval (auxvals[i].type); > + snprintf (auxvals[i].str, sizeof auxvals[i].str, "%ld", v); > + } > + > + { > + int i; > + for (i = 0; i < argc - 1; i++) > + spargs[i] = argv[i + 1]; > + spargs[i++] = (char *) "--direct"; > + spargs[i++] = (char *) "--restart"; > + for (int j = 0; j < array_length (auxvals); j++) > + spargs[i++] = auxvals[j].str; > + spargs[i] = NULL; > + } > + > + { > + char *envs[] = > + { > + /* Add some environment variable that should be filtered out. */ > + (char *) "GLIBC_TUNABLES=glibc.rtld.enable_secure=1", > + (char* ) "LD_BIND_NOW=0", > + NULL, > + }; > + struct support_capture_subprocess result > + = support_capture_subprogram (spargs[0], spargs, envs); > + support_capture_subprocess_check (&result, > + "tst-tunables-enable_secure-env", > + 0, > + sc_allow_none); > + support_capture_subprocess_free (&result); > + } > + > #endif > > return 0; >
diff --git a/elf/rtld.c b/elf/rtld.c index e9525ea987..883c651d48 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -1325,7 +1325,7 @@ _dl_start_args_adjust (int skip_args, int skip_env) /* Shuffle auxv down. */ ElfW(auxv_t) ax; - char *oldp = (char *) (p + 1); + char *oldp = (char *) (p + 1 + skip_env); char *newp = (char *) (sp + 1); do { diff --git a/elf/tst-tunables-enable_secure-env.c b/elf/tst-tunables-enable_secure-env.c index 24e846f299..c78e113c21 100644 --- a/elf/tst-tunables-enable_secure-env.c +++ b/elf/tst-tunables-enable_secure-env.c @@ -17,8 +17,12 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ +#include <stdlib.h> #include <support/capture_subprocess.h> #include <support/check.h> +#ifdef __linux__ + #include <sys/auxv.h> +#endif static int do_test (int argc, char *argv[]) @@ -26,6 +30,21 @@ do_test (int argc, char *argv[]) /* Ensure that no assertions are hit when a dynamically linked application runs. This test requires that GLIBC_TUNABLES=glibc.rtld.enable_secure=1 is set. */ + + /* The environment variable GLIBC_TUNABLES is skipped for secure + execution. */ + TEST_VERIFY (getenv ("GLIBC_TUNABLES") == NULL); + +#ifdef __linux__ + /* On linux, the auxiliary vector is located after the environment variables. + Check that some of them are available as those are also shuffled down by + ld.so. */ + TEST_VERIFY (getauxval (AT_PHDR) != 0); + TEST_VERIFY (getauxval (AT_PHENT) != 0); + TEST_VERIFY (getauxval (AT_PHNUM) != 0); + TEST_VERIFY (getauxval (AT_ENTRY) != 0); +#endif + return 0; }