diff mbox series

[v6,12/20] elf: Do not fail for failed dlmopen on audit modules (BZ #28061)

Message ID 20211115183734.531155-13-adhemerval.zanella@linaro.org
State New
Headers show
Series Multiple rtld-audit fixes | expand

Commit Message

Adhemerval Zanella Nov. 15, 2021, 6:37 p.m. UTC 
The dl_main() sets the LM_ID_BASE to RT_ADD just before starting to
add load new shared objects.  The state is set to R_CONSISTENT just
after all objects are loaded.

However if a audit modules tries to dlmopen() an inexistent module,
the _dl_open() will assert that the namespace is in an inconsistent
state.

This is different than dlopen(), since first it will not use
LM_ID_BASE and second _dl_map_object_from_fd() is the sole responsible
to set and reset the r_state value.

So the assert() on _dl_open() can not really be seen if the state is
consistent, since _dt_main() resets it.  This patch removes the assert.

Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.
---
 elf/Makefile         |  5 ++++
 elf/dl-open.c        |  2 --
 elf/tst-audit20.c    | 25 +++++++++++++++++++
 elf/tst-auditmod20.c | 57 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 87 insertions(+), 2 deletions(-)
 create mode 100644 elf/tst-audit20.c
 create mode 100644 elf/tst-auditmod20.c

Comments

Florian Weimer Dec. 18, 2021, 6:59 p.m. UTC | #1 
* Adhemerval Zanella:

> The dl_main() sets the LM_ID_BASE to RT_ADD just before starting to
> add load new shared objects.  The state is set to R_CONSISTENT just
> after all objects are loaded.
>
> However if a audit modules tries to dlmopen() an inexistent module,
> the _dl_open() will assert that the namespace is in an inconsistent
> state.
>
> This is different than dlopen(), since first it will not use
> LM_ID_BASE and second _dl_map_object_from_fd() is the sole responsible
> to set and reset the r_state value.
>
> So the assert() on _dl_open() can not really be seen if the state is
> consistent, since _dt_main() resets it.  This patch removes the assert.

See previous comments about ().

> diff --git a/elf/dl-open.c b/elf/dl-open.c
> index e2f2e713e7..4f4d72e325 100644
> --- a/elf/dl-open.c
> +++ b/elf/dl-open.c
> @@ -914,8 +914,6 @@ no more namespaces available for dlmopen()"));
>  	     the flag here.  */
>  	}
>  
> -      assert (_dl_debug_update (args.nsid)->r_state == RT_CONSISTENT);
> -


_dl_debug_update has already been called for its side effect above, so
deleting the line is fine.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian
Adhemerval Zanella Dec. 20, 2021, 12:24 p.m. UTC | #2 
On 18/12/2021 15:59, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> The dl_main() sets the LM_ID_BASE to RT_ADD just before starting to
>> add load new shared objects.  The state is set to R_CONSISTENT just
>> after all objects are loaded.
>>
>> However if a audit modules tries to dlmopen() an inexistent module,
>> the _dl_open() will assert that the namespace is in an inconsistent
>> state.
>>
>> This is different than dlopen(), since first it will not use
>> LM_ID_BASE and second _dl_map_object_from_fd() is the sole responsible
>> to set and reset the r_state value.
>>
>> So the assert() on _dl_open() can not really be seen if the state is
>> consistent, since _dt_main() resets it.  This patch removes the assert.
> 
> See previous comments about ().

Ack, I removed all '()'.

> 
>> diff --git a/elf/dl-open.c b/elf/dl-open.c
>> index e2f2e713e7..4f4d72e325 100644
>> --- a/elf/dl-open.c
>> +++ b/elf/dl-open.c
>> @@ -914,8 +914,6 @@ no more namespaces available for dlmopen()"));
>>  	     the flag here.  */
>>  	}
>>  
>> -      assert (_dl_debug_update (args.nsid)->r_state == RT_CONSISTENT);
>> -
> 
> 
> _dl_debug_update has already been called for its side effect above, so
> deleting the line is fine.
> 
> Reviewed-by: Florian Weimer <fweimer@redhat.com>
> 
> Thanks,
> Florian
>
diff mbox series

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 19ec680144..4636a2743a 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -233,6 +233,7 @@  tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-audit18 \
 	 tst-audit19a \
 	 tst-audit19b \
+	 tst-audit20 \
 #	 reldep9
 tests-internal += loadtest unload unload2 circleload1 \
 	 neededtest neededtest2 neededtest3 neededtest4 \
@@ -378,6 +379,7 @@  modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-auditmod19a \
 		tst-auditmod19b \
 		tst-audit19bmod \
+		tst-auditmod20 \
 
 # Most modules build with _ISOMAC defined, but those filtered out
 # depend on internal headers.
@@ -1570,6 +1572,9 @@  $(objpfx)tst-audit19b.out: $(objpfx)tst-auditmod19b.so
 $(objpfx)tst-audit19b: $(objpfx)tst-audit19bmod.so
 tst-audit19b-ARGS = -- $(host-test-program-cmd)
 
+$(objpfx)tst-audit20.out: $(objpfx)tst-auditmod20.so
+tst-audit20-ENV = LD_AUDIT=$(objpfx)tst-auditmod20.so
+
 # tst-sonamemove links against an older implementation of the library.
 LDFLAGS-tst-sonamemove-linkmod1.so = \
   -Wl,--version-script=tst-sonamemove-linkmod1.map \
diff --git a/elf/dl-open.c b/elf/dl-open.c
index e2f2e713e7..4f4d72e325 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -914,8 +914,6 @@  no more namespaces available for dlmopen()"));
 	     the flag here.  */
 	}
 
-      assert (_dl_debug_update (args.nsid)->r_state == RT_CONSISTENT);
-
       /* Release the lock.  */
       __rtld_lock_unlock_recursive (GL(dl_load_lock));
 
diff --git a/elf/tst-audit20.c b/elf/tst-audit20.c
new file mode 100644
index 0000000000..6f39ccee86
--- /dev/null
+++ b/elf/tst-audit20.c
@@ -0,0 +1,25 @@ 
+/* Check dlopen failure on audit modules.
+   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/>.  */
+
+static int
+do_test (void)
+{
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-auditmod20.c b/elf/tst-auditmod20.c
new file mode 100644
index 0000000000..c57e50ee4e
--- /dev/null
+++ b/elf/tst-auditmod20.c
@@ -0,0 +1,57 @@ 
+/* Check dlopen failure on audit modules.
+   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 <dlfcn.h>
+#include <link.h>
+#include <stdlib.h>
+
+unsigned int
+la_version (unsigned int v)
+{
+  return LAV_CURRENT;
+}
+
+static void
+check (void)
+{
+  {
+    void *mod = dlopen ("nonexistent.so", RTLD_NOW);
+    if (mod != NULL)
+      abort ();
+  }
+
+  {
+    void *mod = dlmopen (LM_ID_BASE, "nonexistent.so", RTLD_NOW);
+    if (mod != NULL)
+      abort ();
+  }
+}
+
+void
+la_activity (uintptr_t *cookie, unsigned int flag)
+{
+  if (flag != LA_ACT_CONSISTENT)
+    return;
+  check ();
+}
+
+void
+la_preinit (uintptr_t *cookie)
+{
+  check ();
+}