Message ID | eba12a40ed59a1547896c68e9f99fe10387648c9.1606908528.git.fweimer@redhat.com |
---|---|
State | New |
Headers | show |
Series | [v2,1/2] Replace __libc_multiple_libcs with __libc_initial flag | expand |
On 02/12/2020 08:30, Florian Weimer via Libc-alpha wrote: > Change sbrk to fail for !__libc_initial (in the generic > implementation). As a result, sbrk is (relatively) safe to use > for the __libc_initial case (from the main libc). It is therefore > no longer necessary to avoid using it in that case (or updating the > brk cache), and the __libc_initial flag does not need to be updated > as part of dlmopen or static dlopen. > > As before, direct brk system calls on Linux may lead to memory > corruption. > --- > v2: Switch to a flag with just two states, simplifying the code. > Add sbrk blocking for inner libcs. brk can't be changed yet > due to the pending refactoring. LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > csu/init-first.c | 10 +++------ > csu/libc-start.c | 13 +++++------ > elf/dl-open.c | 6 ----- > elf/dl-sysdep.c | 2 -- > elf/libc_early_init.c | 9 ++++++++ > include/libc-internal.h | 7 +++++- > misc/sbrk.c | 34 ++++++++++++++++++++--------- > sysdeps/mach/hurd/dl-sysdep.c | 2 -- > sysdeps/mach/hurd/i386/init-first.c | 10 +++++---- > 9 files changed, 53 insertions(+), 40 deletions(-) > > diff --git a/csu/init-first.c b/csu/init-first.c > index 47aaacdbd0..2115215668 100644 > --- a/csu/init-first.c > +++ b/csu/init-first.c > @@ -28,10 +28,6 @@ > > #include <ldsodefs.h> > > -/* Set nonzero if we have to be prepared for more than one libc being > - used in the process. Safe assumption if initializer never runs. */ > -int __libc_multiple_libcs attribute_hidden = 1; > - > /* Remember the command line argument and enviroment contents for > later calls of initializers for dynamic libraries. */ > int __libc_argc attribute_hidden; Ok. > @@ -50,16 +46,16 @@ _init_first (int argc, char **argv, char **envp) > { > #endif > > - __libc_multiple_libcs = &_dl_starting_up && !_dl_starting_up; > - > /* Make sure we don't initialize twice. */ > - if (!__libc_multiple_libcs) > +#ifdef SHARED > + if (__libc_initial) > { > /* Set the FPU control word to the proper default value if the > kernel would use a different value. */ > if (__fpu_control != GLRO(dl_fpu_control)) > __setfpucw (__fpu_control); > } > +#endif > > /* Save the command-line arguments. */ > __libc_argc = argc; Ok. > diff --git a/csu/libc-start.c b/csu/libc-start.c > index 2d4d2ed1f9..d330812c2d 100644 > --- a/csu/libc-start.c > +++ b/csu/libc-start.c > @@ -141,8 +141,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > /* Result of the 'main' function. */ > int result; > > - __libc_multiple_libcs = &_dl_starting_up && !_dl_starting_up; > - > #ifndef SHARED > _dl_relocate_static_pie (); > Ok. > @@ -213,12 +211,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > # endif > > # ifdef DL_SYSDEP_OSCHECK > - if (!__libc_multiple_libcs) > - { > - /* This needs to run to initiliaze _dl_osversion before TLS > - setup might check it. */ > - DL_SYSDEP_OSCHECK (__libc_fatal); > - } > + { > + /* This needs to run to initiliaze _dl_osversion before TLS > + setup might check it. */ > + DL_SYSDEP_OSCHECK (__libc_fatal); > + } Maybe remove the brackets here, DL_SYSDEP_OSCHECK already does it and if we need to reimplement to another system a static inline would be a better replacement. > # endif > > /* Initialize libpthread if linked in. */ Ok. > diff --git a/elf/dl-open.c b/elf/dl-open.c > index 8769e47051..6710ea04cd 100644 > --- a/elf/dl-open.c > +++ b/elf/dl-open.c > @@ -787,12 +787,6 @@ dl_open_worker (void *a) > if (mode & RTLD_GLOBAL) > add_to_global_update (new); > > -#ifndef SHARED > - /* We must be the static _dl_open in libc.a. A static program that > - has loaded a dynamic object now has competition. */ > - __libc_multiple_libcs = 1; > -#endif > - > /* Let the user know about the opencount. */ > if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES)) > _dl_debug_printf ("opening file=%s [%lu]; direct_opencount=%u\n\n", Ok. > diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c > index 854570821c..6cc4a76560 100644 > --- a/elf/dl-sysdep.c > +++ b/elf/dl-sysdep.c > @@ -58,8 +58,6 @@ ElfW(Addr) _dl_base_addr; > #endif > int __libc_enable_secure attribute_relro = 0; > rtld_hidden_data_def (__libc_enable_secure) > -int __libc_multiple_libcs = 0; /* Defining this here avoids the inclusion > - of init-first. */ > /* This variable contains the lowest stack address ever used. */ > void *__libc_stack_end attribute_relro = NULL; > rtld_hidden_data_def(__libc_stack_end) Ok. > diff --git a/elf/libc_early_init.c b/elf/libc_early_init.c > index 725ab2f811..28c6adca10 100644 > --- a/elf/libc_early_init.c > +++ b/elf/libc_early_init.c > @@ -18,8 +18,13 @@ > > #include <ctype.h> > #include <libc-early-init.h> > +#include <libc-internal.h> > #include <sys/single_threaded.h> > > +#ifdef SHARED > +_Bool __libc_initial; > +#endif > + > void > __libc_early_init (_Bool initial) > { > @@ -28,4 +33,8 @@ __libc_early_init (_Bool initial) > > /* Only the outer namespace is marked as single-threaded. */ > __libc_single_threaded = initial; > + > +#ifdef SHARED > + __libc_initial = initial; > +#endif > } Ok. > diff --git a/include/libc-internal.h b/include/libc-internal.h > index 915613c030..c1e74056ac 100644 > --- a/include/libc-internal.h > +++ b/include/libc-internal.h > @@ -47,6 +47,11 @@ extern void __init_misc (int, char **, char **) attribute_hidden; > extern __typeof (__profile_frequency) __profile_frequency attribute_hidden; > # endif > > -extern int __libc_multiple_libcs attribute_hidden; > +#ifdef SHARED > +/* True if this libc belongs to the initially loaded program (i.e., it > + is not for an audit module, not loaded via dlmopen, and not loaded > + via static dlopen either). */ > +extern _Bool __libc_initial attribute_hidden; > +#endif > > #endif /* _LIBC_INTERNAL */ Ok. > diff --git a/misc/sbrk.c b/misc/sbrk.c > index ba3322fba6..a6929d736d 100644 > --- a/misc/sbrk.c > +++ b/misc/sbrk.c > @@ -16,9 +16,10 @@ > <https://www.gnu.org/licenses/>. */ > > #include <errno.h> > +#include <libc-internal.h> > +#include <stdbool.h> > #include <stdint.h> > #include <unistd.h> > -#include <libc-internal.h> > > /* Defined in brk.c. */ > extern void *__curbrk; Ok. > @@ -30,21 +31,34 @@ extern int __brk (void *addr); > void * > __sbrk (intptr_t increment) > { > - void *oldbrk; > - > - /* If this is not part of the dynamic library or the library is used > - via dynamic loading in a statically linked program update > - __curbrk from the kernel's brk value. That way two separate > - instances of __brk and __sbrk can share the heap, returning > - interleaved pieces of it. */ > - if (__curbrk == NULL || __libc_multiple_libcs) > + /* Controls whether __brk (0) is called to read the brk value from > + the kernel. */ > + bool update_brk = __curbrk == NULL; > + > +#if defined (SHARED) && ! IS_IN (rtld) > + if (!__libc_initial) > + { > + if (increment != 0) > + { > + /* Do not allow changing the brk from an inner libc because > + it cannot be synchronized with the outer libc's brk. */ > + __set_errno (ENOMEM); > + return (void *) -1; > + } > + /* Querying the kernel's brk value from an inner namespace is > + fine. */ > + update_brk = true; > + } > +#endif > + > + if (update_brk) > if (__brk (0) < 0) /* Initialize the break. */ > return (void *) -1; > > if (increment == 0) > return __curbrk; > > - oldbrk = __curbrk; > + void *oldbrk = __curbrk; > if (increment > 0 > ? ((uintptr_t) oldbrk + (uintptr_t) increment < (uintptr_t) oldbrk) > : ((uintptr_t) oldbrk < (uintptr_t) -increment)) Ok. > diff --git a/sysdeps/mach/hurd/dl-sysdep.c b/sysdeps/mach/hurd/dl-sysdep.c > index 370495710e..a5169d85e7 100644 > --- a/sysdeps/mach/hurd/dl-sysdep.c > +++ b/sysdeps/mach/hurd/dl-sysdep.c > @@ -57,8 +57,6 @@ extern char **_environ; > > int __libc_enable_secure = 0; > rtld_hidden_data_def (__libc_enable_secure) > -int __libc_multiple_libcs = 0; /* Defining this here avoids the inclusion > - of init-first. */ > /* This variable contains the lowest stack address ever used. */ > void *__libc_stack_end = NULL; > rtld_hidden_data_def(__libc_stack_end) Ok. > diff --git a/sysdeps/mach/hurd/i386/init-first.c b/sysdeps/mach/hurd/i386/init-first.c > index 1827479f86..cbbc12204a 100644 > --- a/sysdeps/mach/hurd/i386/init-first.c > +++ b/sysdeps/mach/hurd/i386/init-first.c > @@ -30,6 +30,7 @@ > #include <ldsodefs.h> > #include <fpu_control.h> > #include <libc-diag.h> > +#include <libc-internal.h> > > extern void __mach_init (void); > extern void __init_misc (int, char **, char **); Ok. > @@ -40,7 +41,6 @@ unsigned long int __hurd_threadvar_stack_mask; > #ifndef SHARED > int __libc_enable_secure; > #endif > -int __libc_multiple_libcs attribute_hidden = 1; > > extern int __libc_argc attribute_hidden; > extern char **__libc_argv attribute_hidden; Ok. > @@ -56,13 +56,12 @@ DEFINE_HOOK (_hurd_preinit_hook, (void)); > static void > posixland_init (int argc, char **argv, char **envp) > { > - __libc_multiple_libcs = &_dl_starting_up && !_dl_starting_up; > - > /* Now we have relocations etc. we can start signals etc. */ > _hurd_libc_proc_init (argv); > > +#ifdef SHARED > /* Make sure we don't initialize twice. */ > - if (!__libc_multiple_libcs) > + if (__libc_initial) > { > /* Set the FPU control word to the proper default value. */ > __setfpucw (__fpu_control); > @@ -72,6 +71,9 @@ posixland_init (int argc, char **argv, char **envp) > /* Initialize data structures so the additional libc can do RPCs. */ > __mach_init (); > } > +#else /* !SHARED */ > + __setfpucw (__fpu_control); > +#endif > > /* Save the command-line arguments. */ > __libc_argc = argc; > Ok.
diff --git a/csu/init-first.c b/csu/init-first.c index 47aaacdbd0..2115215668 100644 --- a/csu/init-first.c +++ b/csu/init-first.c @@ -28,10 +28,6 @@ #include <ldsodefs.h> -/* Set nonzero if we have to be prepared for more than one libc being - used in the process. Safe assumption if initializer never runs. */ -int __libc_multiple_libcs attribute_hidden = 1; - /* Remember the command line argument and enviroment contents for later calls of initializers for dynamic libraries. */ int __libc_argc attribute_hidden; @@ -50,16 +46,16 @@ _init_first (int argc, char **argv, char **envp) { #endif - __libc_multiple_libcs = &_dl_starting_up && !_dl_starting_up; - /* Make sure we don't initialize twice. */ - if (!__libc_multiple_libcs) +#ifdef SHARED + if (__libc_initial) { /* Set the FPU control word to the proper default value if the kernel would use a different value. */ if (__fpu_control != GLRO(dl_fpu_control)) __setfpucw (__fpu_control); } +#endif /* Save the command-line arguments. */ __libc_argc = argc; diff --git a/csu/libc-start.c b/csu/libc-start.c index 2d4d2ed1f9..d330812c2d 100644 --- a/csu/libc-start.c +++ b/csu/libc-start.c @@ -141,8 +141,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), /* Result of the 'main' function. */ int result; - __libc_multiple_libcs = &_dl_starting_up && !_dl_starting_up; - #ifndef SHARED _dl_relocate_static_pie (); @@ -213,12 +211,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), # endif # ifdef DL_SYSDEP_OSCHECK - if (!__libc_multiple_libcs) - { - /* This needs to run to initiliaze _dl_osversion before TLS - setup might check it. */ - DL_SYSDEP_OSCHECK (__libc_fatal); - } + { + /* This needs to run to initiliaze _dl_osversion before TLS + setup might check it. */ + DL_SYSDEP_OSCHECK (__libc_fatal); + } # endif /* Initialize libpthread if linked in. */ diff --git a/elf/dl-open.c b/elf/dl-open.c index 8769e47051..6710ea04cd 100644 --- a/elf/dl-open.c +++ b/elf/dl-open.c @@ -787,12 +787,6 @@ dl_open_worker (void *a) if (mode & RTLD_GLOBAL) add_to_global_update (new); -#ifndef SHARED - /* We must be the static _dl_open in libc.a. A static program that - has loaded a dynamic object now has competition. */ - __libc_multiple_libcs = 1; -#endif - /* Let the user know about the opencount. */ if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES)) _dl_debug_printf ("opening file=%s [%lu]; direct_opencount=%u\n\n", diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c index 854570821c..6cc4a76560 100644 --- a/elf/dl-sysdep.c +++ b/elf/dl-sysdep.c @@ -58,8 +58,6 @@ ElfW(Addr) _dl_base_addr; #endif int __libc_enable_secure attribute_relro = 0; rtld_hidden_data_def (__libc_enable_secure) -int __libc_multiple_libcs = 0; /* Defining this here avoids the inclusion - of init-first. */ /* This variable contains the lowest stack address ever used. */ void *__libc_stack_end attribute_relro = NULL; rtld_hidden_data_def(__libc_stack_end) diff --git a/elf/libc_early_init.c b/elf/libc_early_init.c index 725ab2f811..28c6adca10 100644 --- a/elf/libc_early_init.c +++ b/elf/libc_early_init.c @@ -18,8 +18,13 @@ #include <ctype.h> #include <libc-early-init.h> +#include <libc-internal.h> #include <sys/single_threaded.h> +#ifdef SHARED +_Bool __libc_initial; +#endif + void __libc_early_init (_Bool initial) { @@ -28,4 +33,8 @@ __libc_early_init (_Bool initial) /* Only the outer namespace is marked as single-threaded. */ __libc_single_threaded = initial; + +#ifdef SHARED + __libc_initial = initial; +#endif } diff --git a/include/libc-internal.h b/include/libc-internal.h index 915613c030..c1e74056ac 100644 --- a/include/libc-internal.h +++ b/include/libc-internal.h @@ -47,6 +47,11 @@ extern void __init_misc (int, char **, char **) attribute_hidden; extern __typeof (__profile_frequency) __profile_frequency attribute_hidden; # endif -extern int __libc_multiple_libcs attribute_hidden; +#ifdef SHARED +/* True if this libc belongs to the initially loaded program (i.e., it + is not for an audit module, not loaded via dlmopen, and not loaded + via static dlopen either). */ +extern _Bool __libc_initial attribute_hidden; +#endif #endif /* _LIBC_INTERNAL */ diff --git a/misc/sbrk.c b/misc/sbrk.c index ba3322fba6..a6929d736d 100644 --- a/misc/sbrk.c +++ b/misc/sbrk.c @@ -16,9 +16,10 @@ <https://www.gnu.org/licenses/>. */ #include <errno.h> +#include <libc-internal.h> +#include <stdbool.h> #include <stdint.h> #include <unistd.h> -#include <libc-internal.h> /* Defined in brk.c. */ extern void *__curbrk; @@ -30,21 +31,34 @@ extern int __brk (void *addr); void * __sbrk (intptr_t increment) { - void *oldbrk; - - /* If this is not part of the dynamic library or the library is used - via dynamic loading in a statically linked program update - __curbrk from the kernel's brk value. That way two separate - instances of __brk and __sbrk can share the heap, returning - interleaved pieces of it. */ - if (__curbrk == NULL || __libc_multiple_libcs) + /* Controls whether __brk (0) is called to read the brk value from + the kernel. */ + bool update_brk = __curbrk == NULL; + +#if defined (SHARED) && ! IS_IN (rtld) + if (!__libc_initial) + { + if (increment != 0) + { + /* Do not allow changing the brk from an inner libc because + it cannot be synchronized with the outer libc's brk. */ + __set_errno (ENOMEM); + return (void *) -1; + } + /* Querying the kernel's brk value from an inner namespace is + fine. */ + update_brk = true; + } +#endif + + if (update_brk) if (__brk (0) < 0) /* Initialize the break. */ return (void *) -1; if (increment == 0) return __curbrk; - oldbrk = __curbrk; + void *oldbrk = __curbrk; if (increment > 0 ? ((uintptr_t) oldbrk + (uintptr_t) increment < (uintptr_t) oldbrk) : ((uintptr_t) oldbrk < (uintptr_t) -increment)) diff --git a/sysdeps/mach/hurd/dl-sysdep.c b/sysdeps/mach/hurd/dl-sysdep.c index 370495710e..a5169d85e7 100644 --- a/sysdeps/mach/hurd/dl-sysdep.c +++ b/sysdeps/mach/hurd/dl-sysdep.c @@ -57,8 +57,6 @@ extern char **_environ; int __libc_enable_secure = 0; rtld_hidden_data_def (__libc_enable_secure) -int __libc_multiple_libcs = 0; /* Defining this here avoids the inclusion - of init-first. */ /* This variable contains the lowest stack address ever used. */ void *__libc_stack_end = NULL; rtld_hidden_data_def(__libc_stack_end) diff --git a/sysdeps/mach/hurd/i386/init-first.c b/sysdeps/mach/hurd/i386/init-first.c index 1827479f86..cbbc12204a 100644 --- a/sysdeps/mach/hurd/i386/init-first.c +++ b/sysdeps/mach/hurd/i386/init-first.c @@ -30,6 +30,7 @@ #include <ldsodefs.h> #include <fpu_control.h> #include <libc-diag.h> +#include <libc-internal.h> extern void __mach_init (void); extern void __init_misc (int, char **, char **); @@ -40,7 +41,6 @@ unsigned long int __hurd_threadvar_stack_mask; #ifndef SHARED int __libc_enable_secure; #endif -int __libc_multiple_libcs attribute_hidden = 1; extern int __libc_argc attribute_hidden; extern char **__libc_argv attribute_hidden; @@ -56,13 +56,12 @@ DEFINE_HOOK (_hurd_preinit_hook, (void)); static void posixland_init (int argc, char **argv, char **envp) { - __libc_multiple_libcs = &_dl_starting_up && !_dl_starting_up; - /* Now we have relocations etc. we can start signals etc. */ _hurd_libc_proc_init (argv); +#ifdef SHARED /* Make sure we don't initialize twice. */ - if (!__libc_multiple_libcs) + if (__libc_initial) { /* Set the FPU control word to the proper default value. */ __setfpucw (__fpu_control); @@ -72,6 +71,9 @@ posixland_init (int argc, char **argv, char **envp) /* Initialize data structures so the additional libc can do RPCs. */ __mach_init (); } +#else /* !SHARED */ + __setfpucw (__fpu_control); +#endif /* Save the command-line arguments. */ __libc_argc = argc;