This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] rtld: Reject overly long LD_AUDIT path elements
On 06/26/2017 11:47 AM, Andreas Schwab wrote:
> On Jun 26 2017, Florian Weimer <fweimer@redhat.com> wrote:
>
>> I had hoped to clean this up after the new dl-minimal malloc went in,
>> but I can do so now if you want that.
>
> Please do.
This is what I came up with.
Thanks,
Florian
rtld: Simplify audit module processing
2017-06-26 Florian Weimer <fweimer@redhat.com>
* elf/rtld.c (struct audit_list_iter, audit_list_iter_init)
(audit_list_iter_next): Remove.
(handle_one_audit_module): New function, extracted from dl_main.
(handle_audit_modules): New function.
(dl_main): Call handle_audit_modules to process audit modules.
diff --git a/elf/rtld.c b/elf/rtld.c
index 65647fb..e91dcd8 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -141,79 +141,6 @@ static struct audit_list
struct audit_list *next;
} *audit_list;
-/* Iterator for audit_list_string followed by audit_list. */
-struct audit_list_iter
-{
- /* Tail of audit_list_string still needing processing, or NULL. */
- const char *audit_list_tail;
-
- /* The list element returned in the previous iteration. NULL before
- the first element. */
- struct audit_list *previous;
-
- /* Scratch buffer for returning a name which is part of
- audit_list_string. */
- char fname[SECURE_NAME_LIMIT];
-};
-
-/* Initialize an audit list iterator. */
-static void
-audit_list_iter_init (struct audit_list_iter *iter)
-{
- iter->audit_list_tail = audit_list_string;
- iter->previous = NULL;
-}
-
-/* Iterate through both audit_list_string and audit_list. */
-static const char *
-audit_list_iter_next (struct audit_list_iter *iter)
-{
- if (iter->audit_list_tail != NULL)
- {
- /* First iterate over audit_list_string. */
- while (*iter->audit_list_tail != '\0')
- {
- /* Split audit list at colon. */
- size_t len = strcspn (iter->audit_list_tail, ":");
- if (len > 0 && len < sizeof (iter->fname))
- {
- memcpy (iter->fname, iter->audit_list_tail, len);
- iter->fname[len] = '\0';
- }
- else
- /* Do not return this name to the caller. */
- iter->fname[0] = '\0';
-
- /* Skip over the substring and the following delimiter. */
- iter->audit_list_tail += len;
- if (*iter->audit_list_tail == ':')
- ++iter->audit_list_tail;
-
- /* If the name is valid, return it. */
- if (dso_name_valid_for_suid (iter->fname))
- return iter->fname;
- /* Otherwise, wrap around and try the next name. */
- }
- /* Fall through to the procesing of audit_list. */
- }
-
- if (iter->previous == NULL)
- {
- if (audit_list == NULL)
- /* No pre-parsed audit list. */
- return NULL;
- /* Start of audit list. The first list element is at
- audit_list->next (cyclic list). */
- iter->previous = audit_list->next;
- return iter->previous->name;
- }
- if (iter->previous == audit_list)
- /* Cyclic list wrap-around. */
- return NULL;
- iter->previous = iter->previous->next;
- return iter->previous->name;
-}
-
#ifndef HAVE_INLINED_SYSCALLS
/* Set nonzero during loading and initialization of executable and
libraries, cleared before the executable's entry point runs. This
@@ -861,6 +788,181 @@ handle_ld_preload (const char *preloadlist, struct link_map *main_map)
return npreloads;
}
+/* Load a single audit module. */
+void
+handle_one_audit_module (const char *name, struct audit_ifaces **last_audit)
+{
+ int tls_idx = GL(dl_tls_max_dtv_idx);
+
+ /* Now it is time to determine the layout of the static TLS
+ block and allocate it for the initial thread. Note that we
+ always allocate the static block, we never defer it even if
+ no DF_STATIC_TLS bit is set. The reason is that we know
+ glibc will use the static model. */
+ struct dlmopen_args dlmargs;
+ dlmargs.fname = name;
+ dlmargs.map = NULL;
+
+ const char *objname;
+ const char *err_str = NULL;
+ bool malloced;
+ (void) _dl_catch_error (&objname, &err_str, &malloced, dlmopen_doit,
+ &dlmargs);
+ if (__glibc_unlikely (err_str != NULL))
+ {
+ not_loaded:
+ _dl_error_printf ("\
+ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
+ name, err_str);
+ if (malloced)
+ free ((char *) err_str);
+ }
+ else
+ {
+ struct lookup_args largs;
+ largs.name = "la_version";
+ largs.map = dlmargs.map;
+
+ /* Check whether the interface version matches. */
+ (void) _dl_catch_error (&objname, &err_str, &malloced,
+ lookup_doit, &largs);
+
+ unsigned int (*laversion) (unsigned int);
+ unsigned int lav;
+ if (err_str == NULL
+ && (laversion = largs.result) != NULL
+ && (lav = laversion (LAV_CURRENT)) > 0
+ && lav <= LAV_CURRENT)
+ {
+ /* Allocate structure for the callback function pointers.
+ This call can never fail. */
+ union
+ {
+ struct audit_ifaces ifaces;
+#define naudit_ifaces 8
+ void (*fptr[naudit_ifaces]) (void);
+ } *newp = malloc (sizeof (*newp));
+
+ /* Names of the auditing interfaces. All in one
+ long string. */
+ static const char audit_iface_names[] =
+ "la_activity\0"
+ "la_objsearch\0"
+ "la_objopen\0"
+ "la_preinit\0"
+#if __ELF_NATIVE_CLASS == 32
+ "la_symbind32\0"
+#elif __ELF_NATIVE_CLASS == 64
+ "la_symbind64\0"
+#else
+# error "__ELF_NATIVE_CLASS must be defined"
+#endif
+#define STRING(s) __STRING (s)
+ "la_" STRING (ARCH_LA_PLTENTER) "\0"
+ "la_" STRING (ARCH_LA_PLTEXIT) "\0"
+ "la_objclose\0";
+ unsigned int cnt = 0;
+ const char *cp = audit_iface_names;
+ do
+ {
+ largs.name = cp;
+ (void) _dl_catch_error (&objname, &err_str, &malloced,
+ lookup_doit, &largs);
+
+ /* Store the pointer. */
+ if (err_str == NULL && largs.result != NULL)
+ {
+ newp->fptr[cnt] = largs.result;
+
+ /* The dynamic linker link map is statically
+ allocated, initialize the data now. */
+ GL(dl_rtld_map).l_audit[cnt].cookie
+ = (intptr_t) &GL(dl_rtld_map);
+ }
+ else
+ newp->fptr[cnt] = NULL;
+ ++cnt;
+
+ cp = (char *) rawmemchr (cp, '\0') + 1;
+ }
+ while (*cp != '\0');
+ assert (cnt == naudit_ifaces);
+
+ /* Now append the new auditing interface to the list. */
+ newp->ifaces.next = NULL;
+ if (*last_audit == NULL)
+ *last_audit = GLRO(dl_audit) = &newp->ifaces;
+ else
+ *last_audit = (*last_audit)->next = &newp->ifaces;
+ ++GLRO(dl_naudit);
+
+ /* Mark the DSO as being used for auditing. */
+ dlmargs.map->l_auditing = 1;
+ }
+ else
+ {
+ /* We cannot use the DSO, it does not have the
+ appropriate interfaces or it expects something
+ more recent. */
+#ifndef NDEBUG
+ Lmid_t ns = dlmargs.map->l_ns;
+#endif
+ _dl_close (dlmargs.map);
+
+ /* Make sure the namespace has been cleared entirely. */
+ assert (GL(dl_ns)[ns]._ns_loaded == NULL);
+ assert (GL(dl_ns)[ns]._ns_nloaded == 0);
+
+ GL(dl_tls_max_dtv_idx) = tls_idx;
+ goto not_loaded;
+ }
+ }
+}
+
+/* Process the audit modules in audit_list and audit_list_string. */
+void
+handle_audit_modules (void)
+{
+ char fname[SECURE_PATH_LIMIT];
+ struct audit_ifaces *last_audit = NULL;
+
+ if (audit_list_string != NULL)
+ {
+ const char *p = audit_list_string;
+ while (*p != '\0')
+ {
+ /* Split preload list at space/colon. */
+ size_t len = strcspn (p, ":");
+ if (len > 0 && len < sizeof (fname))
+ {
+ memcpy (fname, p, len);
+ fname[len] = '\0';
+ }
+ else
+ fname[0] = '\0';
+
+ /* Skip over the substring and the following delimiter. */
+ p += len;
+ if (*p != '\0')
+ ++p;
+
+ if (dso_name_valid_for_suid (fname))
+ handle_one_audit_module (fname, &last_audit);
+ }
+ }
+
+ if (audit_list != NULL)
+ {
+ struct audit_list *al = audit_list->next;
+ do
+ {
+ handle_one_audit_module (al->name, &last_audit);
+ al = al->next;
+ }
+ while (al != audit_list->next);
+ }
+}
+
static void
dl_main (const ElfW(Phdr) *phdr,
ElfW(Word) phnum,
@@ -1387,10 +1489,6 @@ of this helper program; chances are you did not intend to run this program.\n\
if (__glibc_unlikely (audit_list != NULL)
|| __glibc_unlikely (audit_list_string != NULL))
{
- struct audit_ifaces *last_audit = NULL;
- struct audit_list_iter al_iter;
- audit_list_iter_init (&al_iter);
-
/* Since we start using the auditing DSOs right away we need to
initialize the data structures now. */
tcbp = init_tls ();
@@ -1402,138 +1500,7 @@ of this helper program; chances are you did not intend to run this program.\n\
security_init ();
need_security_init = false;
- while (true)
- {
- const char *name = audit_list_iter_next (&al_iter);
- if (name == NULL)
- break;
-
- int tls_idx = GL(dl_tls_max_dtv_idx);
-
- /* Now it is time to determine the layout of the static TLS
- block and allocate it for the initial thread. Note that we
- always allocate the static block, we never defer it even if
- no DF_STATIC_TLS bit is set. The reason is that we know
- glibc will use the static model. */
- struct dlmopen_args dlmargs;
- dlmargs.fname = name;
- dlmargs.map = NULL;
-
- const char *objname;
- const char *err_str = NULL;
- bool malloced;
- (void) _dl_catch_error (&objname, &err_str, &malloced, dlmopen_doit,
- &dlmargs);
- if (__glibc_unlikely (err_str != NULL))
- {
- not_loaded:
- _dl_error_printf ("\
-ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
- name, err_str);
- if (malloced)
- free ((char *) err_str);
- }
- else
- {
- struct lookup_args largs;
- largs.name = "la_version";
- largs.map = dlmargs.map;
-
- /* Check whether the interface version matches. */
- (void) _dl_catch_error (&objname, &err_str, &malloced,
- lookup_doit, &largs);
-
- unsigned int (*laversion) (unsigned int);
- unsigned int lav;
- if (err_str == NULL
- && (laversion = largs.result) != NULL
- && (lav = laversion (LAV_CURRENT)) > 0
- && lav <= LAV_CURRENT)
- {
- /* Allocate structure for the callback function pointers.
- This call can never fail. */
- union
- {
- struct audit_ifaces ifaces;
-#define naudit_ifaces 8
- void (*fptr[naudit_ifaces]) (void);
- } *newp = malloc (sizeof (*newp));
-
- /* Names of the auditing interfaces. All in one
- long string. */
- static const char audit_iface_names[] =
- "la_activity\0"
- "la_objsearch\0"
- "la_objopen\0"
- "la_preinit\0"
-#if __ELF_NATIVE_CLASS == 32
- "la_symbind32\0"
-#elif __ELF_NATIVE_CLASS == 64
- "la_symbind64\0"
-#else
-# error "__ELF_NATIVE_CLASS must be defined"
-#endif
-#define STRING(s) __STRING (s)
- "la_" STRING (ARCH_LA_PLTENTER) "\0"
- "la_" STRING (ARCH_LA_PLTEXIT) "\0"
- "la_objclose\0";
- unsigned int cnt = 0;
- const char *cp = audit_iface_names;
- do
- {
- largs.name = cp;
- (void) _dl_catch_error (&objname, &err_str, &malloced,
- lookup_doit, &largs);
-
- /* Store the pointer. */
- if (err_str == NULL && largs.result != NULL)
- {
- newp->fptr[cnt] = largs.result;
-
- /* The dynamic linker link map is statically
- allocated, initialize the data now. */
- GL(dl_rtld_map).l_audit[cnt].cookie
- = (intptr_t) &GL(dl_rtld_map);
- }
- else
- newp->fptr[cnt] = NULL;
- ++cnt;
-
- cp = (char *) rawmemchr (cp, '\0') + 1;
- }
- while (*cp != '\0');
- assert (cnt == naudit_ifaces);
-
- /* Now append the new auditing interface to the list. */
- newp->ifaces.next = NULL;
- if (last_audit == NULL)
- last_audit = GLRO(dl_audit) = &newp->ifaces;
- else
- last_audit = last_audit->next = &newp->ifaces;
- ++GLRO(dl_naudit);
-
- /* Mark the DSO as being used for auditing. */
- dlmargs.map->l_auditing = 1;
- }
- else
- {
- /* We cannot use the DSO, it does not have the
- appropriate interfaces or it expects something
- more recent. */
-#ifndef NDEBUG
- Lmid_t ns = dlmargs.map->l_ns;
-#endif
- _dl_close (dlmargs.map);
-
- /* Make sure the namespace has been cleared entirely. */
- assert (GL(dl_ns)[ns]._ns_loaded == NULL);
- assert (GL(dl_ns)[ns]._ns_nloaded == 0);
-
- GL(dl_tls_max_dtv_idx) = tls_idx;
- goto not_loaded;
- }
- }
- }
+ handle_audit_modules ();
/* If we have any auditing modules, announce that we already
have two objects loaded. */