From patchwork Mon Jun 26 10:56:50 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 21261 Received: (qmail 79061 invoked by alias); 26 Jun 2017 10:56:58 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 79022 invoked by uid 89); 26 Jun 2017 10:56:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com DD1EBFEEF5 Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=fweimer@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com DD1EBFEEF5 Subject: Re: [PATCH] rtld: Reject overly long LD_AUDIT path elements To: Andreas Schwab Cc: libc-alpha@sourceware.org References: <20170619161345.7CC73402AEC3E@oldenburg.str.redhat.com> <54c5074e-8b7c-7b3b-e5b1-02ee18a7cabe@redhat.com> <50beae35-82bd-ae09-064d-676fd26fc1c0@redhat.com> From: Florian Weimer Message-ID: <6701f70e-f623-ef1a-1ef2-a4d47d7e11b0@redhat.com> Date: Mon, 26 Jun 2017 12:56:50 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 In-Reply-To: On 06/26/2017 11:47 AM, Andreas Schwab wrote: > On Jun 26 2017, Florian Weimer 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 * 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. */