From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by sourceware.org (Postfix) with ESMTP id EF5863851C3B for ; Thu, 4 Jun 2020 12:57:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org EF5863851C3B Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-221-SU3hBUEkPM-OltC70V486A-1; Thu, 04 Jun 2020 08:57:48 -0400 X-MC-Unique: SU3hBUEkPM-OltC70V486A-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id F213E106B1D4 for ; Thu, 4 Jun 2020 12:57:47 +0000 (UTC) Received: from oldenburg2.str.redhat.com (ovpn-114-15.ams2.redhat.com [10.36.114.15]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 836AF99008; Thu, 4 Jun 2020 12:57:43 +0000 (UTC) From: Florian Weimer To: Carlos O'Donell Cc: libc-alpha@sourceware.org Subject: Re: [PATCH] elf: Fix dlclose of an empty namespace in auditing mode (bug 26076) References: <87zh9kuuw1.fsf@oldenburg2.str.redhat.com> <5d08b5e0-c96a-0c97-304f-8509f7b0f322@redhat.com> <87h7vruaoi.fsf@oldenburg2.str.redhat.com> Date: Thu, 04 Jun 2020 14:57:41 +0200 Message-ID: <87zh9jugxm.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-13.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 Jun 2020 12:57:52 -0000 * Carlos O'Donell: >>>> elf/dl-close.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/elf/dl-close.c b/elf/dl-close.c >>>> index 73b2817bbf..896e59e42e 100644 >>>> --- a/elf/dl-close.c >>>> +++ b/elf/dl-close.c >>>> @@ -782,7 +782,7 @@ _dl_close_worker (struct link_map *map, bool force) >>>> { >>>> struct link_map *head = ns->_ns_loaded; >>>> /* Do not call the functions for any auditing object. */ >>>> - if (head->l_auditing == 0) >>>> + if (head != NULL && head->l_auditing == 0) >>>> { >>>> struct audit_ifaces *afct = GLRO(dl_audit); >>>> for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt) >>>> >>> >>> Use _dl_signal_error to indicate an internal error? >> >> This *is* on the cleanup path after an error already happened. This is >> the backtrace I saw: > > I'm going to ignore the oddity of auditors auditing themselves, I guess it's > logically consistent that the first loaded auditor won't see any of this, but > that subsequently loaded auditors will. > > Once auditor loading fails in your case we begin unloading that object. What I > don't understand though is why is GLRO(dl_naudit) non-zero? > > If this is the first loaded auditor we don't increment GLRO(dl_naudit) until > after we return from load_audit_module, so do_audit is false, and we never > get here Agreed. > If this is the second loaded auditor we do set do_audit to true, but this > means we already dereferenced ns->_ns_loaded->l_auditing already once to > compute do_audit. What has subsequently happened to ns->_ns_loaded? For it > to be null means we removed *every* object from the namespace, but we just > established we had at least one already loaded successfully? It looks like the namespace is not actually empty initially: (gdb) print _rtld_global._dl_ns[9] $10 = {_ns_loaded = 0x7ffff7ee7590, _ns_nloaded = 3, _ns_main_searchlist = 0x0, _ns_global_scope_alloc = 0, _ns_global_scope_pending_adds = 0, libc_map = 0x0, _ns_unique_sym_table = { lock = {mutex = {__data = {__lock = 0, __count = 0, __owner = 0, __nusers = 0, __kind = 1, __spins = 0, __elision = 0, __list = { __prev = 0x0, __next = 0x0}}, __size = '\000' , "\001", '\000' , __align = 0}}, entries = 0x0, size = 0, n_elements = 0, free = 0x0}, _ns_debug = {r_version = 1, r_map = 0x7ffff7ee7590, r_brk = 140737351924736, r_state = RT_CONSISTENT, r_ldbase = 140737351860224}} (gdb) print _rtld_global._dl_ns[9]._ns_loaded->l_name $13 = 0x7ffff7ee7570 "./elf/tst-auditmanymod9.so" (gdb) print _rtld_global._dl_ns[9]._ns_loaded->l_next->l_name $14 = 0x7ffff7ee7b70 "./libc.so.6" (gdb) print _rtld_global._dl_ns[9]._ns_loaded->l_next->l_next->l_name $15 = 0x7ffff7ee8140 "./elf/ld-linux-x86-64.so.2" So the commit message is wrong: the namespace doesn't start out as empty, it becomes empty during dlclose. It should be fairly easy to write a test for this. I'll post a new patch shortly. Thanks, Florian