[v7,03/16] elf: Add _dl_audit_objsearch
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_objsearch audit
callback.
Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.
---
elf/dl-audit.c | 22 ++++++++++++
elf/dl-load.c | 69 +++++++++++---------------------------
sysdeps/generic/ldsodefs.h | 7 ++++
3 files changed, 48 insertions(+), 50 deletions(-)
Comments
* Adhemerval Zanella:
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index d1d02fa70e..cf15f85df6 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -1602,32 +1602,20 @@ open_verify (const char *name, int fd,
> - if (fd != -1 && name != original_name && strcmp (name, original_name))
> - {
> - /* An audit library changed what we're supposed to open,
> - so FD no longer matches it. */
> - __close_nocancel (fd);
> - fd = -1;
> - }
> + if (fd != -1 && name != original_name && strcmp (name, original_name))
> + {
> + /* An audit library changed what we're supposed to open,
> + so FD no longer matches it. */
> + __close_nocancel (fd);
> + fd = -1;
> + }
Spurious whitespace change, I think.
> @@ -2066,36 +2054,17 @@ _dl_map_object (struct link_map *loader, const char *name,
> #ifdef SHARED
> /* Give the auditing libraries a chance to change the name before we
> try anything. */
> - if (__glibc_unlikely (GLRO(dl_naudit) > 0)
> - && (loader == NULL || loader->l_auditing == 0))
> + if (__glibc_unlikely (GLRO(dl_naudit) > 0))
> {
> - struct audit_ifaces *afct = GLRO(dl_audit);
> - for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
> + const char *before = name;
> + name = _dl_audit_objsearch (name, loader, LA_SER_ORIG);
> + if (name == NULL)
> {
> - if (afct->objsearch != NULL)
> - {
> - const char *before = name;
> - struct auditstate *state = link_map_audit_state (loader, cnt);
> - name = afct->objsearch (name, &state->cookie, LA_SER_ORIG);
> - if (name == NULL)
> - {
> - /* Do not try anything further. */
> - fd = -1;
> - goto no_file;
> - }
> - if (before != name && strcmp (before, name) != 0)
> - {
> - if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
> - _dl_debug_printf ("audit changed filename %s -> %s\n",
> - before, name);
> -
> - if (origname == NULL)
> - origname = before;
> - }
> - }
> -
> - afct = afct->next;
> + fd = -1;
> + goto no_file;
> }
> + if (before != name && strcmp (before, name) != 0)
> + origname = before;
> }
> #endif
I had to stare at this for a bit, but the new handling of originame and
before should give the same result in the end.
Reviewed-by: Florian Weimer <fweimer@redhat.com>
Thanks,
Florian
On 24/12/2021 09:05, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> diff --git a/elf/dl-load.c b/elf/dl-load.c
>> index d1d02fa70e..cf15f85df6 100644
>> --- a/elf/dl-load.c
>> +++ b/elf/dl-load.c
>> @@ -1602,32 +1602,20 @@ open_verify (const char *name, int fd,
>
>> - if (fd != -1 && name != original_name && strcmp (name, original_name))
>> - {
>> - /* An audit library changed what we're supposed to open,
>> - so FD no longer matches it. */
>> - __close_nocancel (fd);
>> - fd = -1;
>> - }
>> + if (fd != -1 && name != original_name && strcmp (name, original_name))
>> + {
>> + /* An audit library changed what we're supposed to open,
>> + so FD no longer matches it. */
>> + __close_nocancel (fd);
>> + fd = -1;
>> + }
>
> Spurious whitespace change, I think.
Ack, it also leads to wrong indentation. I will fix it.
>
>> @@ -2066,36 +2054,17 @@ _dl_map_object (struct link_map *loader, const char *name,
>> #ifdef SHARED
>> /* Give the auditing libraries a chance to change the name before we
>> try anything. */
>> - if (__glibc_unlikely (GLRO(dl_naudit) > 0)
>> - && (loader == NULL || loader->l_auditing == 0))
>> + if (__glibc_unlikely (GLRO(dl_naudit) > 0))
>> {
>> - struct audit_ifaces *afct = GLRO(dl_audit);
>> - for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
>> + const char *before = name;
>> + name = _dl_audit_objsearch (name, loader, LA_SER_ORIG);
>> + if (name == NULL)
>> {
>> - if (afct->objsearch != NULL)
>> - {
>> - const char *before = name;
>> - struct auditstate *state = link_map_audit_state (loader, cnt);
>> - name = afct->objsearch (name, &state->cookie, LA_SER_ORIG);
>> - if (name == NULL)
>> - {
>> - /* Do not try anything further. */
>> - fd = -1;
>> - goto no_file;
>> - }
>> - if (before != name && strcmp (before, name) != 0)
>> - {
>> - if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
>> - _dl_debug_printf ("audit changed filename %s -> %s\n",
>> - before, name);
>> -
>> - if (origname == NULL)
>> - origname = before;
>> - }
>> - }
>> -
>> - afct = afct->next;
>> + fd = -1;
>> + goto no_file;
>> }
>> + if (before != name && strcmp (before, name) != 0)
>> + origname = before;
>> }
>> #endif
>
> I had to stare at this for a bit, but the new handling of originame and
> before should give the same result in the end.
>
> Reviewed-by: Florian Weimer <fweimer@redhat.com>
>
> Thanks,
> Florian
>
@@ -48,6 +48,28 @@ _dl_audit_activity_nsid (Lmid_t nsid, int action)
_dl_audit_activity_map (head, action);
}
+const char *
+_dl_audit_objsearch (const char *name, struct link_map *l, unsigned int code)
+{
+ if (l == NULL || l->l_auditing || code == 0)
+ return name;
+
+ struct audit_ifaces *afct = GLRO(dl_audit);
+ for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
+ {
+ if (afct->objsearch != NULL)
+ {
+ struct auditstate *state = link_map_audit_state (l, cnt);
+ name = afct->objsearch (name, &state->cookie, code);
+ if (name == NULL)
+ return NULL;
+ }
+ afct = afct->next;
+ }
+
+ return name;
+}
+
void
_dl_audit_objopen (struct link_map *l, Lmid_t nsid)
{
@@ -1602,32 +1602,20 @@ open_verify (const char *name, int fd,
#ifdef SHARED
/* Give the auditing libraries a chance. */
- if (__glibc_unlikely (GLRO(dl_naudit) > 0) && whatcode != 0
- && loader->l_auditing == 0)
+ if (__glibc_unlikely (GLRO(dl_naudit) > 0))
{
const char *original_name = name;
- struct audit_ifaces *afct = GLRO(dl_audit);
- for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
- {
- if (afct->objsearch != NULL)
- {
- struct auditstate *state = link_map_audit_state (loader, cnt);
- name = afct->objsearch (name, &state->cookie, whatcode);
- if (name == NULL)
- /* Ignore the path. */
- return -1;
- }
-
- afct = afct->next;
- }
+ name = _dl_audit_objsearch (name, loader, whatcode);
+ if (name == NULL)
+ return -1;
- if (fd != -1 && name != original_name && strcmp (name, original_name))
- {
- /* An audit library changed what we're supposed to open,
- so FD no longer matches it. */
- __close_nocancel (fd);
- fd = -1;
- }
+ if (fd != -1 && name != original_name && strcmp (name, original_name))
+ {
+ /* An audit library changed what we're supposed to open,
+ so FD no longer matches it. */
+ __close_nocancel (fd);
+ fd = -1;
+ }
}
#endif
@@ -2066,36 +2054,17 @@ _dl_map_object (struct link_map *loader, const char *name,
#ifdef SHARED
/* Give the auditing libraries a chance to change the name before we
try anything. */
- if (__glibc_unlikely (GLRO(dl_naudit) > 0)
- && (loader == NULL || loader->l_auditing == 0))
+ if (__glibc_unlikely (GLRO(dl_naudit) > 0))
{
- struct audit_ifaces *afct = GLRO(dl_audit);
- for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
+ const char *before = name;
+ name = _dl_audit_objsearch (name, loader, LA_SER_ORIG);
+ if (name == NULL)
{
- if (afct->objsearch != NULL)
- {
- const char *before = name;
- struct auditstate *state = link_map_audit_state (loader, cnt);
- name = afct->objsearch (name, &state->cookie, LA_SER_ORIG);
- if (name == NULL)
- {
- /* Do not try anything further. */
- fd = -1;
- goto no_file;
- }
- if (before != name && strcmp (before, name) != 0)
- {
- if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
- _dl_debug_printf ("audit changed filename %s -> %s\n",
- before, name);
-
- if (origname == NULL)
- origname = before;
- }
- }
-
- afct = afct->next;
+ fd = -1;
+ goto no_file;
}
+ if (before != name && strcmp (before, name) != 0)
+ origname = before;
}
#endif
@@ -1396,6 +1396,13 @@ link_map_audit_state (struct link_map *l, size_t index)
}
}
+/* Call the la_objsearch from the audit modules from the link map L. If
+ ORIGNAME is non NULL, it is updated with the revious name prior calling
+ la_objsearch. */
+const char *_dl_audit_objsearch (const char *name, struct link_map *l,
+ unsigned int code)
+ attribute_hidden;
+
/* Call the la_activity() from the audit modules from the link map L
and issues the ACTION argument. */
void _dl_audit_activity_map (struct link_map *l, int action)