[v6,02/20] elf: Add _dl_audit_objopen
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
It consolidates the code required to call la_objopen() audit callback.
Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.
---
elf/Makefile | 2 +-
elf/dl-audit.c | 42 ++++++++++++++++++++++++++++++++++++++
elf/dl-load.c | 17 +--------------
elf/rtld.c | 23 ++-------------------
sysdeps/generic/ldsodefs.h | 5 +++++
5 files changed, 51 insertions(+), 38 deletions(-)
create mode 100644 elf/dl-audit.c
Comments
* Adhemerval Zanella:
> diff --git a/elf/Makefile b/elf/Makefile
> index 8f3e3a3602..80ed31edbe 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -36,7 +36,7 @@ dl-routines = $(addprefix dl-,load lookup object reloc deps \
> exception sort-maps lookup-direct \
> call-libc-early-init write \
> thread_gscope_wait tls_init_tp \
> - debug-symbols minimal-malloc)
> + debug-symbols minimal-malloc audit)
You can drop the #ifdef SHARED if you add this to rtld-routines.
> +/* Call the la_objopen() from the audit modules for the link_map L on the
> + namespace identification NSID. If CHECK_AUDIT is set it will also check
> + if main mapping of the namespace is a audit modules. */
> +void _dl_audit_objopen (struct link_map *l, Lmid_t nsid, bool check_audit);
No () after function name (not sure?)
Should check_audit be named require_auditing? That would align better
with the l_auditing check, I think.
Rest looks okay.
Thanks,
Florian
* Florian Weimer:
> * Adhemerval Zanella:
>
>> diff --git a/elf/Makefile b/elf/Makefile
>> index 8f3e3a3602..80ed31edbe 100644
>> --- a/elf/Makefile
>> +++ b/elf/Makefile
>> @@ -36,7 +36,7 @@ dl-routines = $(addprefix dl-,load lookup object reloc deps \
>> exception sort-maps lookup-direct \
>> call-libc-early-init write \
>> thread_gscope_wait tls_init_tp \
>> - debug-symbols minimal-malloc)
>> + debug-symbols minimal-malloc audit)
>
> You can drop the #ifdef SHARED if you add this to rtld-routines.
>
>> +/* Call the la_objopen() from the audit modules for the link_map L on the
>> + namespace identification NSID. If CHECK_AUDIT is set it will also check
>> + if main mapping of the namespace is a audit modules. */
>> +void _dl_audit_objopen (struct link_map *l, Lmid_t nsid, bool check_audit);
>
> No () after function name (not sure?)
>
> Should check_audit be named require_auditing? That would align better
> with the l_auditing check, I think.
Hmm. require_auditing is also not quite right. What about
skip_auditing?
And the comment should say which namespace (there's nsid and l->l_ns).
Maybe the check should be moved into the caller because
notify_audit_modules_of_loaded_object does not need it?
Thanks,
Florian
On 10/12/2021 09:48, Florian Weimer wrote:
> * Florian Weimer:
>
>> * Adhemerval Zanella:
>>
>>> diff --git a/elf/Makefile b/elf/Makefile
>>> index 8f3e3a3602..80ed31edbe 100644
>>> --- a/elf/Makefile
>>> +++ b/elf/Makefile
>>> @@ -36,7 +36,7 @@ dl-routines = $(addprefix dl-,load lookup object reloc deps \
>>> exception sort-maps lookup-direct \
>>> call-libc-early-init write \
>>> thread_gscope_wait tls_init_tp \
>>> - debug-symbols minimal-malloc)
>>> + debug-symbols minimal-malloc audit)
>>
>> You can drop the #ifdef SHARED if you add this to rtld-routines.
Ack, I will change it.
>>
>>> +/* Call the la_objopen() from the audit modules for the link_map L on the
>>> + namespace identification NSID. If CHECK_AUDIT is set it will also check
>>> + if main mapping of the namespace is a audit modules. */
>>> +void _dl_audit_objopen (struct link_map *l, Lmid_t nsid, bool check_audit);
>>
>> No () after function name (not sure?)
Ok, I will remove all the () on function descriptions.
>>
>> Should check_audit be named require_auditing? That would align better
>> with the l_auditing check, I think.
>
> Hmm. require_auditing is also not quite right. What about
> skip_auditing?
>
> And the comment should say which namespace (there's nsid and l->l_ns).
>
> Maybe the check should be moved into the caller because
> notify_audit_modules_of_loaded_object does not need it?
Yeah, this make more sense.
On 10/12/2021 10:45, Adhemerval Zanella wrote:
>
>
> On 10/12/2021 09:48, Florian Weimer wrote:
>> * Florian Weimer:
>>
>>> * Adhemerval Zanella:
>>>
>>>> diff --git a/elf/Makefile b/elf/Makefile
>>>> index 8f3e3a3602..80ed31edbe 100644
>>>> --- a/elf/Makefile
>>>> +++ b/elf/Makefile
>>>> @@ -36,7 +36,7 @@ dl-routines = $(addprefix dl-,load lookup object reloc deps \
>>>> exception sort-maps lookup-direct \
>>>> call-libc-early-init write \
>>>> thread_gscope_wait tls_init_tp \
>>>> - debug-symbols minimal-malloc)
>>>> + debug-symbols minimal-malloc audit)
>>>
>>> You can drop the #ifdef SHARED if you add this to rtld-routines.
>
> Ack, I will change it.
I recalled why I addon dl-routines: _dl_audit_pltexit is currently required
for static build as well. I can refactor it, but I think it simpler to
added the SHARED instead.
>
>>>
>>>> +/* Call the la_objopen() from the audit modules for the link_map L on the
>>>> + namespace identification NSID. If CHECK_AUDIT is set it will also check
>>>> + if main mapping of the namespace is a audit modules. */
>>>> +void _dl_audit_objopen (struct link_map *l, Lmid_t nsid, bool check_audit);
>>>
>>> No () after function name (not sure?)
>
> Ok, I will remove all the () on function descriptions.
>
>>>
>>> Should check_audit be named require_auditing? That would align better
>>> with the l_auditing check, I think.
>>
>> Hmm. require_auditing is also not quite right. What about
>> skip_auditing?
>>
>> And the comment should say which namespace (there's nsid and l->l_ns).
>>
>> Maybe the check should be moved into the caller because
>> notify_audit_modules_of_loaded_object does not need it?
>
> Yeah, this make more sense.
>
* Adhemerval Zanella:
> On 10/12/2021 10:45, Adhemerval Zanella wrote:
>>
>>
>> On 10/12/2021 09:48, Florian Weimer wrote:
>>> * Florian Weimer:
>>>
>>>> * Adhemerval Zanella:
>>>>
>>>>> diff --git a/elf/Makefile b/elf/Makefile
>>>>> index 8f3e3a3602..80ed31edbe 100644
>>>>> --- a/elf/Makefile
>>>>> +++ b/elf/Makefile
>>>>> @@ -36,7 +36,7 @@ dl-routines = $(addprefix dl-,load lookup object reloc deps \
>>>>> exception sort-maps lookup-direct \
>>>>> call-libc-early-init write \
>>>>> thread_gscope_wait tls_init_tp \
>>>>> - debug-symbols minimal-malloc)
>>>>> + debug-symbols minimal-malloc audit)
>>>>
>>>> You can drop the #ifdef SHARED if you add this to rtld-routines.
>>
>> Ack, I will change it.
>
> I recalled why I addon dl-routines: _dl_audit_pltexit is currently required
> for static build as well. I can refactor it, but I think it simpler to
> added the SHARED instead.
I think the usual way is to add stubs to elf/dl-support.c, with a
comment why this is needed (different trampolines glued together in the
build system). This obviously can't be called for static executables.
Thanks,
Florian
On 10/12/2021 11:15, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> On 10/12/2021 10:45, Adhemerval Zanella wrote:
>>>
>>>
>>> On 10/12/2021 09:48, Florian Weimer wrote:
>>>> * Florian Weimer:
>>>>
>>>>> * Adhemerval Zanella:
>>>>>
>>>>>> diff --git a/elf/Makefile b/elf/Makefile
>>>>>> index 8f3e3a3602..80ed31edbe 100644
>>>>>> --- a/elf/Makefile
>>>>>> +++ b/elf/Makefile
>>>>>> @@ -36,7 +36,7 @@ dl-routines = $(addprefix dl-,load lookup object reloc deps \
>>>>>> exception sort-maps lookup-direct \
>>>>>> call-libc-early-init write \
>>>>>> thread_gscope_wait tls_init_tp \
>>>>>> - debug-symbols minimal-malloc)
>>>>>> + debug-symbols minimal-malloc audit)
>>>>>
>>>>> You can drop the #ifdef SHARED if you add this to rtld-routines.
>>>
>>> Ack, I will change it.
>>
>> I recalled why I addon dl-routines: _dl_audit_pltexit is currently required
>> for static build as well. I can refactor it, but I think it simpler to
>> added the SHARED instead.
>
> I think the usual way is to add stubs to elf/dl-support.c, with a
> comment why this is needed (different trampolines glued together in the
> build system). This obviously can't be called for static executables.
Ack.
@@ -36,7 +36,7 @@ dl-routines = $(addprefix dl-,load lookup object reloc deps \
exception sort-maps lookup-direct \
call-libc-early-init write \
thread_gscope_wait tls_init_tp \
- debug-symbols minimal-malloc)
+ debug-symbols minimal-malloc audit)
ifeq (yes,$(use-ldconfig))
dl-routines += dl-cache
endif
new file mode 100644
@@ -0,0 +1,42 @@
+/* Audit common functions.
+ 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 <ldsodefs.h>
+
+#ifdef SHARED
+void
+_dl_audit_objopen (struct link_map *l, Lmid_t nsid, bool check_audit)
+{
+ if (__glibc_likely (GLRO(dl_naudit) == 0)
+ || (check_audit && GL(dl_ns)[l->l_ns]._ns_loaded->l_auditing))
+ return;
+
+ struct audit_ifaces *afct = GLRO(dl_audit);
+ for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
+ {
+ if (afct->objopen != NULL)
+ {
+ struct auditstate *state = link_map_audit_state (l, cnt);
+ state->bindflags = afct->objopen (l, nsid, &state->cookie);
+ l->l_audit_any_plt |= state->bindflags != 0;
+ }
+
+ afct = afct->next;
+ }
+}
+#endif
@@ -1517,22 +1517,7 @@ cannot enable executable stack as shared object requires");
#ifdef SHARED
/* Auditing checkpoint: we have a new object. */
- if (__glibc_unlikely (GLRO(dl_naudit) > 0)
- && !GL(dl_ns)[l->l_ns]._ns_loaded->l_auditing)
- {
- struct audit_ifaces *afct = GLRO(dl_audit);
- for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
- {
- if (afct->objopen != NULL)
- {
- struct auditstate *state = link_map_audit_state (l, cnt);
- state->bindflags = afct->objopen (l, nsid, &state->cookie);
- l->l_audit_any_plt |= state->bindflags != 0;
- }
-
- afct = afct->next;
- }
- }
+ _dl_audit_objopen (l, nsid, true);
#endif
return l;
@@ -1063,25 +1063,6 @@ ERROR: audit interface '%s' requires version %d (maximum supported version %d);
dlmargs.map->l_auditing = 1;
}
-/* Notify the the audit modules that the object MAP has already been
- loaded. */
-static void
-notify_audit_modules_of_loaded_object (struct link_map *map)
-{
- struct audit_ifaces *afct = GLRO(dl_audit);
- for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
- {
- if (afct->objopen != NULL)
- {
- struct auditstate *state = link_map_audit_state (map, cnt);
- state->bindflags = afct->objopen (map, LM_ID_BASE, &state->cookie);
- map->l_audit_any_plt |= state->bindflags != 0;
- }
-
- afct = afct->next;
- }
-}
-
/* Load all audit modules. */
static void
load_audit_modules (struct link_map *main_map, struct audit_list *audit_list)
@@ -1100,8 +1081,8 @@ load_audit_modules (struct link_map *main_map, struct audit_list *audit_list)
program and the dynamic linker itself). */
if (GLRO(dl_naudit) > 0)
{
- notify_audit_modules_of_loaded_object (main_map);
- notify_audit_modules_of_loaded_object (&GL(dl_rtld_map));
+ _dl_audit_objopen (main_map, LM_ID_BASE, false);
+ _dl_audit_objopen (&GL(dl_rtld_map), LM_ID_BASE, false);
}
}
@@ -1391,6 +1391,11 @@ link_map_audit_state (struct link_map *l, size_t index)
return &base[index];
}
}
+
+/* Call the la_objopen() from the audit modules for the link_map L on the
+ namespace identification NSID. If CHECK_AUDIT is set it will also check
+ if main mapping of the namespace is a audit modules. */
+void _dl_audit_objopen (struct link_map *l, Lmid_t nsid, bool check_audit);
#endif /* SHARED */
#if PTHREAD_IN_LIBC && defined SHARED