[PATCHv2] Increase robustness of internal dlopen() by using RTLD_NOW [BZ #22766]

[Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com>]

Carlos O'Donell <carlos@redhat.com> writes:

> On 04/25/2018 07:42 AM, Tulio Magno Quites Machado Filho wrote:
>> Prevent random runtime crashes due to missing symbols caused by mixed
>> libnss_* versions.
>
> You are arguing that this should fail at the first call into the NSS
> service, which causes the initial dlopen? That is OK with me.

Yes.  I'm agreeing with Szabolcs' proposal [1].

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=22766

> An even further patch would be to load all NSS modules early, and that's
> something Florian and I have discussed. However, I'm not recommending
> you do that now, but I wanted to be clear about a direction that this
> code might take.

Ack.  Makes sense.

> We should cleanup more.
>
> sysdeps/gnu/unwind-resume.c:  handle = __libc_dlopen_mode (LIBGCC_S_SO, RTLD_NOW | __RTLD_DLOPEN);
> sysdeps/nptl/unwind-forcedunwind.c:  handle = __libc_dlopen_mode (LIBGCC_S_SO, RTLD_NOW | __RTLD_DLOPEN);

Done.

> Since we are now using RTLD_NOW:
>
> * Switch back to __libc_dlopen (effectively reverting part of 
>   Florian's 08c6e95234c, but leave the comments)

Done.

> * Add a big comment in dlfcn.h to explain why RTLD_NOW is needed:
>   * Same comment as in commit 08c6e95234c, plus NSS case.

Done.

--- 8< ---

Prevent random runtime crashes due to missing symbols caused by mixed
libnss_* versions.

2018-04-25  Tulio Magno Quites Machado Filho  <tuliom@linux.ibm.com>

	[BZ #22766]
	* include/dlfcn.h [__libc_dl_open]: Replace RTLD_LAZY with RTLD_NOW.
	* sysdeps/gnu/unwind-resume.c (__lib_gcc_s_init): Replace
	__libc_dlopen_mode() using RTLD_NOW with __libc_dlopen.
	* sysdeps/nptl/unwind-forcedunwind.c: Likewise.

Signed-off-by: Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com>
---
 include/dlfcn.h                    | 7 ++++++-
 sysdeps/gnu/unwind-resume.c        | 2 +-
 sysdeps/nptl/unwind-forcedunwind.c | 3 ++-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/dlfcn.h b/include/dlfcn.h
index 12ef913e19..558c9bfe17 100644
--- a/include/dlfcn.h
+++ b/include/dlfcn.h
@@ -31,8 +31,13 @@ extern char **__libc_argv attribute_hidden;
 
 /* Now define the internal interfaces.  */
 
+/* Use RTLD_NOW here because:
+   1. For consistency between __libgcc_s_init and pthread_cancel_init;
+   2. It allows libnss_* to provide a fallback code at dlopen() time in order
+      to prevent missing symbols caused by mixed glibc versions.  Using
+      RTLD_LAZY here could cause random failures at runtime.  */
 #define __libc_dlopen(name) \
-  __libc_dlopen_mode (name, RTLD_LAZY | __RTLD_DLOPEN)
+  __libc_dlopen_mode (name, RTLD_NOW | __RTLD_DLOPEN)
 extern void *__libc_dlopen_mode  (const char *__name, int __mode);
 extern void *__libc_dlsym   (void *__map, const char *__name);
 extern void *__libc_dlvsym (void *map, const char *name, const char *version);
diff --git a/sysdeps/gnu/unwind-resume.c b/sysdeps/gnu/unwind-resume.c
index 7f9a1bf2c7..72a04c7969 100644
--- a/sysdeps/gnu/unwind-resume.c
+++ b/sysdeps/gnu/unwind-resume.c
@@ -39,7 +39,7 @@ __libgcc_s_init (void)
      RTLD_NOW will rarely make a difference here because unwinding is
      already in progress, so libgcc_s.so has already been loaded if
      its unwinder is used.  */
-  handle = __libc_dlopen_mode (LIBGCC_S_SO, RTLD_NOW | __RTLD_DLOPEN);
+  handle = __libc_dlopen (LIBGCC_S_SO);
 
   if (handle == NULL
       || (resume = __libc_dlsym (handle, "_Unwind_Resume")) == NULL
diff --git a/sysdeps/nptl/unwind-forcedunwind.c b/sysdeps/nptl/unwind-forcedunwind.c
index 67b8e74b53..dbd8d66cda 100644
--- a/sysdeps/nptl/unwind-forcedunwind.c
+++ b/sysdeps/nptl/unwind-forcedunwind.c
@@ -49,7 +49,8 @@ pthread_cancel_init (void)
       return;
     }
 
-  handle = __libc_dlopen_mode (LIBGCC_S_SO, RTLD_NOW | __RTLD_DLOPEN);
+  /* __libgcc_s_init depends on RTLD_NOW being used here.  */
+  handle = __libc_dlopen (LIBGCC_S_SO);
 
   if (handle == NULL
       || (resume = __libc_dlsym (handle, "_Unwind_Resume")) == NULL
-- 
2.14.3