diff mbox series

elf: Make more functions available for binding during dlclose (bug 30425)

Message ID 87zg5w6g0i.fsf@oldenburg3.str.redhat.com
State New
Headers show
Series elf: Make more functions available for binding during dlclose (bug 30425) | expand

Commit Message

Florian Weimer May 22, 2023, 6:23 a.m. UTC 
Previously, after destructors for a DSO have been invoked, ld.so refused
to bind against that DSO in all cases.  Relax this restriction somewhat
if the referencing object is itself a DSO that is being unloaded.  This
assumes that the symbol reference is not going to be stored anywhere.

The situation in the test case can arise fairly easily with C++ and
objects that are built with different optimization levels and therefore
define different functions with vague linkage.

Tested on x86_64-linux-gnu.

---
 elf/Makefile                |  8 ++++++++
 elf/dl-lookup.c             |  9 +++++++--
 elf/tst-dlclose-lazy-mod1.c | 35 ++++++++++++++++++++++++++++++++
 elf/tst-dlclose-lazy-mod2.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
 elf/tst-dlclose-lazy.c      | 36 +++++++++++++++++++++++++++++++++
 5 files changed, 135 insertions(+), 2 deletions(-)


base-commit: e1b02c5ed4099a53db8f356303fc0ef88db8a131

Comments

Andreas Schwab May 22, 2023, 8:33 a.m. UTC | #1 
On Mai 22 2023, Florian Weimer via Libc-alpha wrote:

> diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c
> index 05f36a2507..c709ac4d33 100644
> --- a/elf/dl-lookup.c
> +++ b/elf/dl-lookup.c
> @@ -366,8 +366,13 @@ do_lookup_x (const char *undef_name, unsigned int new_hash,
>        if ((type_class & ELF_RTYPE_CLASS_COPY) && map->l_type == lt_executable)
>  	continue;
>  
> -      /* Do not look into objects which are going to be removed.  */
> -      if (map->l_removed)
> +      /* Do not look into objects which are going to be removed,
> +	 except when the referencing object itself is being removed.
> +	 The second part covers the situation when an object lazily
> +	 binds to another object while running its destructor, but the
> +	 destructor of the other object has already run, so that
> +	 dlclose has already set l_removed.  */

But if the destructor has been run already, it is no longer safe to call
functions from that object.
Florian Weimer May 22, 2023, 9:09 a.m. UTC | #2 
* Andreas Schwab:

> On Mai 22 2023, Florian Weimer via Libc-alpha wrote:
>
>> diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c
>> index 05f36a2507..c709ac4d33 100644
>> --- a/elf/dl-lookup.c
>> +++ b/elf/dl-lookup.c
>> @@ -366,8 +366,13 @@ do_lookup_x (const char *undef_name, unsigned int new_hash,
>>        if ((type_class & ELF_RTYPE_CLASS_COPY) && map->l_type == lt_executable)
>>  	continue;
>>  
>> -      /* Do not look into objects which are going to be removed.  */
>> -      if (map->l_removed)
>> +      /* Do not look into objects which are going to be removed,
>> +	 except when the referencing object itself is being removed.
>> +	 The second part covers the situation when an object lazily
>> +	 binds to another object while running its destructor, but the
>> +	 destructor of the other object has already run, so that
>> +	 dlclose has already set l_removed.  */
>
> But if the destructor has been run already, it is no longer safe to call
> functions from that object.

The library author can take steps to make this work.  Escaping from the
l_removed check is not so easy in general.

The change makes the lazy and BIND_NOW cases more similar (with
BIND_NOW, the call is successful, despite the already-called
destructor), and also aligns with the constructor scenario: we already
allow binding to symbols of objects whose constructors have not run.

Thanks,
Florian
Andreas Schwab May 22, 2023, 9:25 a.m. UTC | #3 
Please add that to the comment.

There is still an arbitrary distinction between objects being removed
and objects not being removed.  The reference may come from the latter
while being called from a destructor in the former.
diff mbox series

Patch

diff --git a/elf/Makefile b/elf/Makefile
index e262f3e6b1..be487b9cfb 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -394,6 +394,7 @@  tests += \
   tst-debug1 \
   tst-deep1 \
   tst-dl-is_dso \
+  tst-dlclose-lazy \
   tst-dlmodcount \
   tst-dlmopen-dlerror \
   tst-dlmopen-gethostbyname \
@@ -813,6 +814,8 @@  modules-names += \
   tst-dl_find_object-mod7 \
   tst-dl_find_object-mod8 \
   tst-dl_find_object-mod9 \
+  tst-dlclose-lazy-mod1 \
+  tst-dlclose-lazy-mod2 \
   tst-dlmopen-dlerror-mod \
   tst-dlmopen-gethostbyname-mod \
   tst-dlmopen-twice-mod1 \
@@ -2997,3 +3000,8 @@  $(objpfx)tst-sprof-basic.out: tst-sprof-basic.sh $(objpfx)tst-sprof-basic
 		 '$(run-program-env)' > $@; \
 	$(evaluate-test)
 generated += tst-sprof-mod.so.profile
+
+LDFLAGS-tst-dlclose-lazy-mod1.so = -Wl,-z,lazy,--no-as-needed
+$(objpfx)tst-dlclose-lazy-mod1.so: $(objpfx)tst-dlclose-lazy-mod2.so
+$(objpfx)tst-dlclose-lazy.out: \
+  $(objpfx)tst-dlclose-lazy-mod1.so $(objpfx)tst-dlclose-lazy-mod2.so
diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c
index 05f36a2507..c709ac4d33 100644
--- a/elf/dl-lookup.c
+++ b/elf/dl-lookup.c
@@ -366,8 +366,13 @@  do_lookup_x (const char *undef_name, unsigned int new_hash,
       if ((type_class & ELF_RTYPE_CLASS_COPY) && map->l_type == lt_executable)
 	continue;
 
-      /* Do not look into objects which are going to be removed.  */
-      if (map->l_removed)
+      /* Do not look into objects which are going to be removed,
+	 except when the referencing object itself is being removed.
+	 The second part covers the situation when an object lazily
+	 binds to another object while running its destructor, but the
+	 destructor of the other object has already run, so that
+	 dlclose has already set l_removed.  */
+      if (map->l_removed && !(undef_map != NULL && undef_map->l_removed))
 	continue;
 
       /* Print some debugging info if wanted.  */
diff --git a/elf/tst-dlclose-lazy-mod1.c b/elf/tst-dlclose-lazy-mod1.c
new file mode 100644
index 0000000000..5535a6b9e0
--- /dev/null
+++ b/elf/tst-dlclose-lazy-mod1.c
@@ -0,0 +1,35 @@ 
+/* Lazy binding during dlclose.  Directly loaded module.
+   Copyright (C) 2023 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/>.  */
+
+/* This function is called from exported_function below.  It is only
+   defined in this module.  */
+void __attribute__ ((weak))
+lazily_bound_exported_function (void)
+{
+}
+
+/* Called from tst-dlclose-lazy-mod2.so.  */
+void
+exported_function (int call_it)
+{
+  if (call_it)
+    /* This used crash if called during dlcose because after invoking
+       the destructor as part of dlclose, symbols were no longer
+       available for binding (bug 30425).  */
+    lazily_bound_exported_function ();
+}
diff --git a/elf/tst-dlclose-lazy-mod2.c b/elf/tst-dlclose-lazy-mod2.c
new file mode 100644
index 0000000000..767f69ffdb
--- /dev/null
+++ b/elf/tst-dlclose-lazy-mod2.c
@@ -0,0 +1,49 @@ 
+/* Lazy binding during dlclose.  Indirectly loaded module.
+   Copyright (C) 2023 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 <stdio.h>
+#include <stdlib.h>
+
+void
+exported_function (int ignored)
+{
+  /* This function is interposed from tst-dlclose-lazy-mod1.so and
+     thus never called.  */
+  abort ();
+}
+
+static void __attribute__ ((constructor))
+init (void)
+{
+  puts ("info: tst-dlclose-lazy-mod2.so constructor called");
+
+  /* Trigger lazy binding to the definition in
+     tst-dlclose-lazy-mod1.so, but not for
+     lazily_bound_exported_function in that module.  */
+  exported_function (0);
+}
+
+static void __attribute__ ((destructor))
+fini (void)
+{
+  puts ("info: tst-dlclose-lazy-mod2.so destructor called");
+
+  /* Trigger the lazily_bound_exported_function call in
+     exported_function in tst-dlclose-lazy-mod1.so.  */
+  exported_function (1);
+}
diff --git a/elf/tst-dlclose-lazy.c b/elf/tst-dlclose-lazy.c
new file mode 100644
index 0000000000..8207d134f3
--- /dev/null
+++ b/elf/tst-dlclose-lazy.c
@@ -0,0 +1,36 @@ 
+/* Test lazy binding during dlclose (bug 30425).
+   Copyright (C) 2023 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 <dlfcn.h>
+#include <support/xdlfcn.h>
+#include <support/check.h>
+
+int
+main (void)
+{
+  /* Load tst-dlclose-lazy-mod1.so, indirectly loading
+     tst-dlclose-lazy-mod2.so.  */
+  void *handle = xdlopen ("tst-dlclose-lazy-mod1.so", RTLD_GLOBAL | RTLD_LAZY);
+
+  /* Invoke the destructor of tst-dlclose-lazy-mod2.so, which calls
+     into tst-dlclose-lazy-mod1.so after its destructor has been
+     called.  */
+  xdlclose (handle);
+
+  return 0;
+}