From: Mark Hatle <mark.hatle@kernel.crashing.org>
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>,
Khem Raj <raj.khem@gmail.com>,
libc-alpha@sourceware.org
Cc: Mark Hatle <mark.hatle@windriver.com>
Subject: Re: [PATCH] elf/dl-deps.c: Make _dl_build_local_scope breadth first
Date: Wed, 12 Jan 2022 13:08:54 -0600 [thread overview]
Message-ID: <62d5866a-ea76-a56a-7063-dada34b3fe66@kernel.crashing.org> (raw)
In-Reply-To: <018ad7e3-c020-3507-94be-ccb21c90899f@linaro.org>
On 1/11/22 1:26 PM, Adhemerval Zanella wrote:
>
>
> On 09/12/2021 20:53, Khem Raj via Libc-alpha wrote:
>> From: Mark Hatle <mark.hatle@windriver.com>
>>
>> According to the ELF specification:
>>
>> When resolving symbolic references, the dynamic linker examines the symbol
>> tables with a breadth-first search.
>>
>> This function was using a depth first search. By doing so the conflict
>> resolution reported to the prelinker (when LD_TRACE_PRELINKING=1 is set)
>> was incorrect. This caused problems when their were various circular
>> dependencies between libraries. The problem usually manifested itself by
>> the wrong IFUNC being executed.
>>
>> Similar issue has been reported here [1]
>>
>> [BZ# 20488]
>>
>> [1] https://sourceware.org/legacy-ml/libc-alpha/2016-05/msg00034.html
>>
>> Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
>
> I am trying to understand why it this only an issue for LD_TRACE_PRELINKING=1,
> do we have a testcase that stress it for a default usercase?
The underlying issue here is that resolution is happening depth first and not
breadth first. According to the ELF spec, all resolution should be breadth-first.
As noted in item in above, the prelinker just happens to be a way to actually
show that the behavior is incorrect. (There even appears to be a related defect
with a reproducer.)
When taking the values from LD_TRACE_PRELINKING=1, various addresses and
conflict resolutions are specified. When you compare what is reported, vs what
happens, vs what the spec says they don't align as they should.
> When you say the 'wrong IFUNC being executed' what exactly you mean here?
> Could we use a testcase based on this?
The prelinker (and possibly just in general), the IFUNC address used is the one
from the wrong library scope. I personally have never tried to reproduce this
outside of the prelinking use-case, but based on the referenced report and the
code at the time of the change, it is believed this could happen (but rarely)
without a prelinked system in a very complex case with multiple libraries
providing the same functions.
The main issue (my memory is sketchy sorry this might be wrong) is that if one
or more functions is an IFUNC and one or more functions is NOT, then the you can
get into a situation with a conflict of the wrong function being called using
the wrong mechanism.
Definitely in the prelink case, the resolver gave the address of a regular
function which was then placed into an IFUNC (or maybe it was the other way
around) triggering the runtime segfault.
--Mark
>> ---
>> elf/dl-deps.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/elf/dl-deps.c b/elf/dl-deps.c
>> index 237d9636c5..e15f7f83d8 100644
>> --- a/elf/dl-deps.c
>> +++ b/elf/dl-deps.c
>> @@ -73,13 +73,19 @@ _dl_build_local_scope (struct link_map **list, struct link_map *map)
>> {
>> struct link_map **p = list;
>> struct link_map **q;
>> + struct link_map **r;
>>
>> *p++ = map;
>> map->l_reserved = 1;
>> - if (map->l_initfini)
>> - for (q = map->l_initfini + 1; *q; ++q)
>> - if (! (*q)->l_reserved)
>> - p += _dl_build_local_scope (p, *q);
>> +
>> + for (r = list; r < p; ++r)
>> + if ((*r)->l_initfini)
>> + for (q = (*r)->l_initfini + 1; *q; ++q)
>> + if (! (*q)->l_reserved)
>> + {
>> + *p++ = *q;
>> + (*q)->l_reserved = 1;
>> + }
>> return p - list;
>> }
>>
next prev parent reply other threads:[~2022-01-12 19:12 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-09 23:53 Khem Raj
2022-01-11 19:26 ` Adhemerval Zanella
2022-01-12 19:08 ` Mark Hatle [this message]
2022-01-12 20:12 ` Adhemerval Zanella
2022-01-12 20:41 ` Mark Hatle
2022-01-13 11:52 ` Adhemerval Zanella
2022-01-13 16:33 ` Mark Hatle
2022-01-13 17:20 ` Adhemerval Zanella
2022-01-13 18:00 ` Mark Hatle
2022-01-13 18:37 ` Adhemerval Zanella
2022-01-13 19:01 ` Carlos O'Donell
2022-01-13 19:02 ` Carlos O'Donell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=62d5866a-ea76-a56a-7063-dada34b3fe66@kernel.crashing.org \
--to=mark.hatle@kernel.crashing.org \
--cc=adhemerval.zanella@linaro.org \
--cc=libc-alpha@sourceware.org \
--cc=mark.hatle@windriver.com \
--cc=raj.khem@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).