* [PATCH v3 0/5] fix ifunc with static pie [BZ #27072] @ 2021-01-12 17:21 Szabolcs Nagy 2021-01-12 17:21 ` [PATCH v3 1/5] configure: Require PI_STATIC_AND_HIDDEN for static pie Szabolcs Nagy ` (4 more replies) 0 siblings, 5 replies; 34+ messages in thread From: Szabolcs Nagy @ 2021-01-12 17:21 UTC (permalink / raw) To: libc-alpha v3: - refactor tunables: move internals out of dl-tunables.h - use generated max string length in the tunables list instead of magic values. v2: - check PI_STATIC_AND_HIDDEN for --enable-static-pie - change string buffer sizes in the tunables - fix env_alias == NULL logic in __tunables_init - move __ehdr_start processing after self relocation force pushed into nsz/bug27072 branch (with a gcc8 bug workaround for aarch64) Issues that are not addressed: - tunables try to allocate memory even with non-suid exe. allocation is only needed for rewriting the GLIBC_TUNABLES env var. i think a case can be made that if anything there is TUNABLE_SECLEVEL_SXID_ERASE then this env var would be simply dropped. - tunable list is not optimized for compactness. - all symbols are forced hidden in libc.a, but i think lib*.a should do the same. (other than lib*_nonshared.a) Szabolcs Nagy (5): configure: Require PI_STATIC_AND_HIDDEN for static pie Make libc symbols hidden in static PIE elf: Make the tunable struct definition internal only elf: Avoid RELATIVE relocs in __tunables_init csu: Move static pie self relocation later [BZ #27072] configure | 14 +++++++++++++ configure.ac | 5 +++++ csu/libc-start.c | 44 +++++++++++++++++++++++----------------- elf/dl-tunable-types.h | 42 +++++++++++++++++++++++++++++--------- elf/dl-tunables.c | 2 +- elf/dl-tunables.h | 35 ++++++++------------------------ include/libc-symbols.h | 8 ++++++-- scripts/gen-tunables.awk | 16 +++++++++++++-- 8 files changed, 105 insertions(+), 61 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 1/5] configure: Require PI_STATIC_AND_HIDDEN for static pie 2021-01-12 17:21 [PATCH v3 0/5] fix ifunc with static pie [BZ #27072] Szabolcs Nagy @ 2021-01-12 17:21 ` Szabolcs Nagy 2021-01-12 18:38 ` Adhemerval Zanella 2021-01-12 17:22 ` [PATCH v3 2/5] Make libc symbols hidden in static PIE Szabolcs Nagy ` (3 subsequent siblings) 4 siblings, 1 reply; 34+ messages in thread From: Szabolcs Nagy @ 2021-01-12 17:21 UTC (permalink / raw) To: libc-alpha The glibc static pie self relocation code relies on that local symbols can be accessed without dynamic relocations in position independent code. --- configure | 14 ++++++++++++++ configure.ac | 5 +++++ 2 files changed, 19 insertions(+) diff --git a/configure b/configure index 49f7b32b52..81fd116d87 100755 --- a/configure +++ b/configure @@ -6818,6 +6818,20 @@ if test "$static_pie" = yes; then if test "$libc_cv_no_dynamic_linker" != yes; then as_fn_error $? "linker support for --no-dynamic-linker needed" "$LINENO" 5 fi + + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ +#ifndef PI_STATIC_AND_HIDDEN +# error static pie depends on PI_STATIC_AND_HIDDEN +#endif +_ACEOF +if ac_fn_c_try_compile "$LINENO"; then : + +else + as_fn_error $? "the target does not support static pie" "$LINENO" 5 +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext + # Default to PIE. libc_cv_pie_default=yes $as_echo "#define ENABLE_STATIC_PIE 1" >>confdefs.h diff --git a/configure.ac b/configure.ac index 341d4eeac2..8b7c99001d 100644 --- a/configure.ac +++ b/configure.ac @@ -1835,6 +1835,11 @@ if test "$static_pie" = yes; then if test "$libc_cv_no_dynamic_linker" != yes; then AC_MSG_ERROR([linker support for --no-dynamic-linker needed]) fi + + AC_COMPILE_IFELSE([AC_LANG_SOURCE([[#ifndef PI_STATIC_AND_HIDDEN +# error static pie depends on PI_STATIC_AND_HIDDEN +#endif]])], , AC_MSG_ERROR([the target does not support static pie])) + # Default to PIE. libc_cv_pie_default=yes AC_DEFINE(ENABLE_STATIC_PIE) -- 2.17.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/5] configure: Require PI_STATIC_AND_HIDDEN for static pie 2021-01-12 17:21 ` [PATCH v3 1/5] configure: Require PI_STATIC_AND_HIDDEN for static pie Szabolcs Nagy @ 2021-01-12 18:38 ` Adhemerval Zanella 0 siblings, 0 replies; 34+ messages in thread From: Adhemerval Zanella @ 2021-01-12 18:38 UTC (permalink / raw) To: libc-alpha, Szabolcs Nagy On 12/01/2021 14:21, Szabolcs Nagy via Libc-alpha wrote: > The glibc static pie self relocation code relies on that local > symbols can be accessed without dynamic relocations in position > independent code. LGTM with the nit below. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > configure | 14 ++++++++++++++ > configure.ac | 5 +++++ > 2 files changed, 19 insertions(+) > > diff --git a/configure b/configure > index 49f7b32b52..81fd116d87 100755 > --- a/configure > +++ b/configure > @@ -6818,6 +6818,20 @@ if test "$static_pie" = yes; then > if test "$libc_cv_no_dynamic_linker" != yes; then > as_fn_error $? "linker support for --no-dynamic-linker needed" "$LINENO" 5 > fi > + > + cat confdefs.h - <<_ACEOF >conftest.$ac_ext > +/* end confdefs.h. */ > +#ifndef PI_STATIC_AND_HIDDEN > +# error static pie depends on PI_STATIC_AND_HIDDEN > +#endif > +_ACEOF > +if ac_fn_c_try_compile "$LINENO"; then : > + > +else > + as_fn_error $? "the target does not support static pie" "$LINENO" 5 > +fi > +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext > + > # Default to PIE. > libc_cv_pie_default=yes > $as_echo "#define ENABLE_STATIC_PIE 1" >>confdefs.h > diff --git a/configure.ac b/configure.ac > index 341d4eeac2..8b7c99001d 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -1835,6 +1835,11 @@ if test "$static_pie" = yes; then > if test "$libc_cv_no_dynamic_linker" != yes; then > AC_MSG_ERROR([linker support for --no-dynamic-linker needed]) > fi > + > + AC_COMPILE_IFELSE([AC_LANG_SOURCE([[#ifndef PI_STATIC_AND_HIDDEN > +# error static pie depends on PI_STATIC_AND_HIDDEN > +#endif]])], , AC_MSG_ERROR([the target does not support static pie])) > + I think we need a more explicit error message stating that the architecture does not support static pie, this might give the user the impression either a compiler options would fix it or, worse, if it defines PI_STATIC_AND_HIDDEN on architecture configure.ac it would fix it. Maybe "the architecture does not support static pie". > # Default to PIE. > libc_cv_pie_default=yes > AC_DEFINE(ENABLE_STATIC_PIE) > ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-12 17:21 [PATCH v3 0/5] fix ifunc with static pie [BZ #27072] Szabolcs Nagy 2021-01-12 17:21 ` [PATCH v3 1/5] configure: Require PI_STATIC_AND_HIDDEN for static pie Szabolcs Nagy @ 2021-01-12 17:22 ` Szabolcs Nagy 2021-01-12 23:09 ` H.J. Lu 2021-01-12 17:22 ` [PATCH v3 3/5] elf: Make the tunable struct definition internal only Szabolcs Nagy ` (2 subsequent siblings) 4 siblings, 1 reply; 34+ messages in thread From: Szabolcs Nagy @ 2021-01-12 17:22 UTC (permalink / raw) To: libc-alpha Hidden matters with static PIE: extern symbol access in position independent code usually involves GOT indirections which needs RELATIVE relocs in a static linked PIE. Using So hidden avoids indirections and RELATIVE relocs on targets that can access symbols pc-relative. The check should use IS_IN_LIB instead of IS_IN(libc) since all static libraries can use hidden visibility to avoid indirections, however the test system links objects from libcrypt.a into dynamic linked test binaries so hidden does not work there. I think mixing static and shared libc components in the same binary should not be supported usage, but to be safe only use hidden in libc.a. From -static-pie linked 'int main(){}' this shaves off 73 relative relocs on aarch64 and reduces code size too. --- include/libc-symbols.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/include/libc-symbols.h b/include/libc-symbols.h index ea126ae70c..93e63ee889 100644 --- a/include/libc-symbols.h +++ b/include/libc-symbols.h @@ -434,13 +434,17 @@ for linking") strong_alias(real, name) #endif -#if defined SHARED || defined LIBC_NONSHARED \ - || (BUILD_PIE_DEFAULT && IS_IN (libc)) +#if defined SHARED || defined LIBC_NONSHARED # define attribute_hidden __attribute__ ((visibility ("hidden"))) #else # define attribute_hidden #endif +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections. */ +#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED +# pragma GCC visibility push(hidden) +#endif + #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec"))) #define attribute_relro __attribute__ ((section (".data.rel.ro"))) -- 2.17.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-12 17:22 ` [PATCH v3 2/5] Make libc symbols hidden in static PIE Szabolcs Nagy @ 2021-01-12 23:09 ` H.J. Lu 2021-01-13 0:02 ` H.J. Lu 0 siblings, 1 reply; 34+ messages in thread From: H.J. Lu @ 2021-01-12 23:09 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Tue, Jan 12, 2021 at 9:26 AM Szabolcs Nagy via Libc-alpha <libc-alpha@sourceware.org> wrote: > > Hidden matters with static PIE: extern symbol access in position > independent code usually involves GOT indirections which needs > RELATIVE relocs in a static linked PIE. Using So hidden avoids > indirections and RELATIVE relocs on targets that can access symbols > pc-relative. > > The check should use IS_IN_LIB instead of IS_IN(libc) since all > static libraries can use hidden visibility to avoid indirections, > however the test system links objects from libcrypt.a into dynamic > linked test binaries so hidden does not work there. I think mixing > static and shared libc components in the same binary should not be > supported usage, but to be safe only use hidden in libc.a. > > From -static-pie linked 'int main(){}' this shaves off 73 relative > relocs on aarch64 and reduces code size too. > --- > include/libc-symbols.h | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/include/libc-symbols.h b/include/libc-symbols.h > index ea126ae70c..93e63ee889 100644 > --- a/include/libc-symbols.h > +++ b/include/libc-symbols.h > @@ -434,13 +434,17 @@ for linking") > strong_alias(real, name) > #endif > > -#if defined SHARED || defined LIBC_NONSHARED \ > - || (BUILD_PIE_DEFAULT && IS_IN (libc)) > +#if defined SHARED || defined LIBC_NONSHARED > # define attribute_hidden __attribute__ ((visibility ("hidden"))) > #else > # define attribute_hidden > #endif > > +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections. */ > +#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED > +# pragma GCC visibility push(hidden) > +#endif > + > #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec"))) > > #define attribute_relro __attribute__ ((section (".data.rel.ro"))) > -- > 2.17.1 > Unfortunately it doesn't work on i686 with --enable-static-pie: /usr/local/bin/ld: /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o): unsupported non-PIC call to IFUNC `strcmp' Please make it opt-out. -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-12 23:09 ` H.J. Lu @ 2021-01-13 0:02 ` H.J. Lu 2021-01-13 0:33 ` H.J. Lu 0 siblings, 1 reply; 34+ messages in thread From: H.J. Lu @ 2021-01-13 0:02 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Tue, Jan 12, 2021 at 3:09 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Jan 12, 2021 at 9:26 AM Szabolcs Nagy via Libc-alpha > <libc-alpha@sourceware.org> wrote: > > > > Hidden matters with static PIE: extern symbol access in position > > independent code usually involves GOT indirections which needs > > RELATIVE relocs in a static linked PIE. Using So hidden avoids > > indirections and RELATIVE relocs on targets that can access symbols > > pc-relative. > > > > The check should use IS_IN_LIB instead of IS_IN(libc) since all > > static libraries can use hidden visibility to avoid indirections, > > however the test system links objects from libcrypt.a into dynamic > > linked test binaries so hidden does not work there. I think mixing > > static and shared libc components in the same binary should not be > > supported usage, but to be safe only use hidden in libc.a. > > > > From -static-pie linked 'int main(){}' this shaves off 73 relative > > relocs on aarch64 and reduces code size too. > > --- > > include/libc-symbols.h | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/include/libc-symbols.h b/include/libc-symbols.h > > index ea126ae70c..93e63ee889 100644 > > --- a/include/libc-symbols.h > > +++ b/include/libc-symbols.h > > @@ -434,13 +434,17 @@ for linking") > > strong_alias(real, name) > > #endif > > > > -#if defined SHARED || defined LIBC_NONSHARED \ > > - || (BUILD_PIE_DEFAULT && IS_IN (libc)) > > +#if defined SHARED || defined LIBC_NONSHARED > > # define attribute_hidden __attribute__ ((visibility ("hidden"))) > > #else > > # define attribute_hidden > > #endif > > > > +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections. */ > > +#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED > > +# pragma GCC visibility push(hidden) > > +#endif > > + > > #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec"))) > > > > #define attribute_relro __attribute__ ((section (".data.rel.ro"))) > > -- > > 2.17.1 > > > > Unfortunately it doesn't work on i686 with --enable-static-pie: > > /usr/local/bin/ld: > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o): > unsupported non-PIC call to IFUNC `strcmp' > > Please make it opt-out. > See: https://sourceware.org/bugzilla/show_bug.cgi?id=14961 -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-13 0:02 ` H.J. Lu @ 2021-01-13 0:33 ` H.J. Lu 2021-01-13 1:19 ` H.J. Lu 0 siblings, 1 reply; 34+ messages in thread From: H.J. Lu @ 2021-01-13 0:33 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Jan 12, 2021 at 3:09 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Tue, Jan 12, 2021 at 9:26 AM Szabolcs Nagy via Libc-alpha > > <libc-alpha@sourceware.org> wrote: > > > > > > Hidden matters with static PIE: extern symbol access in position > > > independent code usually involves GOT indirections which needs > > > RELATIVE relocs in a static linked PIE. Using So hidden avoids > > > indirections and RELATIVE relocs on targets that can access symbols > > > pc-relative. > > > > > > The check should use IS_IN_LIB instead of IS_IN(libc) since all > > > static libraries can use hidden visibility to avoid indirections, > > > however the test system links objects from libcrypt.a into dynamic > > > linked test binaries so hidden does not work there. I think mixing > > > static and shared libc components in the same binary should not be > > > supported usage, but to be safe only use hidden in libc.a. > > > > > > From -static-pie linked 'int main(){}' this shaves off 73 relative > > > relocs on aarch64 and reduces code size too. > > > --- > > > include/libc-symbols.h | 8 ++++++-- > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > diff --git a/include/libc-symbols.h b/include/libc-symbols.h > > > index ea126ae70c..93e63ee889 100644 > > > --- a/include/libc-symbols.h > > > +++ b/include/libc-symbols.h > > > @@ -434,13 +434,17 @@ for linking") > > > strong_alias(real, name) > > > #endif > > > > > > -#if defined SHARED || defined LIBC_NONSHARED \ > > > - || (BUILD_PIE_DEFAULT && IS_IN (libc)) > > > +#if defined SHARED || defined LIBC_NONSHARED > > > # define attribute_hidden __attribute__ ((visibility ("hidden"))) > > > #else > > > # define attribute_hidden > > > #endif > > > > > > +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections. */ > > > +#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED > > > +# pragma GCC visibility push(hidden) > > > +#endif > > > + > > > #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec"))) > > > > > > #define attribute_relro __attribute__ ((section (".data.rel.ro"))) > > > -- > > > 2.17.1 > > > > > > > Unfortunately it doesn't work on i686 with --enable-static-pie: > > > > /usr/local/bin/ld: > > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o): > > unsupported non-PIC call to IFUNC `strcmp' > > > > Please make it opt-out. > > > > See: > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961 > > -- > H.J. Testing: diff --git a/include/libc-symbols.h b/include/libc-symbols.h index 93e63ee889..f4dd735555 100644 --- a/include/libc-symbols.h +++ b/include/libc-symbols.h @@ -441,7 +441,8 @@ for linking") #endif /* Mark all symbols hidden in static PIE libc to avoid GOT indirections. */ -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \ + && IS_IN (libc) && !defined LIBC_NONSHARED # pragma GCC visibility push(hidden) #endif -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-13 0:33 ` H.J. Lu @ 2021-01-13 1:19 ` H.J. Lu 2021-01-13 9:50 ` Szabolcs Nagy 0 siblings, 1 reply; 34+ messages in thread From: H.J. Lu @ 2021-01-13 1:19 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Tue, Jan 12, 2021 at 3:09 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > On Tue, Jan 12, 2021 at 9:26 AM Szabolcs Nagy via Libc-alpha > > > <libc-alpha@sourceware.org> wrote: > > > > > > > > Hidden matters with static PIE: extern symbol access in position > > > > independent code usually involves GOT indirections which needs > > > > RELATIVE relocs in a static linked PIE. Using So hidden avoids > > > > indirections and RELATIVE relocs on targets that can access symbols > > > > pc-relative. > > > > > > > > The check should use IS_IN_LIB instead of IS_IN(libc) since all > > > > static libraries can use hidden visibility to avoid indirections, > > > > however the test system links objects from libcrypt.a into dynamic > > > > linked test binaries so hidden does not work there. I think mixing > > > > static and shared libc components in the same binary should not be > > > > supported usage, but to be safe only use hidden in libc.a. > > > > > > > > From -static-pie linked 'int main(){}' this shaves off 73 relative > > > > relocs on aarch64 and reduces code size too. > > > > --- > > > > include/libc-symbols.h | 8 ++++++-- > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/include/libc-symbols.h b/include/libc-symbols.h > > > > index ea126ae70c..93e63ee889 100644 > > > > --- a/include/libc-symbols.h > > > > +++ b/include/libc-symbols.h > > > > @@ -434,13 +434,17 @@ for linking") > > > > strong_alias(real, name) > > > > #endif > > > > > > > > -#if defined SHARED || defined LIBC_NONSHARED \ > > > > - || (BUILD_PIE_DEFAULT && IS_IN (libc)) > > > > +#if defined SHARED || defined LIBC_NONSHARED > > > > # define attribute_hidden __attribute__ ((visibility ("hidden"))) > > > > #else > > > > # define attribute_hidden > > > > #endif > > > > > > > > +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections. */ > > > > +#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED > > > > +# pragma GCC visibility push(hidden) > > > > +#endif > > > > + > > > > #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec"))) > > > > > > > > #define attribute_relro __attribute__ ((section (".data.rel.ro"))) > > > > -- > > > > 2.17.1 > > > > > > > > > > Unfortunately it doesn't work on i686 with --enable-static-pie: > > > > > > /usr/local/bin/ld: > > > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o): > > > unsupported non-PIC call to IFUNC `strcmp' > > > > > > Please make it opt-out. > > > > > > > See: > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961 > > > > -- > > H.J. > > Testing: > > diff --git a/include/libc-symbols.h b/include/libc-symbols.h > index 93e63ee889..f4dd735555 100644 > --- a/include/libc-symbols.h > +++ b/include/libc-symbols.h > @@ -441,7 +441,8 @@ for linking") > #endif > > /* Mark all symbols hidden in static PIE libc to avoid GOT indirections. */ > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \ > + && IS_IN (libc) && !defined LIBC_NONSHARED > # pragma GCC visibility push(hidden) > #endif > This works on i686. -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-13 1:19 ` H.J. Lu @ 2021-01-13 9:50 ` Szabolcs Nagy 2021-01-14 11:17 ` Szabolcs Nagy 0 siblings, 1 reply; 34+ messages in thread From: Szabolcs Nagy @ 2021-01-13 9:50 UTC (permalink / raw) To: H.J. Lu; +Cc: GNU C Library The 01/12/2021 17:19, H.J. Lu wrote: > On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > On Tue, Jan 12, 2021 at 3:09 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > Unfortunately it doesn't work on i686 with --enable-static-pie: > > > > > > > > /usr/local/bin/ld: > > > > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o): > > > > unsupported non-PIC call to IFUNC `strcmp' > > > > > > > > Please make it opt-out. > > > > > > > > > > See: > > > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961 > > > > > /* Mark all symbols hidden in static PIE libc to avoid GOT indirections. */ > > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED > > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \ > > + && IS_IN (libc) && !defined LIBC_NONSHARED > > # pragma GCC visibility push(hidden) > > #endif > > > > This works on i686. if it works then likely pie uses copy relocs on i686 so object symbols are always accessed locally instead of via GOT. this sounds a bit fragile. ideally there would be a way to mark object symbols hidden but not functions for such targets. i will add this change for now, thanks. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-13 9:50 ` Szabolcs Nagy @ 2021-01-14 11:17 ` Szabolcs Nagy 2021-01-14 15:39 ` H.J. Lu 2021-01-15 3:36 ` H.J. Lu 0 siblings, 2 replies; 34+ messages in thread From: Szabolcs Nagy @ 2021-01-14 11:17 UTC (permalink / raw) To: H.J. Lu, GNU C Library The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote: > The 01/12/2021 17:19, H.J. Lu wrote: > > On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > See: > > > > > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961 > > > > > > > /* Mark all symbols hidden in static PIE libc to avoid GOT indirections. */ > > > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED > > > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \ > > > + && IS_IN (libc) && !defined LIBC_NONSHARED > > > # pragma GCC visibility push(hidden) > > > #endif > > > > > > > This works on i686. The series i plan to commit today is in nsz/bug27072 now, This is the v4 of this patch: Hidden matters with static PIE: extern symbol access in position independent code usually involves GOT indirections which needs RELATIVE relocs in a static linked PIE. Hidden visibility avoids indirections and RELATIVE relocs on targets that can access symbols pc-relative. The check should use IS_IN_LIB instead of IS_IN(libc) since all static libraries can use hidden visibility to avoid indirections, however the test system links objects from libcrypt.a into dynamic linked test binaries so hidden does not work there. I think mixing static and shared libc components in the same binary should not be supported usage, but to be safe only use hidden in libc.a. There are targets (i686) where hidden visibility functions are problematic in PIE code so hidden cannot be applied to all symbols. Then static PIE requires extern object access without relocations (e.g. by relying on copy relocations in shared libraries instead of GOT access in PIE code). See bug 14961. From -static-pie linked 'int main(){}' this shaves off 73 relative relocs on aarch64 and reduces code size too. --- include/libc-symbols.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/include/libc-symbols.h b/include/libc-symbols.h index ea126ae70c..f4dd735555 100644 --- a/include/libc-symbols.h +++ b/include/libc-symbols.h @@ -434,13 +434,18 @@ for linking") strong_alias(real, name) #endif -#if defined SHARED || defined LIBC_NONSHARED \ - || (BUILD_PIE_DEFAULT && IS_IN (libc)) +#if defined SHARED || defined LIBC_NONSHARED # define attribute_hidden __attribute__ ((visibility ("hidden"))) #else # define attribute_hidden #endif +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections. */ +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \ + && IS_IN (libc) && !defined LIBC_NONSHARED +# pragma GCC visibility push(hidden) +#endif + #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec"))) #define attribute_relro __attribute__ ((section (".data.rel.ro"))) -- 2.17.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-14 11:17 ` Szabolcs Nagy @ 2021-01-14 15:39 ` H.J. Lu 2021-01-15 3:36 ` H.J. Lu 1 sibling, 0 replies; 34+ messages in thread From: H.J. Lu @ 2021-01-14 15:39 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote: > > The 01/12/2021 17:19, H.J. Lu wrote: > > > On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > See: > > > > > > > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961 > > > > > > > > > /* Mark all symbols hidden in static PIE libc to avoid GOT indirections. */ > > > > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED > > > > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \ > > > > + && IS_IN (libc) && !defined LIBC_NONSHARED > > > > # pragma GCC visibility push(hidden) > > > > #endif > > > > > > > > > > This works on i686. > > The series i plan to commit today is in nsz/bug27072 now, > > This is the v4 of this patch: > > Hidden matters with static PIE: extern symbol access in position > independent code usually involves GOT indirections which needs > RELATIVE relocs in a static linked PIE. Hidden visibility avoids > indirections and RELATIVE relocs on targets that can access symbols > pc-relative. > > The check should use IS_IN_LIB instead of IS_IN(libc) since all > static libraries can use hidden visibility to avoid indirections, > however the test system links objects from libcrypt.a into dynamic > linked test binaries so hidden does not work there. I think mixing > static and shared libc components in the same binary should not be > supported usage, but to be safe only use hidden in libc.a. > > There are targets (i686) where hidden visibility functions are > problematic in PIE code so hidden cannot be applied to all symbols. > Then static PIE requires extern object access without relocations > (e.g. by relying on copy relocations in shared libraries instead of > GOT access in PIE code). See bug 14961. > > From -static-pie linked 'int main(){}' this shaves off 73 relative > relocs on aarch64 and reduces code size too. > --- > include/libc-symbols.h | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/include/libc-symbols.h b/include/libc-symbols.h > index ea126ae70c..f4dd735555 100644 > --- a/include/libc-symbols.h > +++ b/include/libc-symbols.h > @@ -434,13 +434,18 @@ for linking") > strong_alias(real, name) > #endif > > -#if defined SHARED || defined LIBC_NONSHARED \ > - || (BUILD_PIE_DEFAULT && IS_IN (libc)) > +#if defined SHARED || defined LIBC_NONSHARED > # define attribute_hidden __attribute__ ((visibility ("hidden"))) > #else > # define attribute_hidden > #endif > > +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections. */ > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \ > + && IS_IN (libc) && !defined LIBC_NONSHARED > +# pragma GCC visibility push(hidden) > +#endif > + > #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec"))) > > #define attribute_relro __attribute__ ((section (".data.rel.ro"))) > -- > 2.17.1 > This generates bad static PIE on i386. This patch is needed: diff --git a/include/libc-symbols.h b/include/libc-symbols.h index f4dd735555..72276a5c48 100644 --- a/include/libc-symbols.h +++ b/include/libc-symbols.h @@ -434,7 +434,9 @@ for linking") strong_alias(real, name) #endif -#if defined SHARED || defined LIBC_NONSHARED +#if defined SHARED || defined LIBC_NONSHARED \ + || !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \ + || (BUILD_PIE_DEFAULT && IS_IN (libc)) # define attribute_hidden __attribute__ ((visibility ("hidden"))) #else # define attribute_hidden -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-14 11:17 ` Szabolcs Nagy 2021-01-14 15:39 ` H.J. Lu @ 2021-01-15 3:36 ` H.J. Lu 2021-01-15 4:29 ` H.J. Lu 2021-01-15 11:25 ` Szabolcs Nagy 1 sibling, 2 replies; 34+ messages in thread From: H.J. Lu @ 2021-01-15 3:36 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library [-- Attachment #1: Type: text/plain, Size: 2260 bytes --] On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote: > > The 01/12/2021 17:19, H.J. Lu wrote: > > > On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > See: > > > > > > > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961 > > > > > > > > > /* Mark all symbols hidden in static PIE libc to avoid GOT indirections. */ > > > > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED > > > > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \ > > > > + && IS_IN (libc) && !defined LIBC_NONSHARED > > > > # pragma GCC visibility push(hidden) > > > > #endif > > > > > > > > > > This works on i686. > > The series i plan to commit today is in nsz/bug27072 now, > > This is the v4 of this patch: > > Hidden matters with static PIE: extern symbol access in position > independent code usually involves GOT indirections which needs > RELATIVE relocs in a static linked PIE. Hidden visibility avoids > indirections and RELATIVE relocs on targets that can access symbols > pc-relative. > > The check should use IS_IN_LIB instead of IS_IN(libc) since all > static libraries can use hidden visibility to avoid indirections, > however the test system links objects from libcrypt.a into dynamic > linked test binaries so hidden does not work there. I think mixing > static and shared libc components in the same binary should not be > supported usage, but to be safe only use hidden in libc.a. > > There are targets (i686) where hidden visibility functions are > problematic in PIE code so hidden cannot be applied to all symbols. > Then static PIE requires extern object access without relocations > (e.g. by relying on copy relocations in shared libraries instead of > GOT access in PIE code). See bug 14961. It isn't about copy relocations. It is IFUNC, PLT and PIE. I needed additional patches to make static PIE to work on i386 and x86-64. I am enclosing my patches. Please include them in your patch set. > From -static-pie linked 'int main(){}' this shaves off 73 relative > relocs on aarch64 and reduces code size too. -- H.J. [-- Attachment #2: 0001-libmvec-Add-extra-test-objs-to-test-extras.patch --] [-- Type: text/x-patch, Size: 1695 bytes --] From 15488890220a8c580689e6f78a38847853b78850 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Thu, 14 Jan 2021 18:39:24 -0800 Subject: [PATCH 1/4] libmvec: Add extra-test-objs to test-extras Add extra-test-objs to test-extras so that they are compiled with -DMODULE_NAME=testsuite instead of -DMODULE_NAME=libc. --- sysdeps/x86_64/fpu/Makefile | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sysdeps/x86_64/fpu/Makefile b/sysdeps/x86_64/fpu/Makefile index a4ff2723a8..9a4bdd075c 100644 --- a/sysdeps/x86_64/fpu/Makefile +++ b/sysdeps/x86_64/fpu/Makefile @@ -31,6 +31,12 @@ libmvec-tests += double-vlen2 double-vlen4 double-vlen4-avx2 \ tests += test-double-libmvec-sincos test-double-libmvec-sincos-avx \ test-double-libmvec-sincos-avx2 test-float-libmvec-sincosf \ test-float-libmvec-sincosf-avx test-float-libmvec-sincosf-avx2 +test-extras += test-double-libmvec-sincos-avx-main \ + test-double-libmvec-sincos-avx2-main \ + test-double-libmvec-sincos-main \ + test-float-libmvec-sincosf-avx-main \ + test-float-libmvec-sincosf-avx2-main \ + test-float-libmvec-sincosf-main extra-test-objs += test-double-libmvec-sincos-avx-main.o \ test-double-libmvec-sincos-avx2-main.o \ test-double-libmvec-sincos-main.o \ @@ -66,6 +72,8 @@ ifeq (yes,$(config-cflags-avx512)) libmvec-tests += double-vlen8 float-vlen16 tests += test-double-libmvec-sincos-avx512 \ test-float-libmvec-sincosf-avx512 +test-extras += test-double-libmvec-sincos-avx512-main \ + test-float-libmvec-sincosf-avx512-main extra-test-objs += test-double-libmvec-sincos-avx512-main.o \ test-float-libmvec-sincosf-avx512-main.o -- 2.29.2 [-- Attachment #3: 0002-Make-all-symbols-used-by-_dl_relocate_static_pie-hid.patch --] [-- Type: text/x-patch, Size: 1006 bytes --] From e1e10cd6bd52d9061f138f49b35d4939e1cd5692 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Thu, 14 Jan 2021 16:40:43 -0800 Subject: [PATCH 2/4] Make all symbols used by _dl_relocate_static_pie hidden On i386, all calls to IFUNC functions must go through PLT and calls to hidden functions CANNOT go through PLT in PIE since EBX used in PIE PLT may not be set up for local calls to hidden IFUNC functions. Even if we can't make all libc symbols hidden for static PIE on i386, we must make all symbols used by _dl_relocate_static_pie hidden. --- elf/dl-reloc-static-pie.c | 1 + 1 file changed, 1 insertion(+) diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c index a8d964061e..cc34c2d2fe 100644 --- a/elf/dl-reloc-static-pie.c +++ b/elf/dl-reloc-static-pie.c @@ -17,6 +17,7 @@ <https://www.gnu.org/licenses/>. */ #if ENABLE_STATIC_PIE +#pragma GCC visibility push(hidden) #include <unistd.h> #include <ldsodefs.h> #include "dynamic-link.h" -- 2.29.2 [-- Attachment #4: 0003-i386-Call-_dl_aux_init-after-relocating-static-PIE.patch --] [-- Type: text/x-patch, Size: 17002 bytes --] From c5ffa46591550d945b009f0e3bcf66603d48ac0b Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Thu, 14 Jan 2021 13:26:29 -0800 Subject: [PATCH 3/4] i386: Call _dl_aux_init after relocating static PIE In i386 static PIE, we need to call _dl_aux_init after relocating static PIE so that symbol addresses in _dl_aux_init and ARCH_SETUP_TLS are in sync. Also in i386 static PIE, since __libc_init_secure is called before ARCH_SETUP_TLS, it must use "int $0x80" for system calls. Update __libc_init_secure to use __geteuid_startup, __getuid_startup, __getegid_startup and __getgid_startup. --- config.h.in | 3 +++ csu/libc-start.c | 8 ++++++++ elf/enbl-secure.c | 4 ++-- include/unistd.h | 4 ++++ sysdeps/i386/configure | 3 +++ sysdeps/i386/configure.ac | 4 ++++ sysdeps/mach/hurd/getegid.c | 1 + sysdeps/mach/hurd/geteuid.c | 1 + sysdeps/mach/hurd/getgid.c | 1 + sysdeps/mach/hurd/getuid.c | 1 + sysdeps/unix/syscalls.list | 4 ++-- sysdeps/unix/sysv/linux/arm/syscalls.list | 8 ++++---- sysdeps/unix/sysv/linux/i386/Makefile | 8 ++++++++ sysdeps/unix/sysv/linux/i386/getegid-startup.S | 12 ++++++++++++ sysdeps/unix/sysv/linux/i386/geteuid-startup.S | 12 ++++++++++++ sysdeps/unix/sysv/linux/i386/getgid-startup.S | 12 ++++++++++++ sysdeps/unix/sysv/linux/i386/getuid-startup.S | 12 ++++++++++++ sysdeps/unix/sysv/linux/m68k/syscalls.list | 8 ++++---- sysdeps/unix/sysv/linux/s390/s390-32/syscalls.list | 8 ++++---- sysdeps/unix/sysv/linux/sh/syscalls.list | 8 ++++---- sysdeps/unix/sysv/linux/sparc/sparc32/syscalls.list | 8 ++++---- sysdeps/unix/sysv/linux/syscalls.list | 4 ++-- 22 files changed, 108 insertions(+), 26 deletions(-) create mode 100644 sysdeps/unix/sysv/linux/i386/getegid-startup.S create mode 100644 sysdeps/unix/sysv/linux/i386/geteuid-startup.S create mode 100644 sysdeps/unix/sysv/linux/i386/getgid-startup.S create mode 100644 sysdeps/unix/sysv/linux/i386/getuid-startup.S diff --git a/config.h.in b/config.h.in index 947feeff36..70d3cbde0f 100644 --- a/config.h.in +++ b/config.h.in @@ -94,6 +94,9 @@ and does not need relocations. */ #undef PI_STATIC_AND_HIDDEN +/* Define if _dl_aux_init should be called after relocating static PIE. */ +#undef CALL_DL_AUX_INIT_AFTER_RELOCATING_STATIC_PIE + /* Define this to disable the 'hidden_proto' et al macros in include/libc-symbols.h that avoid PLT slots in PIE. */ #undef NO_HIDDEN_EXTERN_FUNC_IN_PIE diff --git a/csu/libc-start.c b/csu/libc-start.c index fb64cdb2c9..e82c9473c8 100644 --- a/csu/libc-start.c +++ b/csu/libc-start.c @@ -162,7 +162,10 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), auxvec = (ElfW(auxv_t) *) evp; } # endif +# if !(BUILD_PIE_DEFAULT && defined HAVE_AUX_VECTOR \ + && defined CALL_DL_AUX_INIT_AFTER_RELOCATING_STATIC_PIE) _dl_aux_init (auxvec); +# endif # endif /* Initialize very early so that tunables can use it. */ @@ -177,6 +180,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), must be avoided. */ _dl_relocate_static_pie (); +# if BUILD_PIE_DEFAULT && defined HAVE_AUX_VECTOR \ + && defined CALL_DL_AUX_INIT_AFTER_RELOCATING_STATIC_PIE + _dl_aux_init (auxvec); +# endif + /* Perform IREL{,A} relocations. */ ARCH_SETUP_IREL (); diff --git a/elf/enbl-secure.c b/elf/enbl-secure.c index bc8c5e96d2..2fadf96eed 100644 --- a/elf/enbl-secure.c +++ b/elf/enbl-secure.c @@ -31,6 +31,6 @@ void __libc_init_secure (void) { if (__libc_enable_secure_decided == 0) - __libc_enable_secure = (__geteuid () != __getuid () - || __getegid () != __getgid ()); + __libc_enable_secure = (__geteuid_startup () != __getuid_startup () + || __getegid_startup () != __getgid_startup ()); } diff --git a/include/unistd.h b/include/unistd.h index 54becbc9eb..67fe75f1e0 100644 --- a/include/unistd.h +++ b/include/unistd.h @@ -113,9 +113,13 @@ libc_hidden_proto (__getpid) extern __pid_t __getppid (void); extern __pid_t __setsid (void) attribute_hidden; extern __uid_t __getuid (void) attribute_hidden; +extern __uid_t __getuid_startup (void) attribute_hidden; extern __uid_t __geteuid (void) attribute_hidden; +extern __uid_t __geteuid_startup (void) attribute_hidden; extern __gid_t __getgid (void) attribute_hidden; +extern __gid_t __getgid_startup (void) attribute_hidden; extern __gid_t __getegid (void) attribute_hidden; +extern __gid_t __getegid_startup (void) attribute_hidden; extern int __getgroups (int __size, __gid_t __list[]) attribute_hidden; libc_hidden_proto (__getpgid) extern int __group_member (__gid_t __gid) attribute_hidden; diff --git a/sysdeps/i386/configure b/sysdeps/i386/configure index 90c63caf35..60f7afd66b 100644 --- a/sysdeps/i386/configure +++ b/sysdeps/i386/configure @@ -113,6 +113,9 @@ fi $as_echo "#define PI_STATIC_AND_HIDDEN 1" >>confdefs.h +$as_echo "#define CALL_DL_AUX_INIT_AFTER_RELOCATING_STATIC_PIE 1" >>confdefs.h + + if test x"$multi_arch" != xno; then $as_echo "#define NO_HIDDEN_EXTERN_FUNC_IN_PIE 1" >>confdefs.h diff --git a/sysdeps/i386/configure.ac b/sysdeps/i386/configure.ac index 6d2068d2b3..0a7f233de8 100644 --- a/sysdeps/i386/configure.ac +++ b/sysdeps/i386/configure.ac @@ -72,6 +72,10 @@ dnl It is always possible to access static and hidden symbols in an dnl position independent way. AC_DEFINE(PI_STATIC_AND_HIDDEN) +dnl Call _dl_aux_init after relocating static PIE so that symbol addresses +dnl in _dl_aux_init and ARCH_SETUP_TLS are in sync. +AC_DEFINE(CALL_DL_AUX_INIT_AFTER_RELOCATING_STATIC_PIE) + dnl When multi-arch is enabled, all external functions must be called dnl via PIC PLT in PIE, which requires setting up EBX register. if test x"$multi_arch" != xno; then diff --git a/sysdeps/mach/hurd/getegid.c b/sysdeps/mach/hurd/getegid.c index 5a3db22746..da860d3544 100644 --- a/sysdeps/mach/hurd/getegid.c +++ b/sysdeps/mach/hurd/getegid.c @@ -54,3 +54,4 @@ __getegid (void) } weak_alias (__getegid, getegid) +weak_alias (__getegid, __getegid_startup) diff --git a/sysdeps/mach/hurd/geteuid.c b/sysdeps/mach/hurd/geteuid.c index a7af5a9d0d..eecdb7383b 100644 --- a/sysdeps/mach/hurd/geteuid.c +++ b/sysdeps/mach/hurd/geteuid.c @@ -54,3 +54,4 @@ __geteuid (void) } weak_alias (__geteuid, geteuid) +weak_alias (__geteuid, __geteuid_startup) diff --git a/sysdeps/mach/hurd/getgid.c b/sysdeps/mach/hurd/getgid.c index aa13884a8f..4f9414fee7 100644 --- a/sysdeps/mach/hurd/getgid.c +++ b/sysdeps/mach/hurd/getgid.c @@ -51,3 +51,4 @@ __getgid (void) } weak_alias (__getgid, getgid) +weak_alias (__getgid, __getgid_startup) diff --git a/sysdeps/mach/hurd/getuid.c b/sysdeps/mach/hurd/getuid.c index fc4a441569..7a70d05bf8 100644 --- a/sysdeps/mach/hurd/getuid.c +++ b/sysdeps/mach/hurd/getuid.c @@ -51,3 +51,4 @@ __getuid (void) } weak_alias (__getuid, getuid) +weak_alias (__getuid, __getuid_startup) diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list index 7209c24110..6850bc5913 100644 --- a/sysdeps/unix/syscalls.list +++ b/sysdeps/unix/syscalls.list @@ -21,7 +21,7 @@ fcntl - fcntl Ci:iiF __libc_fcntl __fcntl fcntl fstatfs - fstatfs i:ip __fstatfs fstatfs ftruncate - ftruncate i:ii __ftruncate ftruncate getdomain - getdomainname i:si getdomainname -getgid - getgid Ei: __getgid getgid +getgid - getgid Ei: __getgid getgid __getgid_startup getgroups - getgroups i:ip __getgroups getgroups gethostid - gethostid i: gethostid gethostname - gethostname i:bn __gethostname gethostname @@ -31,7 +31,7 @@ getpriority - getpriority i:ii __getpriority getpriority getrlimit - getrlimit i:ip __getrlimit getrlimit getsockname - getsockname i:ibN __getsockname getsockname getsockopt - getsockopt i:iiiBN getsockopt -getuid - getuid Ei: __getuid getuid +getuid - getuid Ei: __getuid getuid __getuid_startup ioctl - ioctl i:iiI __ioctl ioctl kill - kill i:ii __kill kill link - link i:ss __link link diff --git a/sysdeps/unix/sysv/linux/arm/syscalls.list b/sysdeps/unix/sysv/linux/arm/syscalls.list index 13441f7eb4..4b5b1f1960 100644 --- a/sysdeps/unix/sysv/linux/arm/syscalls.list +++ b/sysdeps/unix/sysv/linux/arm/syscalls.list @@ -4,10 +4,10 @@ chown - chown32 i:sii __chown chown lchown - lchown32 i:sii __lchown lchown fchown - fchown32 i:iii __fchown fchown -getegid - getegid32 Ei: __getegid getegid -geteuid - geteuid32 Ei: __geteuid geteuid -getgid - getgid32 Ei: __getgid getgid -getuid - getuid32 Ei: __getuid getuid +getegid - getegid32 Ei: __getegid getegid __getegid_startup +geteuid - geteuid32 Ei: __geteuid geteuid __getegid_startup +getgid - getgid32 Ei: __getgid getgid __getgid_startup +getuid - getuid32 Ei: __getuid getuid __getuid_startup getresgid - getresgid32 i:ppp __getresgid getresgid getresuid - getresuid32 i:ppp __getresuid getresuid getgroups - getgroups32 i:ip __getgroups getgroups diff --git a/sysdeps/unix/sysv/linux/i386/Makefile b/sysdeps/unix/sysv/linux/i386/Makefile index da716e2c1b..e3732bc1a8 100644 --- a/sysdeps/unix/sysv/linux/i386/Makefile +++ b/sysdeps/unix/sysv/linux/i386/Makefile @@ -23,6 +23,14 @@ libpthread-sysdep_routines += libc-do-syscall libpthread-shared-only-routines += libc-do-syscall endif +ifeq ($(subdir),posix) +sysdep_routines += \ + getegid-startup \ + geteuid-startup \ + getgid-startup \ + getuid-startup +endif + ifeq ($(subdir),stdlib) gen-as-const-headers += ucontext_i.sym endif diff --git a/sysdeps/unix/sysv/linux/i386/getegid-startup.S b/sysdeps/unix/sysv/linux/i386/getegid-startup.S new file mode 100644 index 0000000000..4e2cb01361 --- /dev/null +++ b/sysdeps/unix/sysv/linux/i386/getegid-startup.S @@ -0,0 +1,12 @@ +#ifndef SHARED +/* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE. */ +# define I386_USE_SYSENTER 0 +# define SYSCALL_NAME getegid32 +# define SYSCALL_NARGS 0 +# define SYSCALL_ULONG_ARG_1 0 +# define SYSCALL_ULONG_ARG_2 0 +# define SYSCALL_SYMBOL __getegid_startup +# define SYSCALL_NOERRNO 1 +# define SYSCALL_ERRVAL 0 +# include <syscall-template.S> +#endif diff --git a/sysdeps/unix/sysv/linux/i386/geteuid-startup.S b/sysdeps/unix/sysv/linux/i386/geteuid-startup.S new file mode 100644 index 0000000000..ed97bdb3e1 --- /dev/null +++ b/sysdeps/unix/sysv/linux/i386/geteuid-startup.S @@ -0,0 +1,12 @@ +#ifndef SHARED +/* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE. */ +# define I386_USE_SYSENTER 0 +# define SYSCALL_NAME geteuid32 +# define SYSCALL_NARGS 0 +# define SYSCALL_ULONG_ARG_1 0 +# define SYSCALL_ULONG_ARG_2 0 +# define SYSCALL_SYMBOL __geteuid_startup +# define SYSCALL_NOERRNO 1 +# define SYSCALL_ERRVAL 0 +# include <syscall-template.S> +#endif diff --git a/sysdeps/unix/sysv/linux/i386/getgid-startup.S b/sysdeps/unix/sysv/linux/i386/getgid-startup.S new file mode 100644 index 0000000000..dd75b3f2c2 --- /dev/null +++ b/sysdeps/unix/sysv/linux/i386/getgid-startup.S @@ -0,0 +1,12 @@ +#ifndef SHARED +/* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE. */ +# define I386_USE_SYSENTER 0 +# define SYSCALL_NAME getgid32 +# define SYSCALL_NARGS 0 +# define SYSCALL_ULONG_ARG_1 0 +# define SYSCALL_ULONG_ARG_2 0 +# define SYSCALL_SYMBOL __getgid_startup +# define SYSCALL_NOERRNO 1 +# define SYSCALL_ERRVAL 0 +# include <syscall-template.S> +#endif diff --git a/sysdeps/unix/sysv/linux/i386/getuid-startup.S b/sysdeps/unix/sysv/linux/i386/getuid-startup.S new file mode 100644 index 0000000000..51fed82ca2 --- /dev/null +++ b/sysdeps/unix/sysv/linux/i386/getuid-startup.S @@ -0,0 +1,12 @@ +#ifndef SHARED +/* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE. */ +# define I386_USE_SYSENTER 0 +# define SYSCALL_NAME getuid32 +# define SYSCALL_NARGS 0 +# define SYSCALL_ULONG_ARG_1 0 +# define SYSCALL_ULONG_ARG_2 0 +# define SYSCALL_SYMBOL __getuid_startup +# define SYSCALL_NOERRNO 1 +# define SYSCALL_ERRVAL 0 +# include <syscall-template.S> +#endif diff --git a/sysdeps/unix/sysv/linux/m68k/syscalls.list b/sysdeps/unix/sysv/linux/m68k/syscalls.list index 55a377b841..529b548ce5 100644 --- a/sysdeps/unix/sysv/linux/m68k/syscalls.list +++ b/sysdeps/unix/sysv/linux/m68k/syscalls.list @@ -4,10 +4,10 @@ chown - chown32 i:sii __chown chown lchown - lchown32 i:sii __lchown lchown fchown - fchown32 i:iii __fchown fchown -getegid - getegid32 Ei: __getegid getegid -geteuid - geteuid32 Ei: __geteuid geteuid -getgid - getgid32 Ei: __getgid getgid -getuid - getuid32 Ei: __getuid getuid +getegid - getegid32 Ei: __getegid getegid __getegid_startup +geteuid - geteuid32 Ei: __geteuid geteuid __geteuid_startup +getgid - getgid32 Ei: __getgid getgid __getgid_startup +getuid - getuid32 Ei: __getuid getuid __getuid_startup getresgid - getresgid32 i:ppp __getresgid getresgid getresuid - getresuid32 i:ppp __getresuid getresuid getgroups - getgroups32 i:ip __getgroups getgroups diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/syscalls.list b/sysdeps/unix/sysv/linux/s390/s390-32/syscalls.list index 300b13dd01..66253a20ff 100644 --- a/sysdeps/unix/sysv/linux/s390/s390-32/syscalls.list +++ b/sysdeps/unix/sysv/linux/s390/s390-32/syscalls.list @@ -4,10 +4,10 @@ chown - chown32 i:sii __chown chown@@GLIBC_2.1 lchown - lchown32 i:sii __lchown lchown@@GLIBC_2.0 chown@GLIBC_2.0 fchown - fchown32 i:iii __fchown fchown -getegid - getegid32 Ei: __getegid getegid -geteuid - geteuid32 Ei: __geteuid geteuid -getgid - getgid32 Ei: __getgid getgid -getuid - getuid32 Ei: __getuid getuid +getegid - getegid32 Ei: __getegid getegid __getegid_startup +geteuid - geteuid32 Ei: __geteuid geteuid __geteuid_startup +getgid - getgid32 Ei: __getgid getgid __getgid_startup +getuid - getuid32 Ei: __getuid getuid __getuid_startup getresgid - getresgid32 i:ppp __getresgid getresgid getresuid - getresuid32 i:ppp __getresuid getresuid getgroups - getgroups32 i:ip __getgroups getgroups diff --git a/sysdeps/unix/sysv/linux/sh/syscalls.list b/sysdeps/unix/sysv/linux/sh/syscalls.list index 32badd1ee0..603f324f60 100644 --- a/sysdeps/unix/sysv/linux/sh/syscalls.list +++ b/sysdeps/unix/sysv/linux/sh/syscalls.list @@ -4,10 +4,10 @@ chown - chown32 i:sii __chown chown lchown - lchown32 i:sii __lchown lchown fchown - fchown32 i:iii __fchown fchown -getegid - getegid32 Ei: __getegid getegid -geteuid - geteuid32 Ei: __geteuid geteuid -getgid - getgid32 Ei: __getgid getgid -getuid - getuid32 Ei: __getuid getuid +getegid - getegid32 Ei: __getegid getegid __getegid_startup +geteuid - geteuid32 Ei: __geteuid geteuid __geteuid_startup +getgid - getgid32 Ei: __getgid getgid __getgid_startup +getuid - getuid32 Ei: __getuid getuid __getuid_startup getresgid - getresgid32 i:ppp __getresgid getresgid getresuid - getresuid32 i:ppp __getresuid getresuid getgroups - getgroups32 i:ip __getgroups getgroups diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/syscalls.list b/sysdeps/unix/sysv/linux/sparc/sparc32/syscalls.list index 0b6095ffab..672e7f7254 100644 --- a/sysdeps/unix/sysv/linux/sparc/sparc32/syscalls.list +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/syscalls.list @@ -4,10 +4,10 @@ chown - chown32 i:sii __chown chown lchown - lchown32 i:sii __lchown lchown fchown - fchown32 i:iii __fchown fchown -getegid - getegid32 Ei: __getegid getegid -geteuid - geteuid32 Ei: __geteuid geteuid -getgid - getgid32 Ei: __getgid getgid -getuid - getuid32 Ei: __getuid getuid +getegid - getegid32 Ei: __getegid getegid __getegid_startup +geteuid - geteuid32 Ei: __geteuid geteuid __geteuid_startup +getgid - getgid32 Ei: __getgid getgid __getgid_startup +getuid - getuid32 Ei: __getuid getuid __getuid_startup getresuid - getresuid32 3 getresuid getresgid - getresgid32 3 getresgid getgroups - getgroups32 i:ip __getgroups getgroups diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list index 01ec2bfa95..b1b8940772 100644 --- a/sysdeps/unix/sysv/linux/syscalls.list +++ b/sysdeps/unix/sysv/linux/syscalls.list @@ -14,8 +14,8 @@ execve - execve i:spp __execve execve flock - flock i:ii __flock flock get_kernel_syms EXTRA get_kernel_syms i:p __compat_get_kernel_syms get_kernel_syms@GLIBC_2.0:GLIBC_2.23 getpid - getpid Ei: __getpid getpid -getegid - getegid Ei: __getegid getegid -geteuid - geteuid Ei: __geteuid geteuid +getegid - getegid Ei: __getegid getegid __getegid_startup +geteuid - geteuid Ei: __geteuid geteuid __geteuid_startup getpgid - getpgid i:i __getpgid getpgid getpgrp - getpgrp Ei: getpgrp getppid - getppid Ei: __getppid getppid -- 2.29.2 [-- Attachment #5: 0004-x86-Check-ifunc-resolver-with-CPU_FEATURE_USABLE-BZ-.patch --] [-- Type: text/x-patch, Size: 3938 bytes --] From e289c42f8b40b76bb029779a6cb73c739f1f9c72 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Tue, 12 Jan 2021 14:41:10 -0800 Subject: [PATCH 4/4] x86: Check ifunc resolver with CPU_FEATURE_USABLE [BZ #27072] Check ifunc resolver with CPU_FEATURE_USABLE in dynamic and static executables to verify that CPUID features are initialized early in static PIE. --- sysdeps/x86/Makefile | 7 ++ sysdeps/x86/tst-ifunc-isa-1-static.c | 1 + sysdeps/x86/tst-ifunc-isa-1.c | 115 +++++++++++++++++++++++++++ 3 files changed, 123 insertions(+) create mode 100644 sysdeps/x86/tst-ifunc-isa-1-static.c create mode 100644 sysdeps/x86/tst-ifunc-isa-1.c diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile index adaa2a92cd..996ff46dbc 100644 --- a/sysdeps/x86/Makefile +++ b/sysdeps/x86/Makefile @@ -9,6 +9,13 @@ sysdep_headers += sys/platform/x86.h tests += tst-get-cpu-features tst-get-cpu-features-static \ tst-cpu-features-cpuinfo tst-cpu-features-supports tests-static += tst-get-cpu-features-static +ifeq (yes,$(have-ifunc)) +tests += \ + tst-ifunc-isa-1 \ + tst-ifunc-isa-1-static +tests-static += \ + tst-ifunc-isa-1-static +endif ifeq (yes,$(enable-x86-isa-level)) tests += tst-isa-level-1 modules-names += tst-isa-level-mod-1-baseline \ diff --git a/sysdeps/x86/tst-ifunc-isa-1-static.c b/sysdeps/x86/tst-ifunc-isa-1-static.c new file mode 100644 index 0000000000..0e94f6119b --- /dev/null +++ b/sysdeps/x86/tst-ifunc-isa-1-static.c @@ -0,0 +1 @@ +#include "tst-ifunc-isa-1.c" diff --git a/sysdeps/x86/tst-ifunc-isa-1.c b/sysdeps/x86/tst-ifunc-isa-1.c new file mode 100644 index 0000000000..b3bc2a55a2 --- /dev/null +++ b/sysdeps/x86/tst-ifunc-isa-1.c @@ -0,0 +1,115 @@ +/* Check ifunc with CPU_FEATURE_USABLE. + Copyright (C) 2021 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <stdlib.h> +#include <sys/platform/x86.h> + +enum isa +{ + none, + sse2, + sse4_2, + avx, + avx2, + avx512f +}; + +enum isa +get_isa (void) +{ + if (CPU_FEATURE_USABLE (AVX512F)) + return avx512f; + if (CPU_FEATURE_USABLE (AVX2)) + return avx2; + if (CPU_FEATURE_USABLE (AVX)) + return avx; + if (CPU_FEATURE_USABLE (SSE4_2)) + return sse4_2; + if (CPU_FEATURE_USABLE (SSE2)) + return sse2; + return none; +} + +static int +isa_sse2 (void) +{ + return sse2; +} + +static int +isa_sse4_2 (void) +{ + return sse4_2; +} + +static int +isa_avx (void) +{ + return avx; +} + +static int +isa_avx2 (void) +{ + return avx2; +} + +static int +isa_avx512f (void) +{ + return avx512f; +} + +static int +isa_none (void) +{ + return none; +} + +int foo (void) __attribute__ ((ifunc ("foo_ifunc"))); + +void * +foo_ifunc (void) +{ + switch (get_isa ()) + { + case avx512f: + return isa_avx512f; + case avx2: + return isa_avx2; + case avx: + return isa_avx; + case sse4_2: + return isa_sse4_2; + case sse2: + return isa_sse2; + default: + break; + } + return isa_none; +} + +static int +do_test (void) +{ + enum isa value = foo (); + enum isa expected = get_isa (); + return value == expected ? EXIT_SUCCESS : EXIT_FAILURE; +} + +#include <support/test-driver.c> -- 2.29.2 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-15 3:36 ` H.J. Lu @ 2021-01-15 4:29 ` H.J. Lu 2021-01-15 11:25 ` Szabolcs Nagy 1 sibling, 0 replies; 34+ messages in thread From: H.J. Lu @ 2021-01-15 4:29 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Thu, Jan 14, 2021 at 7:36 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote: > > > The 01/12/2021 17:19, H.J. Lu wrote: > > > > On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > See: > > > > > > > > > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961 > > > > > > > > > > > /* Mark all symbols hidden in static PIE libc to avoid GOT indirections. */ > > > > > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED > > > > > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \ > > > > > + && IS_IN (libc) && !defined LIBC_NONSHARED > > > > > # pragma GCC visibility push(hidden) > > > > > #endif > > > > > > > > > > > > > This works on i686. > > > > The series i plan to commit today is in nsz/bug27072 now, > > > > This is the v4 of this patch: > > > > Hidden matters with static PIE: extern symbol access in position > > independent code usually involves GOT indirections which needs > > RELATIVE relocs in a static linked PIE. Hidden visibility avoids > > indirections and RELATIVE relocs on targets that can access symbols > > pc-relative. > > > > The check should use IS_IN_LIB instead of IS_IN(libc) since all > > static libraries can use hidden visibility to avoid indirections, > > however the test system links objects from libcrypt.a into dynamic > > linked test binaries so hidden does not work there. I think mixing > > static and shared libc components in the same binary should not be > > supported usage, but to be safe only use hidden in libc.a. > > > > There are targets (i686) where hidden visibility functions are > > problematic in PIE code so hidden cannot be applied to all symbols. > > Then static PIE requires extern object access without relocations > > (e.g. by relying on copy relocations in shared libraries instead of > > GOT access in PIE code). See bug 14961. > > It isn't about copy relocations. It is IFUNC, PLT and PIE. I needed > additional patches to make static PIE to work on i386 and x86-64. > I am enclosing my patches. Please include them in your patch set. > > > From -static-pie linked 'int main(){}' this shaves off 73 relative > > relocs on aarch64 and reduces code size too. > > > -- > H.J. commit c5ffa46591550d945b009f0e3bcf66603d48ac0b Author: H.J. Lu <hjl.tools@gmail.com> Date: Thu Jan 14 13:26:29 2021 -0800 i386: Call _dl_aux_init after relocating static PIE is too complicated. I will submit a simple version. -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-15 3:36 ` H.J. Lu 2021-01-15 4:29 ` H.J. Lu @ 2021-01-15 11:25 ` Szabolcs Nagy 2021-01-15 13:43 ` H.J. Lu 1 sibling, 1 reply; 34+ messages in thread From: Szabolcs Nagy @ 2021-01-15 11:25 UTC (permalink / raw) To: H.J. Lu; +Cc: GNU C Library The 01/14/2021 19:36, H.J. Lu wrote: > On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote: > > There are targets (i686) where hidden visibility functions are > > problematic in PIE code so hidden cannot be applied to all symbols. > > Then static PIE requires extern object access without relocations > > (e.g. by relying on copy relocations in shared libraries instead of > > GOT access in PIE code). See bug 14961. > > It isn't about copy relocations. It is IFUNC, PLT and PIE. I needed > additional patches to make static PIE to work on i386 and x86-64. > I am enclosing my patches. Please include them in your patch set. it is about avoiding GOT for object access, which copy relocations can do. hidden does it better, but you said that does not work on i686 so i removed it (assuming you know the implications: every pie object access must be local and copy relocated in shared libraries) morally all symbols should be hidden in static pie because everything is local (the code is only linked into static exectuables). this is useful outside the start code too to avoid GOT indirections. i686 does not want to set up EBX for hidden extern calls, which is needed for ifuncs, so making everything hidden does not work. options: (1) fix extern hidden pie calls on i686 (by making them the same as default vis pie calls so ifuncs work). (2) annotate ifuncs (avoid hidden for them, ifuncs cannot appear in early start code anyway because of IRELATIVE): this can be difficult to maintain. (3) annotate early object accesses to be hidden so RELATIVE relocs are avoided. (most targets want all objects to be hidden, but this solves bug 27072 without causing problems on i686) (4) make pie always use copy relocations on i686. (and then no hidden annotation is needed, object access is always local in pie). my patches assumed (4), but that seems to not work so i think doing (3) is reasonable: you either need a few carefully placed 'pragma GCC visibility push(hidden)' or an attribute_hidden_pie_data on object declarations that may be used by the early start code. > From 15488890220a8c580689e6f78a38847853b78850 Mon Sep 17 00:00:00 2001 > From: "H.J. Lu" <hjl.tools@gmail.com> > Date: Thu, 14 Jan 2021 18:39:24 -0800 > Subject: [PATCH 1/4] libmvec: Add extra-test-objs to test-extras > > Add extra-test-objs to test-extras so that they are compiled with > -DMODULE_NAME=testsuite instead of -DMODULE_NAME=libc. this makes sense. > From e1e10cd6bd52d9061f138f49b35d4939e1cd5692 Mon Sep 17 00:00:00 2001 > From: "H.J. Lu" <hjl.tools@gmail.com> > Date: Thu, 14 Jan 2021 16:40:43 -0800 > Subject: [PATCH 2/4] Make all symbols used by _dl_relocate_static_pie hidden > > On i386, all calls to IFUNC functions must go through PLT and calls to > hidden functions CANNOT go through PLT in PIE since EBX used in PIE PLT > may not be set up for local calls to hidden IFUNC functions. > > Even if we can't make all libc symbols hidden for static PIE on i386, we > must make all symbols used by _dl_relocate_static_pie hidden. > --- > elf/dl-reloc-static-pie.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c > index a8d964061e..cc34c2d2fe 100644 > --- a/elf/dl-reloc-static-pie.c > +++ b/elf/dl-reloc-static-pie.c > @@ -17,6 +17,7 @@ > <https://www.gnu.org/licenses/>. */ > > #if ENABLE_STATIC_PIE > +#pragma GCC visibility push(hidden) yes, this is option (3). you will also need it in _dl_aux_init and __libc_init_secure and __tunables_init. > From c5ffa46591550d945b009f0e3bcf66603d48ac0b Mon Sep 17 00:00:00 2001 > From: "H.J. Lu" <hjl.tools@gmail.com> > Date: Thu, 14 Jan 2021 13:26:29 -0800 > Subject: [PATCH 3/4] i386: Call _dl_aux_init after relocating static PIE > > In i386 static PIE, we need to call _dl_aux_init after relocating static > PIE so that symbol addresses in _dl_aux_init and ARCH_SETUP_TLS are in > sync. Also in i386 static PIE, since __libc_init_secure is called before > ARCH_SETUP_TLS, it must use "int $0x80" for system calls. Update > __libc_init_secure to use __geteuid_startup, __getuid_startup, > __getegid_startup and __getgid_startup. the syscall part i understand, but auxv vs tls i don't: i thought you only need to ensure that objects are hidden visibility in _dl_aux_init. i think the dependency order is: 1 auxv 2 libc_secure 3 tunables 4 cpu features 5 self reloc 6 setup _dl_phdr from __ehdr_start 7 setup tls i got 6 wrong in my patch: setup tls can use _dl_phdr, i will fix it. moving auxv a bit later is possible (if you don't mind syscalls in libc_secure and nothing requires it in cpu features), but i don't see how that's related to tls. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-15 11:25 ` Szabolcs Nagy @ 2021-01-15 13:43 ` H.J. Lu 2021-01-15 14:27 ` Szabolcs Nagy 0 siblings, 1 reply; 34+ messages in thread From: H.J. Lu @ 2021-01-15 13:43 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > The 01/14/2021 19:36, H.J. Lu wrote: > > On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote: > > > There are targets (i686) where hidden visibility functions are > > > problematic in PIE code so hidden cannot be applied to all symbols. > > > Then static PIE requires extern object access without relocations > > > (e.g. by relying on copy relocations in shared libraries instead of > > > GOT access in PIE code). See bug 14961. > > > > It isn't about copy relocations. It is IFUNC, PLT and PIE. I needed > > additional patches to make static PIE to work on i386 and x86-64. > > I am enclosing my patches. Please include them in your patch set. > > it is about avoiding GOT for object access, which copy > relocations can do. hidden does it better, but you said > that does not work on i686 so i removed it (assuming > you know the implications: every pie object access must > be local and copy relocated in shared libraries) > > morally all symbols should be hidden in static pie > because everything is local (the code is only linked > into static exectuables). this is useful outside the > start code too to avoid GOT indirections. > > i686 does not want to set up EBX for hidden extern calls, > which is needed for ifuncs, so making everything hidden > does not work. > > options: > > (1) fix extern hidden pie calls on i686 (by making > them the same as default vis pie calls so ifuncs > work). > > (2) annotate ifuncs (avoid hidden for them, ifuncs > cannot appear in early start code anyway because > of IRELATIVE): this can be difficult to maintain. > > (3) annotate early object accesses to be hidden so > RELATIVE relocs are avoided. (most targets want > all objects to be hidden, but this solves bug > 27072 without causing problems on i686) > > (4) make pie always use copy relocations on i686. > (and then no hidden annotation is needed, object > access is always local in pie). Linker doesn't generate copy relocations for static PIE. The problem is GOT indirections which require RELATIVE relocations. > my patches assumed (4), but that seems to not work so > i think doing (3) is reasonable: you either need a few > carefully placed 'pragma GCC visibility push(hidden)' > or an attribute_hidden_pie_data on object declarations > that may be used by the early start code. > > > From 15488890220a8c580689e6f78a38847853b78850 Mon Sep 17 00:00:00 2001 > > From: "H.J. Lu" <hjl.tools@gmail.com> > > Date: Thu, 14 Jan 2021 18:39:24 -0800 > > Subject: [PATCH 1/4] libmvec: Add extra-test-objs to test-extras > > > > Add extra-test-objs to test-extras so that they are compiled with > > -DMODULE_NAME=testsuite instead of -DMODULE_NAME=libc. > > this makes sense. > > > From e1e10cd6bd52d9061f138f49b35d4939e1cd5692 Mon Sep 17 00:00:00 2001 > > From: "H.J. Lu" <hjl.tools@gmail.com> > > Date: Thu, 14 Jan 2021 16:40:43 -0800 > > Subject: [PATCH 2/4] Make all symbols used by _dl_relocate_static_pie hidden > > > > On i386, all calls to IFUNC functions must go through PLT and calls to > > hidden functions CANNOT go through PLT in PIE since EBX used in PIE PLT > > may not be set up for local calls to hidden IFUNC functions. > > > > Even if we can't make all libc symbols hidden for static PIE on i386, we > > must make all symbols used by _dl_relocate_static_pie hidden. > > --- > > elf/dl-reloc-static-pie.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c > > index a8d964061e..cc34c2d2fe 100644 > > --- a/elf/dl-reloc-static-pie.c > > +++ b/elf/dl-reloc-static-pie.c > > @@ -17,6 +17,7 @@ > > <https://www.gnu.org/licenses/>. */ > > > > #if ENABLE_STATIC_PIE > > +#pragma GCC visibility push(hidden) > > yes, this is option (3). you will also need it in _dl_aux_init > and __libc_init_secure and __tunables_init. I will try it. > > From c5ffa46591550d945b009f0e3bcf66603d48ac0b Mon Sep 17 00:00:00 2001 > > From: "H.J. Lu" <hjl.tools@gmail.com> > > Date: Thu, 14 Jan 2021 13:26:29 -0800 > > Subject: [PATCH 3/4] i386: Call _dl_aux_init after relocating static PIE > > > > In i386 static PIE, we need to call _dl_aux_init after relocating static > > PIE so that symbol addresses in _dl_aux_init and ARCH_SETUP_TLS are in > > sync. Also in i386 static PIE, since __libc_init_secure is called before > > ARCH_SETUP_TLS, it must use "int $0x80" for system calls. Update > > __libc_init_secure to use __geteuid_startup, __getuid_startup, > > __getegid_startup and __getgid_startup. > > the syscall part i understand, but auxv vs tls i don't: > > i thought you only need to ensure that objects are hidden > visibility in _dl_aux_init. > > i think the dependency order is: > > 1 auxv > 2 libc_secure > 3 tunables > 4 cpu features > 5 self reloc > 6 setup _dl_phdr from __ehdr_start > 7 setup tls > > i got 6 wrong in my patch: setup tls can use _dl_phdr, > i will fix it. On i386, setup tls uses auxv. But there are GOT indirections in _dl_aux_init () which require RELATIVE relocations. > moving auxv a bit later is possible (if you don't mind > syscalls in libc_secure and nothing requires it in > cpu features), but i don't see how that's related to > tls. -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-15 13:43 ` H.J. Lu @ 2021-01-15 14:27 ` Szabolcs Nagy 2021-01-15 15:28 ` H.J. Lu 0 siblings, 1 reply; 34+ messages in thread From: Szabolcs Nagy @ 2021-01-15 14:27 UTC (permalink / raw) To: H.J. Lu; +Cc: GNU C Library The 01/15/2021 05:43, H.J. Lu wrote: > On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > options: > > > > (1) fix extern hidden pie calls on i686 (by making > > them the same as default vis pie calls so ifuncs > > work). > > > > (2) annotate ifuncs (avoid hidden for them, ifuncs > > cannot appear in early start code anyway because > > of IRELATIVE): this can be difficult to maintain. > > > > (3) annotate early object accesses to be hidden so > > RELATIVE relocs are avoided. (most targets want > > all objects to be hidden, but this solves bug > > 27072 without causing problems on i686) > > > > (4) make pie always use copy relocations on i686. > > (and then no hidden annotation is needed, object > > access is always local in pie). ... > > > --- a/elf/dl-reloc-static-pie.c > > > +++ b/elf/dl-reloc-static-pie.c > > > @@ -17,6 +17,7 @@ > > > <https://www.gnu.org/licenses/>. */ > > > > > > #if ENABLE_STATIC_PIE > > > +#pragma GCC visibility push(hidden) > > > > yes, this is option (3). you will also need it in _dl_aux_init > > and __libc_init_secure and __tunables_init. > > I will try it. the naive way does not seem to work: _dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init, the former needs hidden the latter does not and calls rawmemchr which is ifunc on i686. i think the easiest fix is to move those two functions into separate files. (ideally we would have a small set of files that are involved in the start code before self relocation) now i realized that there is another option: (5) remove all ifuncs from i686 libc. i assume there are not many users who care about i686 performance. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-15 14:27 ` Szabolcs Nagy @ 2021-01-15 15:28 ` H.J. Lu 2021-01-15 22:42 ` H.J. Lu 0 siblings, 1 reply; 34+ messages in thread From: H.J. Lu @ 2021-01-15 15:28 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Fri, Jan 15, 2021 at 6:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > The 01/15/2021 05:43, H.J. Lu wrote: > > On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > options: > > > > > > (1) fix extern hidden pie calls on i686 (by making > > > them the same as default vis pie calls so ifuncs > > > work). > > > > > > (2) annotate ifuncs (avoid hidden for them, ifuncs > > > cannot appear in early start code anyway because > > > of IRELATIVE): this can be difficult to maintain. > > > > > > (3) annotate early object accesses to be hidden so > > > RELATIVE relocs are avoided. (most targets want > > > all objects to be hidden, but this solves bug > > > 27072 without causing problems on i686) > > > > > > (4) make pie always use copy relocations on i686. > > > (and then no hidden annotation is needed, object > > > access is always local in pie). > ... > > > > --- a/elf/dl-reloc-static-pie.c > > > > +++ b/elf/dl-reloc-static-pie.c > > > > @@ -17,6 +17,7 @@ > > > > <https://www.gnu.org/licenses/>. */ > > > > > > > > #if ENABLE_STATIC_PIE > > > > +#pragma GCC visibility push(hidden) > > > > > > yes, this is option (3). you will also need it in _dl_aux_init > > > and __libc_init_secure and __tunables_init. > > > > I will try it. > > the naive way does not seem to work: > > _dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init, > the former needs hidden the latter does not and calls rawmemchr > which is ifunc on i686. > > i think the easiest fix is to move those two functions into > separate files. (ideally we would have a small set of files > that are involved in the start code before self relocation) > > now i realized that there is another option: > > (5) remove all ifuncs from i686 libc. > > i assume there are not many users who care about i686 performance. I don't know if this will work on i686. Since i386 doesn't have IP relative addressing, we can't remove all RELATIVE relocations. We need to call _dl_aux_init again after relocating PIE. I don't know what other symbols are affected. My current patches are on users/hjl/pr27072/master branch: https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-15 15:28 ` H.J. Lu @ 2021-01-15 22:42 ` H.J. Lu 2021-01-16 0:41 ` H.J. Lu 0 siblings, 1 reply; 34+ messages in thread From: H.J. Lu @ 2021-01-15 22:42 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library [-- Attachment #1: Type: text/plain, Size: 2709 bytes --] On Fri, Jan 15, 2021 at 7:28 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Fri, Jan 15, 2021 at 6:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > The 01/15/2021 05:43, H.J. Lu wrote: > > > On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > options: > > > > > > > > (1) fix extern hidden pie calls on i686 (by making > > > > them the same as default vis pie calls so ifuncs > > > > work). > > > > > > > > (2) annotate ifuncs (avoid hidden for them, ifuncs > > > > cannot appear in early start code anyway because > > > > of IRELATIVE): this can be difficult to maintain. > > > > > > > > (3) annotate early object accesses to be hidden so > > > > RELATIVE relocs are avoided. (most targets want > > > > all objects to be hidden, but this solves bug > > > > 27072 without causing problems on i686) > > > > > > > > (4) make pie always use copy relocations on i686. > > > > (and then no hidden annotation is needed, object > > > > access is always local in pie). > > ... > > > > > --- a/elf/dl-reloc-static-pie.c > > > > > +++ b/elf/dl-reloc-static-pie.c > > > > > @@ -17,6 +17,7 @@ > > > > > <https://www.gnu.org/licenses/>. */ > > > > > > > > > > #if ENABLE_STATIC_PIE > > > > > +#pragma GCC visibility push(hidden) > > > > > > > > yes, this is option (3). you will also need it in _dl_aux_init > > > > and __libc_init_secure and __tunables_init. > > > > > > I will try it. > > > > the naive way does not seem to work: > > > > _dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init, > > the former needs hidden the latter does not and calls rawmemchr > > which is ifunc on i686. > > > > i think the easiest fix is to move those two functions into > > separate files. (ideally we would have a small set of files > > that are involved in the start code before self relocation) > > > > now i realized that there is another option: > > > > (5) remove all ifuncs from i686 libc. > > > > i assume there are not many users who care about i686 performance. > > I don't know if this will work on i686. Since i386 doesn't have IP relative > addressing, we can't remove all RELATIVE relocations. We need to > call _dl_aux_init again after relocating PIE. I don't know what other symbols > are affected. My current patches are on users/hjl/pr27072/master branch: > > https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master > The problem is #ifdef NEED_DL_SYSINFO /* Needed for improved syscall handling on at least x86/Linux. */ uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT; #endif We can initialize it in _dl_aux_init instead. I am testing this set of patches on top of yours on i686 and x86-64. -- H.J. [-- Attachment #2: 0004-x86-Check-ifunc-resolver-with-CPU_FEATURE_USABLE-BZ-.patch --] [-- Type: text/x-patch, Size: 3938 bytes --] From 5f7eeb0aa5251849ac516e027ea5ec8dfeec5727 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Tue, 12 Jan 2021 14:41:10 -0800 Subject: [PATCH 4/4] x86: Check ifunc resolver with CPU_FEATURE_USABLE [BZ #27072] Check ifunc resolver with CPU_FEATURE_USABLE in dynamic and static executables to verify that CPUID features are initialized early in static PIE. --- sysdeps/x86/Makefile | 7 ++ sysdeps/x86/tst-ifunc-isa-1-static.c | 1 + sysdeps/x86/tst-ifunc-isa-1.c | 115 +++++++++++++++++++++++++++ 3 files changed, 123 insertions(+) create mode 100644 sysdeps/x86/tst-ifunc-isa-1-static.c create mode 100644 sysdeps/x86/tst-ifunc-isa-1.c diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile index adaa2a92cd..996ff46dbc 100644 --- a/sysdeps/x86/Makefile +++ b/sysdeps/x86/Makefile @@ -9,6 +9,13 @@ sysdep_headers += sys/platform/x86.h tests += tst-get-cpu-features tst-get-cpu-features-static \ tst-cpu-features-cpuinfo tst-cpu-features-supports tests-static += tst-get-cpu-features-static +ifeq (yes,$(have-ifunc)) +tests += \ + tst-ifunc-isa-1 \ + tst-ifunc-isa-1-static +tests-static += \ + tst-ifunc-isa-1-static +endif ifeq (yes,$(enable-x86-isa-level)) tests += tst-isa-level-1 modules-names += tst-isa-level-mod-1-baseline \ diff --git a/sysdeps/x86/tst-ifunc-isa-1-static.c b/sysdeps/x86/tst-ifunc-isa-1-static.c new file mode 100644 index 0000000000..0e94f6119b --- /dev/null +++ b/sysdeps/x86/tst-ifunc-isa-1-static.c @@ -0,0 +1 @@ +#include "tst-ifunc-isa-1.c" diff --git a/sysdeps/x86/tst-ifunc-isa-1.c b/sysdeps/x86/tst-ifunc-isa-1.c new file mode 100644 index 0000000000..b3bc2a55a2 --- /dev/null +++ b/sysdeps/x86/tst-ifunc-isa-1.c @@ -0,0 +1,115 @@ +/* Check ifunc with CPU_FEATURE_USABLE. + Copyright (C) 2021 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <stdlib.h> +#include <sys/platform/x86.h> + +enum isa +{ + none, + sse2, + sse4_2, + avx, + avx2, + avx512f +}; + +enum isa +get_isa (void) +{ + if (CPU_FEATURE_USABLE (AVX512F)) + return avx512f; + if (CPU_FEATURE_USABLE (AVX2)) + return avx2; + if (CPU_FEATURE_USABLE (AVX)) + return avx; + if (CPU_FEATURE_USABLE (SSE4_2)) + return sse4_2; + if (CPU_FEATURE_USABLE (SSE2)) + return sse2; + return none; +} + +static int +isa_sse2 (void) +{ + return sse2; +} + +static int +isa_sse4_2 (void) +{ + return sse4_2; +} + +static int +isa_avx (void) +{ + return avx; +} + +static int +isa_avx2 (void) +{ + return avx2; +} + +static int +isa_avx512f (void) +{ + return avx512f; +} + +static int +isa_none (void) +{ + return none; +} + +int foo (void) __attribute__ ((ifunc ("foo_ifunc"))); + +void * +foo_ifunc (void) +{ + switch (get_isa ()) + { + case avx512f: + return isa_avx512f; + case avx2: + return isa_avx2; + case avx: + return isa_avx; + case sse4_2: + return isa_sse4_2; + case sse2: + return isa_sse2; + default: + break; + } + return isa_none; +} + +static int +do_test (void) +{ + enum isa value = foo (); + enum isa expected = get_isa (); + return value == expected ? EXIT_SUCCESS : EXIT_FAILURE; +} + +#include <support/test-driver.c> -- 2.29.2 [-- Attachment #3: 0003-Use-startup.h-in-__libc_init_secure.patch --] [-- Type: text/x-patch, Size: 3377 bytes --] From 24676232cd5485fc8b4d9707eb81ce0ba7e65f4a Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Fri, 15 Jan 2021 06:46:12 -0800 Subject: [PATCH 3/4] Use <startup.h> in __libc_init_secure Since __libc_init_secure is called before ARCH_SETUP_TLS, it must use "int $0x80" for system calls in i386 static PIE. Add startup_getuid, startup_geteuid, startup_getgid and startup_getegid to <startup.h>. Update __libc_init_secure to use them. --- elf/enbl-secure.c | 6 +++--- sysdeps/generic/startup.h | 26 +++++++++++++++++++++++ sysdeps/unix/sysv/linux/i386/startup.h | 29 ++++++++++++++++++++++++-- 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/elf/enbl-secure.c b/elf/enbl-secure.c index dba39a123e..3b1dd9795f 100644 --- a/elf/enbl-secure.c +++ b/elf/enbl-secure.c @@ -22,7 +22,7 @@ #if BUILD_PIE_DEFAULT # pragma GCC visibility push(hidden) #endif -#include <unistd.h> +#include <startup.h> #include <libc-internal.h> /* If nonzero __libc_enable_secure is already set. */ @@ -34,6 +34,6 @@ void __libc_init_secure (void) { if (__libc_enable_secure_decided == 0) - __libc_enable_secure = (__geteuid () != __getuid () - || __getegid () != __getgid ()); + __libc_enable_secure = (startup_geteuid () != startup_getuid () + || startup_getegid () != startup_getgid ()); } diff --git a/sysdeps/generic/startup.h b/sysdeps/generic/startup.h index 56c899a65e..04f20cde47 100644 --- a/sysdeps/generic/startup.h +++ b/sysdeps/generic/startup.h @@ -19,5 +19,31 @@ /* Targets should override this file if the default definitions below will not work correctly very early before TLS is initialized. */ +#include <unistd.h> + /* Use macro instead of inline function to avoid including <stdio.h>. */ #define _startup_fatal(message) __libc_fatal ((message)) + +static inline uid_t +startup_getuid (void) +{ + return __getuid (); +} + +static inline uid_t +startup_geteuid (void) +{ + return __geteuid (); +} + +static inline gid_t +startup_getgid (void) +{ + return __getgid (); +} + +static inline gid_t +startup_getegid (void) +{ + return __getegid (); +} diff --git a/sysdeps/unix/sysv/linux/i386/startup.h b/sysdeps/unix/sysv/linux/i386/startup.h index 3eb4cc43a2..dee7a4f1d3 100644 --- a/sysdeps/unix/sysv/linux/i386/startup.h +++ b/sysdeps/unix/sysv/linux/i386/startup.h @@ -17,11 +17,12 @@ <https://www.gnu.org/licenses/>. */ #if BUILD_PIE_DEFAULT -# include <abort-instr.h> - /* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE. */ # define I386_USE_SYSENTER 0 +# include <sysdep.h> +# include <abort-instr.h> + __attribute__ ((__noreturn__)) static inline void _startup_fatal (const char *message __attribute__ ((unused))) @@ -31,6 +32,30 @@ _startup_fatal (const char *message __attribute__ ((unused))) ABORT_INSTRUCTION; __builtin_unreachable (); } + +static inline uid_t +startup_getuid (void) +{ + return (uid_t) INTERNAL_SYSCALL_CALL (getuid32); +} + +static inline uid_t +startup_geteuid (void) +{ + return (uid_t) INTERNAL_SYSCALL_CALL (geteuid32); +} + +static inline gid_t +startup_getgid (void) +{ + return (gid_t) INTERNAL_SYSCALL_CALL (getgid32); +} + +static inline gid_t +startup_getegid (void) +{ + return (gid_t) INTERNAL_SYSCALL_CALL (getegid32); +} #else # include_next <startup.h> #endif -- 2.29.2 [-- Attachment #4: 0001-libmvec-Add-extra-test-objs-to-test-extras.patch --] [-- Type: text/x-patch, Size: 1695 bytes --] From aa3d762e41610bbf5c016a219021f055b8b46337 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Thu, 14 Jan 2021 18:39:24 -0800 Subject: [PATCH 1/4] libmvec: Add extra-test-objs to test-extras Add extra-test-objs to test-extras so that they are compiled with -DMODULE_NAME=testsuite instead of -DMODULE_NAME=libc. --- sysdeps/x86_64/fpu/Makefile | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sysdeps/x86_64/fpu/Makefile b/sysdeps/x86_64/fpu/Makefile index a4ff2723a8..9a4bdd075c 100644 --- a/sysdeps/x86_64/fpu/Makefile +++ b/sysdeps/x86_64/fpu/Makefile @@ -31,6 +31,12 @@ libmvec-tests += double-vlen2 double-vlen4 double-vlen4-avx2 \ tests += test-double-libmvec-sincos test-double-libmvec-sincos-avx \ test-double-libmvec-sincos-avx2 test-float-libmvec-sincosf \ test-float-libmvec-sincosf-avx test-float-libmvec-sincosf-avx2 +test-extras += test-double-libmvec-sincos-avx-main \ + test-double-libmvec-sincos-avx2-main \ + test-double-libmvec-sincos-main \ + test-float-libmvec-sincosf-avx-main \ + test-float-libmvec-sincosf-avx2-main \ + test-float-libmvec-sincosf-main extra-test-objs += test-double-libmvec-sincos-avx-main.o \ test-double-libmvec-sincos-avx2-main.o \ test-double-libmvec-sincos-main.o \ @@ -66,6 +72,8 @@ ifeq (yes,$(config-cflags-avx512)) libmvec-tests += double-vlen8 float-vlen16 tests += test-double-libmvec-sincos-avx512 \ test-float-libmvec-sincosf-avx512 +test-extras += test-double-libmvec-sincos-avx512-main \ + test-float-libmvec-sincosf-avx512-main extra-test-objs += test-double-libmvec-sincos-avx512-main.o \ test-float-libmvec-sincosf-avx512-main.o -- 2.29.2 [-- Attachment #5: 0002-Make-all-symbols-used-before-and-by-_dl_relocate_sta.patch --] [-- Type: text/x-patch, Size: 3312 bytes --] From 52dd650d7b231fe20634afe3e261e67f33b66869 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Thu, 14 Jan 2021 16:40:43 -0800 Subject: [PATCH 2/4] Make all symbols used before and by _dl_relocate_static_pie hidden On i386, all calls to IFUNC functions must go through PLT and calls to hidden functions CANNOT go through PLT in PIE since EBX used in PIE PLT may not be set up for local calls to hidden IFUNC functions. Even if we can't make all libc symbols hidden for static PIE on i386, we must make all symbols used before and by _dl_relocate_static_pie hidden. In static PIE, set the default _dl_sysinfo in _dl_aux_init, instead of using the RELATIVE relocation to intialize it. --- elf/dl-reloc-static-pie.c | 1 + elf/dl-support.c | 16 +++++++++++++++- elf/dl-tunables.c | 3 +++ elf/enbl-secure.c | 3 +++ 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c index a8d964061e..cc34c2d2fe 100644 --- a/elf/dl-reloc-static-pie.c +++ b/elf/dl-reloc-static-pie.c @@ -17,6 +17,7 @@ <https://www.gnu.org/licenses/>. */ #if ENABLE_STATIC_PIE +#pragma GCC visibility push(hidden) #include <unistd.h> #include <ldsodefs.h> #include "dynamic-link.h" diff --git a/elf/dl-support.c b/elf/dl-support.c index 9d468d5a4b..4fbf33c982 100644 --- a/elf/dl-support.c +++ b/elf/dl-support.c @@ -19,6 +19,10 @@ /* This file defines some things that for the dynamic linker are defined in rtld.c and dl-sysdep.c in ways appropriate to bootstrap dynamic linking. */ +#include <string.h> +#if BUILD_PIE_DEFAULT +# pragma GCC visibility push(hidden) +#endif #include <errno.h> #include <libintl.h> #include <stdlib.h> @@ -194,7 +198,12 @@ struct dl_scope_free_list *_dl_scope_free_list; #ifdef NEED_DL_SYSINFO /* Needed for improved syscall handling on at least x86/Linux. */ -uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT; +uintptr_t _dl_sysinfo +/* NB: Avoid RELATIVE relocation in static PIE. */ +# ifndef BUILD_PIE_DEFAULT + = DL_SYSINFO_DEFAULT +# endif +; #endif #ifdef NEED_DL_SYSINFO_DSO /* Address of the ELF headers in the vsyscall page. */ @@ -232,6 +241,11 @@ _dl_aux_init (ElfW(auxv_t) *av) uid_t uid = 0; gid_t gid = 0; +#if defined NEED_DL_SYSINFO && BUILD_PIE_DEFAULT + /* NB: Avoid RELATIVE relocation in static PIE. */ + GL(dl_sysinfo) = DL_SYSINFO_DEFAULT; +#endif + _dl_auxv = av; for (; av->a_type != AT_NULL; ++av) switch (av->a_type) diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index e44476f204..060956902d 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -18,6 +18,9 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ +#if BUILD_PIE_DEFAULT +# pragma GCC visibility push(hidden) +#endif #include <startup.h> #include <stdint.h> #include <stdbool.h> diff --git a/elf/enbl-secure.c b/elf/enbl-secure.c index bc8c5e96d2..dba39a123e 100644 --- a/elf/enbl-secure.c +++ b/elf/enbl-secure.c @@ -19,6 +19,9 @@ /* This file is used in the static libc. For the shared library, dl-sysdep.c defines and initializes __libc_enable_secure. */ +#if BUILD_PIE_DEFAULT +# pragma GCC visibility push(hidden) +#endif #include <unistd.h> #include <libc-internal.h> -- 2.29.2 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-15 22:42 ` H.J. Lu @ 2021-01-16 0:41 ` H.J. Lu 2021-01-16 13:18 ` H.J. Lu 0 siblings, 1 reply; 34+ messages in thread From: H.J. Lu @ 2021-01-16 0:41 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Fri, Jan 15, 2021 at 2:42 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Fri, Jan 15, 2021 at 7:28 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Fri, Jan 15, 2021 at 6:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > The 01/15/2021 05:43, H.J. Lu wrote: > > > > On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > options: > > > > > > > > > > (1) fix extern hidden pie calls on i686 (by making > > > > > them the same as default vis pie calls so ifuncs > > > > > work). > > > > > > > > > > (2) annotate ifuncs (avoid hidden for them, ifuncs > > > > > cannot appear in early start code anyway because > > > > > of IRELATIVE): this can be difficult to maintain. > > > > > > > > > > (3) annotate early object accesses to be hidden so > > > > > RELATIVE relocs are avoided. (most targets want > > > > > all objects to be hidden, but this solves bug > > > > > 27072 without causing problems on i686) > > > > > > > > > > (4) make pie always use copy relocations on i686. > > > > > (and then no hidden annotation is needed, object > > > > > access is always local in pie). > > > ... > > > > > > --- a/elf/dl-reloc-static-pie.c > > > > > > +++ b/elf/dl-reloc-static-pie.c > > > > > > @@ -17,6 +17,7 @@ > > > > > > <https://www.gnu.org/licenses/>. */ > > > > > > > > > > > > #if ENABLE_STATIC_PIE > > > > > > +#pragma GCC visibility push(hidden) > > > > > > > > > > yes, this is option (3). you will also need it in _dl_aux_init > > > > > and __libc_init_secure and __tunables_init. > > > > > > > > I will try it. > > > > > > the naive way does not seem to work: > > > > > > _dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init, > > > the former needs hidden the latter does not and calls rawmemchr > > > which is ifunc on i686. > > > > > > i think the easiest fix is to move those two functions into > > > separate files. (ideally we would have a small set of files > > > that are involved in the start code before self relocation) > > > > > > now i realized that there is another option: > > > > > > (5) remove all ifuncs from i686 libc. > > > > > > i assume there are not many users who care about i686 performance. > > > > I don't know if this will work on i686. Since i386 doesn't have IP relative > > addressing, we can't remove all RELATIVE relocations. We need to > > call _dl_aux_init again after relocating PIE. I don't know what other symbols > > are affected. My current patches are on users/hjl/pr27072/master branch: > > > > https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master > > > > The problem is > > #ifdef NEED_DL_SYSINFO > /* Needed for improved syscall handling on at least x86/Linux. */ > uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT; > #endif > > We can initialize it in _dl_aux_init instead. > > I am testing this set of patches on top of yours on i686 and x86-64. > They worked and they passed build-many-glibcs.py. -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-16 0:41 ` H.J. Lu @ 2021-01-16 13:18 ` H.J. Lu 2021-01-18 16:22 ` Szabolcs Nagy 0 siblings, 1 reply; 34+ messages in thread From: H.J. Lu @ 2021-01-16 13:18 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Fri, Jan 15, 2021 at 4:41 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Fri, Jan 15, 2021 at 2:42 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Fri, Jan 15, 2021 at 7:28 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > On Fri, Jan 15, 2021 at 6:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > > > The 01/15/2021 05:43, H.J. Lu wrote: > > > > > On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > options: > > > > > > > > > > > > (1) fix extern hidden pie calls on i686 (by making > > > > > > them the same as default vis pie calls so ifuncs > > > > > > work). > > > > > > > > > > > > (2) annotate ifuncs (avoid hidden for them, ifuncs > > > > > > cannot appear in early start code anyway because > > > > > > of IRELATIVE): this can be difficult to maintain. > > > > > > > > > > > > (3) annotate early object accesses to be hidden so > > > > > > RELATIVE relocs are avoided. (most targets want > > > > > > all objects to be hidden, but this solves bug > > > > > > 27072 without causing problems on i686) > > > > > > > > > > > > (4) make pie always use copy relocations on i686. > > > > > > (and then no hidden annotation is needed, object > > > > > > access is always local in pie). > > > > ... > > > > > > > --- a/elf/dl-reloc-static-pie.c > > > > > > > +++ b/elf/dl-reloc-static-pie.c > > > > > > > @@ -17,6 +17,7 @@ > > > > > > > <https://www.gnu.org/licenses/>. */ > > > > > > > > > > > > > > #if ENABLE_STATIC_PIE > > > > > > > +#pragma GCC visibility push(hidden) > > > > > > > > > > > > yes, this is option (3). you will also need it in _dl_aux_init > > > > > > and __libc_init_secure and __tunables_init. > > > > > > > > > > I will try it. > > > > > > > > the naive way does not seem to work: > > > > > > > > _dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init, > > > > the former needs hidden the latter does not and calls rawmemchr > > > > which is ifunc on i686. > > > > > > > > i think the easiest fix is to move those two functions into > > > > separate files. (ideally we would have a small set of files > > > > that are involved in the start code before self relocation) > > > > > > > > now i realized that there is another option: > > > > > > > > (5) remove all ifuncs from i686 libc. > > > > > > > > i assume there are not many users who care about i686 performance. > > > > > > I don't know if this will work on i686. Since i386 doesn't have IP relative > > > addressing, we can't remove all RELATIVE relocations. We need to > > > call _dl_aux_init again after relocating PIE. I don't know what other symbols > > > are affected. My current patches are on users/hjl/pr27072/master branch: > > > > > > https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master > > > > > > > The problem is > > > > #ifdef NEED_DL_SYSINFO > > /* Needed for improved syscall handling on at least x86/Linux. */ > > uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT; > > #endif > > > > We can initialize it in _dl_aux_init instead. > > > > I am testing this set of patches on top of yours on i686 and x86-64. > > > > They worked and they passed build-many-glibcs.py. > I combined my patches, including 4 testcases, with yours in the right order here: https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE 2021-01-16 13:18 ` H.J. Lu @ 2021-01-18 16:22 ` Szabolcs Nagy 0 siblings, 0 replies; 34+ messages in thread From: Szabolcs Nagy @ 2021-01-18 16:22 UTC (permalink / raw) To: H.J. Lu; +Cc: GNU C Library The 01/16/2021 05:18, H.J. Lu wrote: > I combined my patches, including 4 testcases, with yours in the right > order here: > > https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master thanks. i moved things around a bit: if we have fine grain marking in files that need hidden then we can just use that instead of making everything hidden, this needed hidden in a few more files. i also moved the _dl_sysinfo bit into a separate patch. i'm sending a v4 of the series. ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 3/5] elf: Make the tunable struct definition internal only 2021-01-12 17:21 [PATCH v3 0/5] fix ifunc with static pie [BZ #27072] Szabolcs Nagy 2021-01-12 17:21 ` [PATCH v3 1/5] configure: Require PI_STATIC_AND_HIDDEN for static pie Szabolcs Nagy 2021-01-12 17:22 ` [PATCH v3 2/5] Make libc symbols hidden in static PIE Szabolcs Nagy @ 2021-01-12 17:22 ` Szabolcs Nagy 2021-01-13 17:38 ` Adhemerval Zanella 2021-01-12 17:22 ` [PATCH v3 4/5] elf: Avoid RELATIVE relocs in __tunables_init Szabolcs Nagy 2021-01-12 17:23 ` [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072] Szabolcs Nagy 4 siblings, 1 reply; 34+ messages in thread From: Szabolcs Nagy @ 2021-01-12 17:22 UTC (permalink / raw) To: libc-alpha The representation of the tunables including type information and the tunable list structure are only used in the implementation not in the tunables api that is exposed to usage within glibc. This patch moves the representation related definitions into the existing dl-tunable-types.h and uses that only for implementation. The tunable callback and related types are moved to dl-tunables.h because they are part of the tunables api. This reduces the details exposed in the tunables api so the internals are easier to change. --- elf/dl-tunable-types.h | 42 ++++++++++++++++++++++++++++++---------- elf/dl-tunables.h | 35 ++++++++------------------------- scripts/gen-tunables.awk | 4 +++- 3 files changed, 43 insertions(+), 38 deletions(-) diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h index 8f6a383dcc..05d4958e1c 100644 --- a/elf/dl-tunable-types.h +++ b/elf/dl-tunable-types.h @@ -1,4 +1,4 @@ -/* Tunable type information. +/* Internal representation of tunables. Copyright (C) 2016-2021 Free Software Foundation, Inc. This file is part of the GNU C Library. @@ -18,8 +18,14 @@ <https://www.gnu.org/licenses/>. */ #ifndef _TUNABLE_TYPES_H_ -# define _TUNABLE_TYPES_H_ +#define _TUNABLE_TYPES_H_ + +/* Note: This header is included in the generated dl-tunables-list.h and + only used internally in the tunables implementation in dl-tunables.c. */ + +#include <stdbool.h> #include <stddef.h> +#include <stdint.h> typedef enum { @@ -36,14 +42,6 @@ typedef struct int64_t max; } tunable_type_t; -typedef union -{ - int64_t numval; - 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 @@ -58,5 +56,29 @@ typedef enum TUNABLE_SECLEVEL_NONE = 2, } tunable_seclevel_t; +/* 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. */ + bool initialized; /* Flag to indicate that the tunable is + initialized. */ + tunable_seclevel_t security_level; /* Specify the security level for the + tunable with respect to AT_SECURE + programs. See description of + tunable_seclevel_t to see a + description of the values. + + Note that even if the tunable is + read, it may not get used by the + target module if the value is + considered unsafe. */ + /* Compatibility elements. */ + const char *env_alias; /* The compatibility environment + variable name. */ +}; + +typedef struct _tunable tunable_t; #endif diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h index 518342a300..1773c7e254 100644 --- a/elf/dl-tunables.h +++ b/elf/dl-tunables.h @@ -21,8 +21,6 @@ #ifndef _TUNABLES_H_ #define _TUNABLES_H_ -#include <stdbool.h> - #if !HAVE_TUNABLES static inline void __always_inline @@ -31,34 +29,17 @@ __tunables_init (char **unused __attribute__ ((unused))) /* This is optimized out if tunables are not enabled. */ } #else - +# include <stdbool.h> # include <stddef.h> -# include "dl-tunable-types.h" +# include <stdint.h> -/* A tunable. */ -struct _tunable +typedef union { - const char *name; /* 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 - initialized. */ - tunable_seclevel_t security_level; /* Specify the security level for the - tunable with respect to AT_SECURE - programs. See description of - tunable_seclevel_t to see a - description of the values. - - Note that even if the tunable is - read, it may not get used by the - target module if the value is - considered unsafe. */ - /* Compatibility elements. */ - const char *env_alias; /* The compatibility environment - variable name. */ -}; - -typedef struct _tunable tunable_t; + int64_t numval; + const char *strval; +} tunable_val_t; + +typedef void (*tunable_callback_t) (tunable_val_t *); /* Full name for a tunable is top_ns.tunable_ns.id. */ # define TUNABLE_NAME_S(top,ns,id) #top "." #ns "." #id diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk index 622199061a..cda12ef62e 100644 --- a/scripts/gen-tunables.awk +++ b/scripts/gen-tunables.awk @@ -156,8 +156,10 @@ END { } print "} tunable_id_t;\n" - # Finally, the tunable list. print "\n#ifdef TUNABLES_INTERNAL" + # Internal definitions. + print "# include \"dl-tunable-types.h\"" + # Finally, the tunable list. print "static tunable_t tunable_list[] attribute_relro = {" for (tnm in types) { split (tnm, indices, SUBSEP); -- 2.17.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 3/5] elf: Make the tunable struct definition internal only 2021-01-12 17:22 ` [PATCH v3 3/5] elf: Make the tunable struct definition internal only Szabolcs Nagy @ 2021-01-13 17:38 ` Adhemerval Zanella 0 siblings, 0 replies; 34+ messages in thread From: Adhemerval Zanella @ 2021-01-13 17:38 UTC (permalink / raw) To: libc-alpha, Szabolcs Nagy On 12/01/2021 14:22, Szabolcs Nagy via Libc-alpha wrote: > The representation of the tunables including type information and > the tunable list structure are only used in the implementation not > in the tunables api that is exposed to usage within glibc. > > This patch moves the representation related definitions into the > existing dl-tunable-types.h and uses that only for implementation. > > The tunable callback and related types are moved to dl-tunables.h > because they are part of the tunables api. > > This reduces the details exposed in the tunables api so the internals > are easier to change. LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > elf/dl-tunable-types.h | 42 ++++++++++++++++++++++++++++++---------- > elf/dl-tunables.h | 35 ++++++++------------------------- > scripts/gen-tunables.awk | 4 +++- > 3 files changed, 43 insertions(+), 38 deletions(-) > > diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h > index 8f6a383dcc..05d4958e1c 100644 > --- a/elf/dl-tunable-types.h > +++ b/elf/dl-tunable-types.h > @@ -1,4 +1,4 @@ > -/* Tunable type information. > +/* Internal representation of tunables. > > Copyright (C) 2016-2021 Free Software Foundation, Inc. > This file is part of the GNU C Library. > @@ -18,8 +18,14 @@ > <https://www.gnu.org/licenses/>. */ > > #ifndef _TUNABLE_TYPES_H_ > -# define _TUNABLE_TYPES_H_ > +#define _TUNABLE_TYPES_H_ > + > +/* Note: This header is included in the generated dl-tunables-list.h and > + only used internally in the tunables implementation in dl-tunables.c. */ > + > +#include <stdbool.h> > #include <stddef.h> > +#include <stdint.h> > > typedef enum > { > @@ -36,14 +42,6 @@ typedef struct > int64_t max; > } tunable_type_t; > > -typedef union > -{ > - int64_t numval; > - 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 > @@ -58,5 +56,29 @@ typedef enum > TUNABLE_SECLEVEL_NONE = 2, > } tunable_seclevel_t; > > +/* 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. */ > + bool initialized; /* Flag to indicate that the tunable is > + initialized. */ > + tunable_seclevel_t security_level; /* Specify the security level for the > + tunable with respect to AT_SECURE > + programs. See description of > + tunable_seclevel_t to see a > + description of the values. > + > + Note that even if the tunable is > + read, it may not get used by the > + target module if the value is > + considered unsafe. */ > + /* Compatibility elements. */ > + const char *env_alias; /* The compatibility environment > + variable name. */ > +}; > + > +typedef struct _tunable tunable_t; > > #endif Ok. > diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h > index 518342a300..1773c7e254 100644 > --- a/elf/dl-tunables.h > +++ b/elf/dl-tunables.h > @@ -21,8 +21,6 @@ > #ifndef _TUNABLES_H_ > #define _TUNABLES_H_ > > -#include <stdbool.h> > - > #if !HAVE_TUNABLES > static inline void > __always_inline > @@ -31,34 +29,17 @@ __tunables_init (char **unused __attribute__ ((unused))) > /* This is optimized out if tunables are not enabled. */ > } > #else > - > +# include <stdbool.h> > # include <stddef.h> > -# include "dl-tunable-types.h" > +# include <stdint.h> > > -/* A tunable. */ > -struct _tunable > +typedef union > { > - const char *name; /* 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 > - initialized. */ > - tunable_seclevel_t security_level; /* Specify the security level for the > - tunable with respect to AT_SECURE > - programs. See description of > - tunable_seclevel_t to see a > - description of the values. > - > - Note that even if the tunable is > - read, it may not get used by the > - target module if the value is > - considered unsafe. */ > - /* Compatibility elements. */ > - const char *env_alias; /* The compatibility environment > - variable name. */ > -}; > - > -typedef struct _tunable tunable_t; > + int64_t numval; > + const char *strval; > +} tunable_val_t; > + > +typedef void (*tunable_callback_t) (tunable_val_t *); > > /* Full name for a tunable is top_ns.tunable_ns.id. */ > # define TUNABLE_NAME_S(top,ns,id) #top "." #ns "." #id Ok. > diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk > index 622199061a..cda12ef62e 100644 > --- a/scripts/gen-tunables.awk > +++ b/scripts/gen-tunables.awk > @@ -156,8 +156,10 @@ END { > } > print "} tunable_id_t;\n" > > - # Finally, the tunable list. > print "\n#ifdef TUNABLES_INTERNAL" > + # Internal definitions. > + print "# include \"dl-tunable-types.h\"" > + # Finally, the tunable list. > print "static tunable_t tunable_list[] attribute_relro = {" > for (tnm in types) { > split (tnm, indices, SUBSEP); > Ok. ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 4/5] elf: Avoid RELATIVE relocs in __tunables_init 2021-01-12 17:21 [PATCH v3 0/5] fix ifunc with static pie [BZ #27072] Szabolcs Nagy ` (2 preceding siblings ...) 2021-01-12 17:22 ` [PATCH v3 3/5] elf: Make the tunable struct definition internal only Szabolcs Nagy @ 2021-01-12 17:22 ` Szabolcs Nagy 2021-01-13 17:42 ` Adhemerval Zanella 2021-01-12 17:23 ` [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072] Szabolcs Nagy 4 siblings, 1 reply; 34+ messages in thread From: Szabolcs Nagy @ 2021-01-12 17:22 UTC (permalink / raw) To: libc-alpha 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. --- 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. */ }; 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 = {" -- 2.17.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 4/5] elf: Avoid RELATIVE relocs in __tunables_init 2021-01-12 17:22 ` [PATCH v3 4/5] elf: Avoid RELATIVE relocs in __tunables_init Szabolcs Nagy @ 2021-01-13 17:42 ` Adhemerval Zanella 0 siblings, 0 replies; 34+ messages in thread From: Adhemerval Zanella @ 2021-01-13 17:42 UTC (permalink / raw) To: Szabolcs Nagy, libc-alpha 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. ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072] 2021-01-12 17:21 [PATCH v3 0/5] fix ifunc with static pie [BZ #27072] Szabolcs Nagy ` (3 preceding siblings ...) 2021-01-12 17:22 ` [PATCH v3 4/5] elf: Avoid RELATIVE relocs in __tunables_init Szabolcs Nagy @ 2021-01-12 17:23 ` Szabolcs Nagy 2021-01-12 22:55 ` H.J. Lu 4 siblings, 1 reply; 34+ messages in thread From: Szabolcs Nagy @ 2021-01-12 17:23 UTC (permalink / raw) To: libc-alpha IFUNC resolvers may depend on tunables and cpu feature setup so move static pie self relocation after those. It is hard to guarantee that the ealy startup code does not rely on relocations so this is a bit fragile. It would be more robust to handle RELATIVE relocs early and only IRELATIVE relocs later, but the current relocation processing code cannot do that. The early startup code before relocation processing includes _dl_aux_init (auxvec); __libc_init_secure (); __tunables_init (__environ); ARCH_INIT_CPU_FEATURES (); These are simple enough that RELATIVE relocs can be avoided. __ehdr_start may require RELATIVE relocation so it was moved later, fortunately ehdr and phdr are not used in the early code. --- csu/libc-start.c | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/csu/libc-start.c b/csu/libc-start.c index db859c3bed..fb64cdb2c9 100644 --- a/csu/libc-start.c +++ b/csu/libc-start.c @@ -142,8 +142,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), int result; #ifndef SHARED - _dl_relocate_static_pie (); - char **ev = &argv[argc + 1]; __environ = ev; @@ -165,24 +163,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), } # endif _dl_aux_init (auxvec); - if (GL(dl_phdr) == NULL) # endif - { - /* Starting from binutils-2.23, the linker will define the - magic symbol __ehdr_start to point to our own ELF header - if it is visible in a segment that also includes the phdrs. - So we can set up _dl_phdr and _dl_phnum even without any - information from auxv. */ - - extern const ElfW(Ehdr) __ehdr_start - __attribute__ ((weak, visibility ("hidden"))); - if (&__ehdr_start != NULL) - { - assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); - GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; - GL(dl_phnum) = __ehdr_start.e_phnum; - } - } /* Initialize very early so that tunables can use it. */ __libc_init_secure (); @@ -191,6 +172,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), ARCH_INIT_CPU_FEATURES (); + /* Do static pie self relocation after tunables and cpu features + are setup for ifunc resolvers. Before this point relocations + must be avoided. */ + _dl_relocate_static_pie (); + /* Perform IREL{,A} relocations. */ ARCH_SETUP_IREL (); @@ -202,6 +188,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), hwcap and platform fields available in the TCB. */ ARCH_APPLY_IREL (); +# ifdef HAVE_AUX_VECTOR + if (GL(dl_phdr) == NULL) +# endif + { + /* Starting from binutils-2.23, the linker will define the + magic symbol __ehdr_start to point to our own ELF header + if it is visible in a segment that also includes the phdrs. + So we can set up _dl_phdr and _dl_phnum even without any + information from auxv. */ + + extern const ElfW(Ehdr) __ehdr_start + __attribute__ ((weak, visibility ("hidden"))); + if (&__ehdr_start != NULL) + { + assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); + GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; + GL(dl_phnum) = __ehdr_start.e_phnum; + } + } + /* Set up the stack checker's canary. */ uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); # ifdef THREAD_SET_STACK_GUARD -- 2.17.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072] 2021-01-12 17:23 ` [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072] Szabolcs Nagy @ 2021-01-12 22:55 ` H.J. Lu 2021-01-14 15:49 ` H.J. Lu 0 siblings, 1 reply; 34+ messages in thread From: H.J. Lu @ 2021-01-12 22:55 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Tue, Jan 12, 2021 at 9:27 AM Szabolcs Nagy via Libc-alpha <libc-alpha@sourceware.org> wrote: > > IFUNC resolvers may depend on tunables and cpu feature setup so > move static pie self relocation after those. > > It is hard to guarantee that the ealy startup code does not rely > on relocations so this is a bit fragile. It would be more robust > to handle RELATIVE relocs early and only IRELATIVE relocs later, > but the current relocation processing code cannot do that. > > The early startup code before relocation processing includes > > _dl_aux_init (auxvec); > __libc_init_secure (); > __tunables_init (__environ); > ARCH_INIT_CPU_FEATURES (); > > These are simple enough that RELATIVE relocs can be avoided. > > __ehdr_start may require RELATIVE relocation so it was moved > later, fortunately ehdr and phdr are not used in the early code. > --- > csu/libc-start.c | 44 +++++++++++++++++++++++++------------------- > 1 file changed, 25 insertions(+), 19 deletions(-) > > diff --git a/csu/libc-start.c b/csu/libc-start.c > index db859c3bed..fb64cdb2c9 100644 > --- a/csu/libc-start.c > +++ b/csu/libc-start.c > @@ -142,8 +142,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > int result; > > #ifndef SHARED > - _dl_relocate_static_pie (); > - > char **ev = &argv[argc + 1]; > > __environ = ev; > @@ -165,24 +163,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > } > # endif > _dl_aux_init (auxvec); > - if (GL(dl_phdr) == NULL) > # endif > - { > - /* Starting from binutils-2.23, the linker will define the > - magic symbol __ehdr_start to point to our own ELF header > - if it is visible in a segment that also includes the phdrs. > - So we can set up _dl_phdr and _dl_phnum even without any > - information from auxv. */ > - > - extern const ElfW(Ehdr) __ehdr_start > - __attribute__ ((weak, visibility ("hidden"))); > - if (&__ehdr_start != NULL) > - { > - assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > - GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > - GL(dl_phnum) = __ehdr_start.e_phnum; > - } > - } > > /* Initialize very early so that tunables can use it. */ > __libc_init_secure (); > @@ -191,6 +172,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > ARCH_INIT_CPU_FEATURES (); > > + /* Do static pie self relocation after tunables and cpu features > + are setup for ifunc resolvers. Before this point relocations > + must be avoided. */ > + _dl_relocate_static_pie (); > + > /* Perform IREL{,A} relocations. */ > ARCH_SETUP_IREL (); > > @@ -202,6 +188,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > hwcap and platform fields available in the TCB. */ > ARCH_APPLY_IREL (); > > +# ifdef HAVE_AUX_VECTOR > + if (GL(dl_phdr) == NULL) > +# endif > + { > + /* Starting from binutils-2.23, the linker will define the > + magic symbol __ehdr_start to point to our own ELF header > + if it is visible in a segment that also includes the phdrs. > + So we can set up _dl_phdr and _dl_phnum even without any > + information from auxv. */ > + > + extern const ElfW(Ehdr) __ehdr_start > + __attribute__ ((weak, visibility ("hidden"))); > + if (&__ehdr_start != NULL) > + { > + assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > + GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > + GL(dl_phnum) = __ehdr_start.e_phnum; > + } > + } > + > /* Set up the stack checker's canary. */ > uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); > # ifdef THREAD_SET_STACK_GUARD > -- > 2.17.1 > LGTM. Thanks. -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072] 2021-01-12 22:55 ` H.J. Lu @ 2021-01-14 15:49 ` H.J. Lu 2021-01-14 15:52 ` H.J. Lu 0 siblings, 1 reply; 34+ messages in thread From: H.J. Lu @ 2021-01-14 15:49 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Tue, Jan 12, 2021 at 2:55 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Jan 12, 2021 at 9:27 AM Szabolcs Nagy via Libc-alpha > <libc-alpha@sourceware.org> wrote: > > > > IFUNC resolvers may depend on tunables and cpu feature setup so > > move static pie self relocation after those. > > > > It is hard to guarantee that the ealy startup code does not rely > > on relocations so this is a bit fragile. It would be more robust > > to handle RELATIVE relocs early and only IRELATIVE relocs later, > > but the current relocation processing code cannot do that. > > > > The early startup code before relocation processing includes > > > > _dl_aux_init (auxvec); > > __libc_init_secure (); > > __tunables_init (__environ); > > ARCH_INIT_CPU_FEATURES (); > > > > These are simple enough that RELATIVE relocs can be avoided. > > > > __ehdr_start may require RELATIVE relocation so it was moved > > later, fortunately ehdr and phdr are not used in the early code. > > --- > > csu/libc-start.c | 44 +++++++++++++++++++++++++------------------- > > 1 file changed, 25 insertions(+), 19 deletions(-) > > > > diff --git a/csu/libc-start.c b/csu/libc-start.c > > index db859c3bed..fb64cdb2c9 100644 > > --- a/csu/libc-start.c > > +++ b/csu/libc-start.c > > @@ -142,8 +142,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > int result; > > > > #ifndef SHARED > > - _dl_relocate_static_pie (); > > - > > char **ev = &argv[argc + 1]; > > > > __environ = ev; > > @@ -165,24 +163,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > } > > # endif > > _dl_aux_init (auxvec); > > - if (GL(dl_phdr) == NULL) > > # endif > > - { > > - /* Starting from binutils-2.23, the linker will define the > > - magic symbol __ehdr_start to point to our own ELF header > > - if it is visible in a segment that also includes the phdrs. > > - So we can set up _dl_phdr and _dl_phnum even without any > > - information from auxv. */ > > - > > - extern const ElfW(Ehdr) __ehdr_start > > - __attribute__ ((weak, visibility ("hidden"))); > > - if (&__ehdr_start != NULL) > > - { > > - assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > > - GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > > - GL(dl_phnum) = __ehdr_start.e_phnum; > > - } > > - } > > > > /* Initialize very early so that tunables can use it. */ > > __libc_init_secure (); > > @@ -191,6 +172,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > ARCH_INIT_CPU_FEATURES (); > > > > + /* Do static pie self relocation after tunables and cpu features > > + are setup for ifunc resolvers. Before this point relocations > > + must be avoided. */ > > + _dl_relocate_static_pie (); > > + > > /* Perform IREL{,A} relocations. */ > > ARCH_SETUP_IREL (); > > > > @@ -202,6 +188,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > hwcap and platform fields available in the TCB. */ > > ARCH_APPLY_IREL (); > > > > +# ifdef HAVE_AUX_VECTOR > > + if (GL(dl_phdr) == NULL) > > +# endif > > + { > > + /* Starting from binutils-2.23, the linker will define the > > + magic symbol __ehdr_start to point to our own ELF header > > + if it is visible in a segment that also includes the phdrs. > > + So we can set up _dl_phdr and _dl_phnum even without any > > + information from auxv. */ > > + > > + extern const ElfW(Ehdr) __ehdr_start > > + __attribute__ ((weak, visibility ("hidden"))); > > + if (&__ehdr_start != NULL) > > + { > > + assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > > + GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > > + GL(dl_phnum) = __ehdr_start.e_phnum; > > + } > > + } > > + > > /* Set up the stack checker's canary. */ > > uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); > > # ifdef THREAD_SET_STACK_GUARD > > -- > > 2.17.1 > > > > LGTM. > > Thanks. > Unfortunately, this failed on i686: (gdb) r Starting program: /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/elf/sln Program received signal SIGSEGV, Segmentation fault. 0xefec0550 in ?? () Breakpoint 1, __libc_start_main (main=0xf7f64760 <main>, argc=1, argv=0xffffc704, init=0xf7f670e0 <__libc_csu_init>, fini=0xf7f67190 <__libc_csu_fini>, rtld_fini=0x0, stack_end=0xffffc6fc) at ../csu/libc-start.c:145 145 char **ev = &argv[argc + 1]; (gdb) next 147 __environ = ev; (gdb) 151 __libc_stack_end = stack_end; (gdb) 160 while (*evp++ != NULL) (gdb) 165 _dl_aux_init (auxvec); (gdb) 169 __libc_init_secure (); (gdb) 171 __tunables_init (__environ); (gdb) 173 ARCH_INIT_CPU_FEATURES (); (gdb) 178 _dl_relocate_static_pie (); (gdb) 181 ARCH_SETUP_IREL (); (gdb) 184 ARCH_SETUP_TLS (); (gdb) 203 if (&__ehdr_start != NULL) (gdb) 212 uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); (gdb) 223 DL_SYSDEP_OSCHECK (__libc_fatal); (gdb) Program received signal SIGSEGV, Segmentation fault. 0xefec0550 in ?? () (gdb) -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072] 2021-01-14 15:49 ` H.J. Lu @ 2021-01-14 15:52 ` H.J. Lu 2021-01-14 16:01 ` H.J. Lu 0 siblings, 1 reply; 34+ messages in thread From: H.J. Lu @ 2021-01-14 15:52 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Thu, Jan 14, 2021 at 7:49 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Jan 12, 2021 at 2:55 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Tue, Jan 12, 2021 at 9:27 AM Szabolcs Nagy via Libc-alpha > > <libc-alpha@sourceware.org> wrote: > > > > > > IFUNC resolvers may depend on tunables and cpu feature setup so > > > move static pie self relocation after those. > > > > > > It is hard to guarantee that the ealy startup code does not rely > > > on relocations so this is a bit fragile. It would be more robust > > > to handle RELATIVE relocs early and only IRELATIVE relocs later, > > > but the current relocation processing code cannot do that. > > > > > > The early startup code before relocation processing includes > > > > > > _dl_aux_init (auxvec); > > > __libc_init_secure (); > > > __tunables_init (__environ); > > > ARCH_INIT_CPU_FEATURES (); > > > > > > These are simple enough that RELATIVE relocs can be avoided. > > > > > > __ehdr_start may require RELATIVE relocation so it was moved > > > later, fortunately ehdr and phdr are not used in the early code. > > > --- > > > csu/libc-start.c | 44 +++++++++++++++++++++++++------------------- > > > 1 file changed, 25 insertions(+), 19 deletions(-) > > > > > > diff --git a/csu/libc-start.c b/csu/libc-start.c > > > index db859c3bed..fb64cdb2c9 100644 > > > --- a/csu/libc-start.c > > > +++ b/csu/libc-start.c > > > @@ -142,8 +142,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > int result; > > > > > > #ifndef SHARED > > > - _dl_relocate_static_pie (); > > > - > > > char **ev = &argv[argc + 1]; > > > > > > __environ = ev; > > > @@ -165,24 +163,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > } > > > # endif > > > _dl_aux_init (auxvec); > > > - if (GL(dl_phdr) == NULL) > > > # endif > > > - { > > > - /* Starting from binutils-2.23, the linker will define the > > > - magic symbol __ehdr_start to point to our own ELF header > > > - if it is visible in a segment that also includes the phdrs. > > > - So we can set up _dl_phdr and _dl_phnum even without any > > > - information from auxv. */ > > > - > > > - extern const ElfW(Ehdr) __ehdr_start > > > - __attribute__ ((weak, visibility ("hidden"))); > > > - if (&__ehdr_start != NULL) > > > - { > > > - assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > > > - GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > > > - GL(dl_phnum) = __ehdr_start.e_phnum; > > > - } > > > - } > > > > > > /* Initialize very early so that tunables can use it. */ > > > __libc_init_secure (); > > > @@ -191,6 +172,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > > > ARCH_INIT_CPU_FEATURES (); > > > > > > + /* Do static pie self relocation after tunables and cpu features > > > + are setup for ifunc resolvers. Before this point relocations > > > + must be avoided. */ > > > + _dl_relocate_static_pie (); > > > + > > > /* Perform IREL{,A} relocations. */ > > > ARCH_SETUP_IREL (); > > > > > > @@ -202,6 +188,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > hwcap and platform fields available in the TCB. */ > > > ARCH_APPLY_IREL (); > > > > > > +# ifdef HAVE_AUX_VECTOR > > > + if (GL(dl_phdr) == NULL) > > > +# endif > > > + { > > > + /* Starting from binutils-2.23, the linker will define the > > > + magic symbol __ehdr_start to point to our own ELF header > > > + if it is visible in a segment that also includes the phdrs. > > > + So we can set up _dl_phdr and _dl_phnum even without any > > > + information from auxv. */ > > > + > > > + extern const ElfW(Ehdr) __ehdr_start > > > + __attribute__ ((weak, visibility ("hidden"))); > > > + if (&__ehdr_start != NULL) > > > + { > > > + assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > > > + GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > > > + GL(dl_phnum) = __ehdr_start.e_phnum; > > > + } > > > + } > > > + > > > /* Set up the stack checker's canary. */ > > > uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); > > > # ifdef THREAD_SET_STACK_GUARD > > > -- > > > 2.17.1 > > > > > > > LGTM. > > > > Thanks. > > > > Unfortunately, this failed on i686: > > (gdb) r > Starting program: > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/elf/sln > > Program received signal SIGSEGV, Segmentation fault. > 0xefec0550 in ?? () > > Breakpoint 1, __libc_start_main (main=0xf7f64760 <main>, argc=1, > argv=0xffffc704, init=0xf7f670e0 <__libc_csu_init>, > fini=0xf7f67190 <__libc_csu_fini>, rtld_fini=0x0, stack_end=0xffffc6fc) > at ../csu/libc-start.c:145 > 145 char **ev = &argv[argc + 1]; > (gdb) next > 147 __environ = ev; > (gdb) > 151 __libc_stack_end = stack_end; > (gdb) > 160 while (*evp++ != NULL) > (gdb) > 165 _dl_aux_init (auxvec); > (gdb) > 169 __libc_init_secure (); > (gdb) > 171 __tunables_init (__environ); > (gdb) > 173 ARCH_INIT_CPU_FEATURES (); > (gdb) > 178 _dl_relocate_static_pie (); > (gdb) > 181 ARCH_SETUP_IREL (); > (gdb) > 184 ARCH_SETUP_TLS (); > (gdb) > 203 if (&__ehdr_start != NULL) > (gdb) > 212 uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); > (gdb) > 223 DL_SYSDEP_OSCHECK (__libc_fatal); > (gdb) > > Program received signal SIGSEGV, Segmentation fault. > 0xefec0550 in ?? () > (gdb) > > -- > H.J. (gdb) si uname () at ../sysdeps/unix/syscall-template.S:120 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) (gdb) si 0xf7fba3a2 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) (gdb) si 0xf7fba3a6 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) (gdb) si 0xf7fba3ab 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) (gdb) si 0xefec0550 in ?? () (gdb) disass uname Dump of assembler code for function uname: 0xf7fba3a0 <+0>: mov %ebx,%edx 0xf7fba3a2 <+2>: mov 0x4(%esp),%ebx 0xf7fba3a6 <+6>: mov $0x7a,%eax 0xf7fba3ab <+11>: call *%gs:0x10 <<<<<<<<<<<< This may not be setup yet. 0xf7fba3b2 <+18>: mov %edx,%ebx 0xf7fba3b4 <+20>: cmp $0xfffff001,%eax 0xf7fba3b9 <+25>: jae 0xf7f9efd0 <__syscall_error> 0xf7fba3bf <+31>: ret End of assembler dump. (gdb) -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072] 2021-01-14 15:52 ` H.J. Lu @ 2021-01-14 16:01 ` H.J. Lu 2021-01-14 16:26 ` H.J. Lu 2021-01-14 17:05 ` Szabolcs Nagy 0 siblings, 2 replies; 34+ messages in thread From: H.J. Lu @ 2021-01-14 16:01 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Thu, Jan 14, 2021 at 7:52 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Thu, Jan 14, 2021 at 7:49 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Tue, Jan 12, 2021 at 2:55 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > On Tue, Jan 12, 2021 at 9:27 AM Szabolcs Nagy via Libc-alpha > > > <libc-alpha@sourceware.org> wrote: > > > > > > > > IFUNC resolvers may depend on tunables and cpu feature setup so > > > > move static pie self relocation after those. > > > > > > > > It is hard to guarantee that the ealy startup code does not rely > > > > on relocations so this is a bit fragile. It would be more robust > > > > to handle RELATIVE relocs early and only IRELATIVE relocs later, > > > > but the current relocation processing code cannot do that. > > > > > > > > The early startup code before relocation processing includes > > > > > > > > _dl_aux_init (auxvec); > > > > __libc_init_secure (); > > > > __tunables_init (__environ); > > > > ARCH_INIT_CPU_FEATURES (); > > > > > > > > These are simple enough that RELATIVE relocs can be avoided. > > > > > > > > __ehdr_start may require RELATIVE relocation so it was moved > > > > later, fortunately ehdr and phdr are not used in the early code. > > > > --- > > > > csu/libc-start.c | 44 +++++++++++++++++++++++++------------------- > > > > 1 file changed, 25 insertions(+), 19 deletions(-) > > > > > > > > diff --git a/csu/libc-start.c b/csu/libc-start.c > > > > index db859c3bed..fb64cdb2c9 100644 > > > > --- a/csu/libc-start.c > > > > +++ b/csu/libc-start.c > > > > @@ -142,8 +142,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > int result; > > > > > > > > #ifndef SHARED > > > > - _dl_relocate_static_pie (); > > > > - > > > > char **ev = &argv[argc + 1]; > > > > > > > > __environ = ev; > > > > @@ -165,24 +163,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > } > > > > # endif > > > > _dl_aux_init (auxvec); > > > > - if (GL(dl_phdr) == NULL) > > > > # endif > > > > - { > > > > - /* Starting from binutils-2.23, the linker will define the > > > > - magic symbol __ehdr_start to point to our own ELF header > > > > - if it is visible in a segment that also includes the phdrs. > > > > - So we can set up _dl_phdr and _dl_phnum even without any > > > > - information from auxv. */ > > > > - > > > > - extern const ElfW(Ehdr) __ehdr_start > > > > - __attribute__ ((weak, visibility ("hidden"))); > > > > - if (&__ehdr_start != NULL) > > > > - { > > > > - assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > > > > - GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > > > > - GL(dl_phnum) = __ehdr_start.e_phnum; > > > > - } > > > > - } > > > > > > > > /* Initialize very early so that tunables can use it. */ > > > > __libc_init_secure (); > > > > @@ -191,6 +172,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > > > > > ARCH_INIT_CPU_FEATURES (); > > > > > > > > + /* Do static pie self relocation after tunables and cpu features > > > > + are setup for ifunc resolvers. Before this point relocations > > > > + must be avoided. */ > > > > + _dl_relocate_static_pie (); > > > > + > > > > /* Perform IREL{,A} relocations. */ > > > > ARCH_SETUP_IREL (); > > > > > > > > @@ -202,6 +188,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > hwcap and platform fields available in the TCB. */ > > > > ARCH_APPLY_IREL (); > > > > > > > > +# ifdef HAVE_AUX_VECTOR > > > > + if (GL(dl_phdr) == NULL) > > > > +# endif > > > > + { > > > > + /* Starting from binutils-2.23, the linker will define the > > > > + magic symbol __ehdr_start to point to our own ELF header > > > > + if it is visible in a segment that also includes the phdrs. > > > > + So we can set up _dl_phdr and _dl_phnum even without any > > > > + information from auxv. */ > > > > + > > > > + extern const ElfW(Ehdr) __ehdr_start > > > > + __attribute__ ((weak, visibility ("hidden"))); > > > > + if (&__ehdr_start != NULL) > > > > + { > > > > + assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > > > > + GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > > > > + GL(dl_phnum) = __ehdr_start.e_phnum; > > > > + } > > > > + } > > > > + > > > > /* Set up the stack checker's canary. */ > > > > uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); > > > > # ifdef THREAD_SET_STACK_GUARD > > > > -- > > > > 2.17.1 > > > > > > > > > > LGTM. > > > > > > Thanks. > > > > > > > Unfortunately, this failed on i686: > > > > (gdb) r > > Starting program: > > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/elf/sln > > > > Program received signal SIGSEGV, Segmentation fault. > > 0xefec0550 in ?? () > > > > Breakpoint 1, __libc_start_main (main=0xf7f64760 <main>, argc=1, > > argv=0xffffc704, init=0xf7f670e0 <__libc_csu_init>, > > fini=0xf7f67190 <__libc_csu_fini>, rtld_fini=0x0, stack_end=0xffffc6fc) > > at ../csu/libc-start.c:145 > > 145 char **ev = &argv[argc + 1]; > > (gdb) next > > 147 __environ = ev; > > (gdb) > > 151 __libc_stack_end = stack_end; > > (gdb) > > 160 while (*evp++ != NULL) > > (gdb) > > 165 _dl_aux_init (auxvec); > > (gdb) > > 169 __libc_init_secure (); > > (gdb) > > 171 __tunables_init (__environ); > > (gdb) > > 173 ARCH_INIT_CPU_FEATURES (); > > (gdb) > > 178 _dl_relocate_static_pie (); > > (gdb) > > 181 ARCH_SETUP_IREL (); > > (gdb) > > 184 ARCH_SETUP_TLS (); > > (gdb) > > 203 if (&__ehdr_start != NULL) > > (gdb) > > 212 uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); > > (gdb) > > 223 DL_SYSDEP_OSCHECK (__libc_fatal); > > (gdb) > > > > Program received signal SIGSEGV, Segmentation fault. > > 0xefec0550 in ?? () > > (gdb) > > > > -- > > H.J. > > (gdb) si > uname () at ../sysdeps/unix/syscall-template.S:120 > 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > (gdb) si > 0xf7fba3a2 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > (gdb) si > 0xf7fba3a6 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > (gdb) si > 0xf7fba3ab 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > (gdb) si > 0xefec0550 in ?? () > (gdb) disass uname > Dump of assembler code for function uname: > 0xf7fba3a0 <+0>: mov %ebx,%edx > 0xf7fba3a2 <+2>: mov 0x4(%esp),%ebx > 0xf7fba3a6 <+6>: mov $0x7a,%eax > 0xf7fba3ab <+11>: call *%gs:0x10 <<<<<<<<<<<< This may not be setup yet. > 0xf7fba3b2 <+18>: mov %edx,%ebx > 0xf7fba3b4 <+20>: cmp $0xfffff001,%eax > 0xf7fba3b9 <+25>: jae 0xf7f9efd0 <__syscall_error> > 0xf7fba3bf <+31>: ret > End of assembler dump. > (gdb) > > > -- > H.J. GL(dl_sysinfo) points to the wrong address. This may affect all variables accessed in _dl_aux_init. -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072] 2021-01-14 16:01 ` H.J. Lu @ 2021-01-14 16:26 ` H.J. Lu 2021-01-14 17:19 ` Szabolcs Nagy 2021-01-14 17:05 ` Szabolcs Nagy 1 sibling, 1 reply; 34+ messages in thread From: H.J. Lu @ 2021-01-14 16:26 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: GNU C Library On Thu, Jan 14, 2021 at 8:01 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Thu, Jan 14, 2021 at 7:52 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Thu, Jan 14, 2021 at 7:49 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > On Tue, Jan 12, 2021 at 2:55 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > On Tue, Jan 12, 2021 at 9:27 AM Szabolcs Nagy via Libc-alpha > > > > <libc-alpha@sourceware.org> wrote: > > > > > > > > > > IFUNC resolvers may depend on tunables and cpu feature setup so > > > > > move static pie self relocation after those. > > > > > > > > > > It is hard to guarantee that the ealy startup code does not rely > > > > > on relocations so this is a bit fragile. It would be more robust > > > > > to handle RELATIVE relocs early and only IRELATIVE relocs later, > > > > > but the current relocation processing code cannot do that. > > > > > > > > > > The early startup code before relocation processing includes > > > > > > > > > > _dl_aux_init (auxvec); > > > > > __libc_init_secure (); > > > > > __tunables_init (__environ); > > > > > ARCH_INIT_CPU_FEATURES (); > > > > > > > > > > These are simple enough that RELATIVE relocs can be avoided. > > > > > > > > > > __ehdr_start may require RELATIVE relocation so it was moved > > > > > later, fortunately ehdr and phdr are not used in the early code. > > > > > --- > > > > > csu/libc-start.c | 44 +++++++++++++++++++++++++------------------- > > > > > 1 file changed, 25 insertions(+), 19 deletions(-) > > > > > > > > > > diff --git a/csu/libc-start.c b/csu/libc-start.c > > > > > index db859c3bed..fb64cdb2c9 100644 > > > > > --- a/csu/libc-start.c > > > > > +++ b/csu/libc-start.c > > > > > @@ -142,8 +142,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > > int result; > > > > > > > > > > #ifndef SHARED > > > > > - _dl_relocate_static_pie (); > > > > > - > > > > > char **ev = &argv[argc + 1]; > > > > > > > > > > __environ = ev; > > > > > @@ -165,24 +163,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > > } > > > > > # endif > > > > > _dl_aux_init (auxvec); > > > > > - if (GL(dl_phdr) == NULL) > > > > > # endif > > > > > - { > > > > > - /* Starting from binutils-2.23, the linker will define the > > > > > - magic symbol __ehdr_start to point to our own ELF header > > > > > - if it is visible in a segment that also includes the phdrs. > > > > > - So we can set up _dl_phdr and _dl_phnum even without any > > > > > - information from auxv. */ > > > > > - > > > > > - extern const ElfW(Ehdr) __ehdr_start > > > > > - __attribute__ ((weak, visibility ("hidden"))); > > > > > - if (&__ehdr_start != NULL) > > > > > - { > > > > > - assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > > > > > - GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > > > > > - GL(dl_phnum) = __ehdr_start.e_phnum; > > > > > - } > > > > > - } > > > > > > > > > > /* Initialize very early so that tunables can use it. */ > > > > > __libc_init_secure (); > > > > > @@ -191,6 +172,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > > > > > > > ARCH_INIT_CPU_FEATURES (); > > > > > > > > > > + /* Do static pie self relocation after tunables and cpu features > > > > > + are setup for ifunc resolvers. Before this point relocations > > > > > + must be avoided. */ > > > > > + _dl_relocate_static_pie (); > > > > > + > > > > > /* Perform IREL{,A} relocations. */ > > > > > ARCH_SETUP_IREL (); > > > > > > > > > > @@ -202,6 +188,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > > hwcap and platform fields available in the TCB. */ > > > > > ARCH_APPLY_IREL (); > > > > > > > > > > +# ifdef HAVE_AUX_VECTOR > > > > > + if (GL(dl_phdr) == NULL) > > > > > +# endif > > > > > + { > > > > > + /* Starting from binutils-2.23, the linker will define the > > > > > + magic symbol __ehdr_start to point to our own ELF header > > > > > + if it is visible in a segment that also includes the phdrs. > > > > > + So we can set up _dl_phdr and _dl_phnum even without any > > > > > + information from auxv. */ > > > > > + > > > > > + extern const ElfW(Ehdr) __ehdr_start > > > > > + __attribute__ ((weak, visibility ("hidden"))); > > > > > + if (&__ehdr_start != NULL) > > > > > + { > > > > > + assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > > > > > + GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > > > > > + GL(dl_phnum) = __ehdr_start.e_phnum; > > > > > + } > > > > > + } > > > > > + > > > > > /* Set up the stack checker's canary. */ > > > > > uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); > > > > > # ifdef THREAD_SET_STACK_GUARD > > > > > -- > > > > > 2.17.1 > > > > > > > > > > > > > LGTM. > > > > > > > > Thanks. > > > > > > > > > > Unfortunately, this failed on i686: > > > > > > (gdb) r > > > Starting program: > > > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/elf/sln > > > > > > Program received signal SIGSEGV, Segmentation fault. > > > 0xefec0550 in ?? () > > > > > > Breakpoint 1, __libc_start_main (main=0xf7f64760 <main>, argc=1, > > > argv=0xffffc704, init=0xf7f670e0 <__libc_csu_init>, > > > fini=0xf7f67190 <__libc_csu_fini>, rtld_fini=0x0, stack_end=0xffffc6fc) > > > at ../csu/libc-start.c:145 > > > 145 char **ev = &argv[argc + 1]; > > > (gdb) next > > > 147 __environ = ev; > > > (gdb) > > > 151 __libc_stack_end = stack_end; > > > (gdb) > > > 160 while (*evp++ != NULL) > > > (gdb) > > > 165 _dl_aux_init (auxvec); > > > (gdb) > > > 169 __libc_init_secure (); > > > (gdb) > > > 171 __tunables_init (__environ); > > > (gdb) > > > 173 ARCH_INIT_CPU_FEATURES (); > > > (gdb) > > > 178 _dl_relocate_static_pie (); > > > (gdb) > > > 181 ARCH_SETUP_IREL (); > > > (gdb) > > > 184 ARCH_SETUP_TLS (); > > > (gdb) > > > 203 if (&__ehdr_start != NULL) > > > (gdb) > > > 212 uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); > > > (gdb) > > > 223 DL_SYSDEP_OSCHECK (__libc_fatal); > > > (gdb) > > > > > > Program received signal SIGSEGV, Segmentation fault. > > > 0xefec0550 in ?? () > > > (gdb) > > > > > > -- > > > H.J. > > > > (gdb) si > > uname () at ../sysdeps/unix/syscall-template.S:120 > > 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > > (gdb) si > > 0xf7fba3a2 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > > (gdb) si > > 0xf7fba3a6 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > > (gdb) si > > 0xf7fba3ab 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > > (gdb) si > > 0xefec0550 in ?? () > > (gdb) disass uname > > Dump of assembler code for function uname: > > 0xf7fba3a0 <+0>: mov %ebx,%edx > > 0xf7fba3a2 <+2>: mov 0x4(%esp),%ebx > > 0xf7fba3a6 <+6>: mov $0x7a,%eax > > 0xf7fba3ab <+11>: call *%gs:0x10 <<<<<<<<<<<< This may not be setup yet. > > 0xf7fba3b2 <+18>: mov %edx,%ebx > > 0xf7fba3b4 <+20>: cmp $0xfffff001,%eax > > 0xf7fba3b9 <+25>: jae 0xf7f9efd0 <__syscall_error> > > 0xf7fba3bf <+31>: ret > > End of assembler dump. > > (gdb) > > > > > > -- > > H.J. > > GL(dl_sysinfo) points to the wrong address. This may affect all > variables accessed > in _dl_aux_init. > > -- > H.J. We need to make sure that there are no RELATIVE relocations before _dl_relocate_static_pie is called. The problems with i386 are 1. All calls to IFUNC functions must go through PLT. 2. Calls to hidden functions CANNOT go through PLT in PIE since EBX used in PIE PLT may not be set up for local calls. I think we should add a new attribute, attribute_hidden_ifunc which should be defined as 1. __attribute__ ((visibility ("default"))) if in PIE on i386 2. __attribute__ ((visibility ("hidden"))) else attribute_hidden_ifunc should be used on prototypes of all IFUNC functions. This is similar to NO_HIDDEN_EXTERN_FUNC_IN_PIE. -- H.J. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072] 2021-01-14 16:26 ` H.J. Lu @ 2021-01-14 17:19 ` Szabolcs Nagy 2021-01-14 17:59 ` Szabolcs Nagy 0 siblings, 1 reply; 34+ messages in thread From: Szabolcs Nagy @ 2021-01-14 17:19 UTC (permalink / raw) To: H.J. Lu; +Cc: GNU C Library The 01/14/2021 08:26, H.J. Lu wrote: > > We need to make sure that there are no RELATIVE relocations before > _dl_relocate_static_pie is called. The problems with i386 are > > 1. All calls to IFUNC functions must go through PLT. > 2. Calls to hidden functions CANNOT go through PLT in PIE since > EBX used in PIE PLT may not be set up for local calls. > > I think we should add a new attribute, attribute_hidden_ifunc > which should be defined as > > 1. __attribute__ ((visibility ("default"))) if in PIE on i386 > 2. __attribute__ ((visibility ("hidden"))) else > > attribute_hidden_ifunc should be used on prototypes of all IFUNC > functions. This is similar to NO_HIDDEN_EXTERN_FUNC_IN_PIE. so is it enough to declare ifuncs with such attribute? e.g. would it work if memcpy is default visibility in PIE libc.a but user code is static linking that with non-pie caller? do we have a way to track which functions may be defined as ifunc? should we do that manually? or add the attribute to every extern function declaration within the libc? ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072] 2021-01-14 17:19 ` Szabolcs Nagy @ 2021-01-14 17:59 ` Szabolcs Nagy 0 siblings, 0 replies; 34+ messages in thread From: Szabolcs Nagy @ 2021-01-14 17:59 UTC (permalink / raw) To: H.J. Lu, GNU C Library The 01/14/2021 17:19, Szabolcs Nagy via Libc-alpha wrote: > The 01/14/2021 08:26, H.J. Lu wrote: > > > > We need to make sure that there are no RELATIVE relocations before > > _dl_relocate_static_pie is called. The problems with i386 are > > > > 1. All calls to IFUNC functions must go through PLT. > > 2. Calls to hidden functions CANNOT go through PLT in PIE since > > EBX used in PIE PLT may not be set up for local calls. > > > > I think we should add a new attribute, attribute_hidden_ifunc > > which should be defined as > > > > 1. __attribute__ ((visibility ("default"))) if in PIE on i386 > > 2. __attribute__ ((visibility ("hidden"))) else > > > > attribute_hidden_ifunc should be used on prototypes of all IFUNC > > functions. This is similar to NO_HIDDEN_EXTERN_FUNC_IN_PIE. > > so is it enough to declare ifuncs with such attribute? > > e.g. would it work if memcpy is default visibility > in PIE libc.a but user code is static linking that > with non-pie caller? hm no, i think the only inconsistency that can happen is if an ifunc function is marked hidden in non-pie libc.a, but user calls it with default visibility, but that is not a problem i guess. maybe this works: target gives a list of ifunc declarations with explicit visibility attribute (default vis for i686 PIE) in a header that is pre-included very early everywhere so later declarations keep the explicit visibility instead of the one specified by the gcc pragma. (but i don't yet see how to have the right prototypes in an early declaration) > > do we have a way to track which functions may be > defined as ifunc? should we do that manually? or add > the attribute to every extern function declaration > within the libc? ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072] 2021-01-14 16:01 ` H.J. Lu 2021-01-14 16:26 ` H.J. Lu @ 2021-01-14 17:05 ` Szabolcs Nagy 1 sibling, 0 replies; 34+ messages in thread From: Szabolcs Nagy @ 2021-01-14 17:05 UTC (permalink / raw) To: H.J. Lu; +Cc: GNU C Library The 01/14/2021 08:01, H.J. Lu wrote: > On Thu, Jan 14, 2021 at 7:52 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Thu, Jan 14, 2021 at 7:49 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > On Tue, Jan 12, 2021 at 2:55 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > (gdb) > > > 223 DL_SYSDEP_OSCHECK (__libc_fatal); > > > (gdb) > > > > > > Program received signal SIGSEGV, Segmentation fault. > > > 0xefec0550 in ?? () > > > (gdb) > > > > > (gdb) si > > uname () at ../sysdeps/unix/syscall-template.S:120 > > 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > > (gdb) si > > 0xf7fba3a2 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > > (gdb) si > > 0xf7fba3a6 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > > (gdb) si > > 0xf7fba3ab 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) > > (gdb) si > > 0xefec0550 in ?? () > > (gdb) disass uname > > Dump of assembler code for function uname: > > 0xf7fba3a0 <+0>: mov %ebx,%edx > > 0xf7fba3a2 <+2>: mov 0x4(%esp),%ebx > > 0xf7fba3a6 <+6>: mov $0x7a,%eax > > 0xf7fba3ab <+11>: call *%gs:0x10 <<<<<<<<<<<< This may not be setup yet. > > 0xf7fba3b2 <+18>: mov %edx,%ebx > > 0xf7fba3b4 <+20>: cmp $0xfffff001,%eax > > 0xf7fba3b9 <+25>: jae 0xf7f9efd0 <__syscall_error> > > 0xf7fba3bf <+31>: ret > > End of assembler dump. > > (gdb) > > GL(dl_sysinfo) points to the wrong address. This may affect all > variables accessed > in _dl_aux_init. so is GL(dl_sysinfo_*) accessed via a GOT entry which require relocations or is this some other problem? it's not clear to me from this description, but the hidden visibility was added to avoid the GOT problem, without that it won't work, unless i686 has some magic to avoid GOT access for extern objects in PIE (which i thought it might have because of copy relocs). the solution is to ensure object symbols are hidden but functions aren't (so functions use the normal PIC call abi on i686 which is compatible with PIE ifunc) and hope that there are no extern function address computations in the early start code. but i don't see an easy way to do that (other than maintaining manual annotations either on object or function declarations). ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2021-01-18 16:22 UTC | newest] Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-12 17:21 [PATCH v3 0/5] fix ifunc with static pie [BZ #27072] Szabolcs Nagy 2021-01-12 17:21 ` [PATCH v3 1/5] configure: Require PI_STATIC_AND_HIDDEN for static pie Szabolcs Nagy 2021-01-12 18:38 ` Adhemerval Zanella 2021-01-12 17:22 ` [PATCH v3 2/5] Make libc symbols hidden in static PIE Szabolcs Nagy 2021-01-12 23:09 ` H.J. Lu 2021-01-13 0:02 ` H.J. Lu 2021-01-13 0:33 ` H.J. Lu 2021-01-13 1:19 ` H.J. Lu 2021-01-13 9:50 ` Szabolcs Nagy 2021-01-14 11:17 ` Szabolcs Nagy 2021-01-14 15:39 ` H.J. Lu 2021-01-15 3:36 ` H.J. Lu 2021-01-15 4:29 ` H.J. Lu 2021-01-15 11:25 ` Szabolcs Nagy 2021-01-15 13:43 ` H.J. Lu 2021-01-15 14:27 ` Szabolcs Nagy 2021-01-15 15:28 ` H.J. Lu 2021-01-15 22:42 ` H.J. Lu 2021-01-16 0:41 ` H.J. Lu 2021-01-16 13:18 ` H.J. Lu 2021-01-18 16:22 ` Szabolcs Nagy 2021-01-12 17:22 ` [PATCH v3 3/5] elf: Make the tunable struct definition internal only Szabolcs Nagy 2021-01-13 17:38 ` Adhemerval Zanella 2021-01-12 17:22 ` [PATCH v3 4/5] elf: Avoid RELATIVE relocs in __tunables_init Szabolcs Nagy 2021-01-13 17:42 ` Adhemerval Zanella 2021-01-12 17:23 ` [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072] Szabolcs Nagy 2021-01-12 22:55 ` H.J. Lu 2021-01-14 15:49 ` H.J. Lu 2021-01-14 15:52 ` H.J. Lu 2021-01-14 16:01 ` H.J. Lu 2021-01-14 16:26 ` H.J. Lu 2021-01-14 17:19 ` Szabolcs Nagy 2021-01-14 17:59 ` Szabolcs Nagy 2021-01-14 17:05 ` Szabolcs Nagy
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).