[v6,12/20] elf: Do not fail for failed dlmopen on audit modules (BZ #28061)
Checks
Context |
Check |
Description |
dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
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
* 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
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
>
@@ -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 \
@@ -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));
new file mode 100644
@@ -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>
new file mode 100644
@@ -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 ();
+}