Re: [PATCH 1/2] Fix libX11 early init causing mmap layout changed

[Camm Maguire]

Greetings!  In followup to my earlier query, can you please detail what
happens when resource limits are set before execve as you mention in
your comment?  Isn't this still the case with this patch?  Rather the
patch appears to intend to reassert this setting immediately before
execve.

Take care,

> Greetings, and thanks so much again for these patches!

> Can you please give me a bit of explanation on this one?  What alters
> the resource limits between gcl_init_alloc 'before main' and main
> itself?  Would this patch work just as well if the saved resource limits
> were restored in the second invocation of gcl_init_alloc from main
> instead of immediately prior to execve in unrandomize.h?

> Take care,

Jinyang He <hejinyang@loongson.cn> writes:

> After libX.so release 1.8 with `--enable-thread-safety-constructor`,
> the libX.so calls `malloc` indirectly by calling XInitThreads() in
> init_array. The raw_ preempts `malloc` in EXE and calls gcl_init_alloc()
> before main. It set rlimit_stack before execve itself, and in linux
> kernel this action changes the mmap way from topdown to downtop
> (in many archs). Thus, saved the rlimit_stack if before_main and
> restore it if it need re-execve itself.
> ---
>  gcl/h/unrandomize.h | 16 ++++++++++++++++
>  gcl/o/alloc.c       | 10 ++++++++++
>  gcl/o/main.c        |  2 ++
>  3 files changed, 28 insertions(+)
>
> diff --git a/gcl/h/unrandomize.h b/gcl/h/unrandomize.h
> index 04b7c507e..9ca66d0a9 100644
> --- a/gcl/h/unrandomize.h
> +++ b/gcl/h/unrandomize.h
> @@ -5,6 +5,9 @@
>  #include <string.h>
>  #include <alloca.h>
>  #include <errno.h>
> +#if defined(__linux__) && defined(RLIMIT_STACK)
> +#include <sys/resource.h>
> +#endif
>  
>  {
>    errno=0;
> @@ -53,6 +56,19 @@
>       errno=0;
>  #ifdef HAVE_GCL_CLEANUP
>       gcl_cleanup(0);
> +#endif
> +#if defined(__linux__) && defined(RLIMIT_STACK)
> +     {
> +       extern struct rlimit rl_stack_saved;
> +       /* Reset the rlim_cur incase*/
> +       if (rl_stack_saved.rlim_cur != 0 && rl_stack_saved.rlim_max != 0)
> +         if(setrlimit(RLIMIT_STACK, &rl_stack_saved)) {
> +           printf("restore rlimit_stack failure %d\n",errno);
> +           exit(-1);
> +         }
> +       rl_stack_saved = (struct rlimit){0, 0};
> +       errno=0;
> +     }
>  #endif
>       execve(*a,a,n);
>       printf("execve failure %d\n",errno);
> diff --git a/gcl/o/alloc.c b/gcl/o/alloc.c
> index 8620df495..4d29ec860 100644
> --- a/gcl/o/alloc.c
> +++ b/gcl/o/alloc.c
> @@ -1189,6 +1189,10 @@ init_tm(enum type t, char *name, int elsize, int 
> nelts, int sgc,int distinct) {
>     simplicity.  set_maxpage is overloaded, and the positioning of its
>     call is too fragile.  20050115 CM*/
>  static int gcl_alloc_initialized;
> +#if defined(__linux__) && defined(RLIMIT_STACK)
> +extern int before_main;
> +struct rlimit rl_stack_saved;
> +#endif
>  
>  object malloc_list=Cnil;
>  
> @@ -1238,6 +1242,12 @@ gcl_init_alloc(void *cs_start) {
>      }
>  
>      massert(!getrlimit(RLIMIT_STACK, &rl));
> +#ifdef __linux__
> +    if (before_main)
> +      rl_stack_saved = rl;
> +    else
> +      rl_stack_saved = (struct rlimit){0, 0};
> +#endif
>      if (rl.rlim_cur!=RLIM_INFINITY && (rl.rlim_max == RLIM_INFINITY || 
> rl.rlim_max > rl.rlim_cur)) {
>        rl.rlim_cur = rl.rlim_max; /* == RLIM_INFINITY ? rl.rlim_max : 
> rl.rlim_max/64; */
>        massert(!setrlimit(RLIMIT_STACK,&rl));
> diff --git a/gcl/o/main.c b/gcl/o/main.c
> index 6621c3a16..be241d1af 100644
> --- a/gcl/o/main.c
> +++ b/gcl/o/main.c
> @@ -574,8 +574,10 @@ 
> DEFUN("KCL-SELF",object,fSkcl_self,SI,0,0,NONE,OO,OO,OO,OO,(void),"") {
>  
>  }
>  
> +int before_main=1;
>  int
>  main(int argc, char **argv, char **envp) {
> +  before_main=0;
>  
>    GET_FULL_PATH_SELF(kcl_self);
>    *argv=kcl_self;

-- 
Camm Maguire                                        camm@maguirefamily.org
==========================================================================
"The earth is but one country, and mankind its citizens."  --  Baha'u'llah