From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2178) id 55C183858438; Tue, 20 Sep 2022 10:12:17 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 55C183858438 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1663668737; bh=RRLzkJ4VWO43Zviv4ZQ3WVIYmbVaHu/jcsOd2uVBwhI=; h=From:To:Subject:Date:From; b=gItKrP/fNI0HsiJjlPCVxknyTCtNHbxlvhr5RtFkeaTw8JEu+f1ioDOsRVpU7bUY9 Nd0nC1NvUmylhtA6WDFr3VGppP30cr+Ul52E/tMiS7Z15DzscSk5lTM2z0APABDLLp sLX8wno4oMG+pyCKcj175Qdgf23pKm8n1h+ue2+w= MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="utf-8" From: Florian Weimer To: glibc-cvs@sourceware.org Subject: [glibc/release/2.35/master] elf: Implement force_first handling in _dl_sort_maps_dfs (bug 28937) X-Act-Checkin: glibc X-Git-Author: Florian Weimer X-Git-Refname: refs/heads/release/2.35/master X-Git-Oldrev: c15c0f198584ecb2d58a478d7e88b1ce5bc81df8 X-Git-Newrev: d883d02008b0bd214bdca982f4be82a8d6856934 Message-Id: <20220920101217.55C183858438@sourceware.org> Date: Tue, 20 Sep 2022 10:12:17 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=d883d02008b0bd214bdca982f4be82a8d6856934 commit d883d02008b0bd214bdca982f4be82a8d6856934 Author: Florian Weimer Date: Tue Sep 20 11:00:42 2022 +0200 elf: Implement force_first handling in _dl_sort_maps_dfs (bug 28937) The implementation in _dl_close_worker requires that the first element of l_initfini is always this very map (“We are always the zeroth entry, and since we don't include ourselves in the dependency analysis start at 1.”). Rather than fixing that assumption, this commit adds an implementation of the force_first argument to the new dependency sorting algorithm. This also means that the directly dlopen'ed shared object is always initialized last, which is the least surprising behavior in the presence of cycles. Reviewed-by: Adhemerval Zanella (cherry picked from commit 1df71d32fe5f5905ffd5d100e5e9ca8ad6210891) Diff: --- NEWS | 1 + elf/dl-sort-maps.c | 32 +++++++++++++++++++++++--------- elf/dso-sort-tests-1.def | 7 +++++++ 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/NEWS b/NEWS index cabd3f0b5e..ee0b808516 100644 --- a/NEWS +++ b/NEWS @@ -24,6 +24,7 @@ The following bugs are resolved with this release: missing libraries [28896] strncmp-avx2-rtm and wcsncmp-avx2-rtm fallback on non-rtm variants when avoiding overflow + [28937] New DSO dependency sorter does not put new map first if in a cycle [28953] nss: Protect against errno changes in function lookup [29029] nptl: poll() spuriously returns EINTR during thread cancellation and with cancellation disabled diff --git a/elf/dl-sort-maps.c b/elf/dl-sort-maps.c index 5b550b1e94..3e2a6a584e 100644 --- a/elf/dl-sort-maps.c +++ b/elf/dl-sort-maps.c @@ -182,8 +182,9 @@ dfs_traversal (struct link_map ***rpo, struct link_map *map, static void _dl_sort_maps_dfs (struct link_map **maps, unsigned int nmaps, - bool force_first __attribute__ ((unused)), bool for_fini) + bool force_first, bool for_fini) { + struct link_map *first_map = maps[0]; for (int i = nmaps - 1; i >= 0; i--) maps[i]->l_visited = 0; @@ -208,14 +209,6 @@ _dl_sort_maps_dfs (struct link_map **maps, unsigned int nmaps, Adjusting the order so that maps[0] is last traversed naturally avoids this problem. - Further, the old "optimization" of skipping the main object at maps[0] - from the call-site (i.e. _dl_sort_maps(maps+1,nmaps-1)) is in general - no longer valid, since traversing along object dependency-links - may "find" the main object even when it is not included in the initial - order (e.g. a dlopen()'ed shared object can have circular dependencies - linked back to itself). In such a case, traversing N-1 objects will - create a N-object result, and raise problems. - To summarize, just passing in the full list, and iterating from back to front makes things much more straightforward. */ @@ -274,6 +267,27 @@ _dl_sort_maps_dfs (struct link_map **maps, unsigned int nmaps, } memcpy (maps, rpo, sizeof (struct link_map *) * nmaps); + + /* Skipping the first object at maps[0] is not valid in general, + since traversing along object dependency-links may "find" that + first object even when it is not included in the initial order + (e.g., a dlopen'ed shared object can have circular dependencies + linked back to itself). In such a case, traversing N-1 objects + will create a N-object result, and raise problems. Instead, + force the object back into first place after sorting. This naive + approach may introduce further dependency ordering violations + compared to rotating the cycle until the first map is again in + the first position, but as there is a cycle, at least one + violation is already present. */ + if (force_first && maps[0] != first_map) + { + int i; + for (i = 0; maps[i] != first_map; ++i) + ; + assert (i < nmaps); + memmove (&maps[1], maps, i * sizeof (maps[0])); + maps[0] = first_map; + } } void diff --git a/elf/dso-sort-tests-1.def b/elf/dso-sort-tests-1.def index 5f7f18ef27..4bf9052db1 100644 --- a/elf/dso-sort-tests-1.def +++ b/elf/dso-sort-tests-1.def @@ -64,3 +64,10 @@ output: b>a>{}b->c->d;d=>[ba];c=>a;b=>e=>a;c=>f=>b;d=>g=>c output(glibc.rtld.dynamic_sort=1): {+a[d>c>b>a>];+e[e>];+f[f>];+g[g>];+d[];%d(b(e(a()))a()g(c(a()f(b(e(a()))))));-d[];-g[];-f[];-e[];-a[c>b>a>];+e[e>];+f[f>];+g[g>];+d[];%d(b(e(a()))a()g(c(a()f(b(e(a()))))));-d[];-g[];-f[];-e[];-a[a1;a->a2;a2->a;b->b1;c->a1;c=>a1 +output(glibc.rtld.dynamic_sort=1): {+a[a2>a1>a>];+b[b1>b>];-b[];%c(a1());}a1>a>];+b[b1>b>];-b[];%c(a1());}