* [PATCH] ldconfig: handle .dynstr located in separate segment (bug 25087)
@ 2019-10-10 9:31 Andreas Schwab
2019-10-16 10:34 ` Florian Weimer
0 siblings, 1 reply; 4+ messages in thread
From: Andreas Schwab @ 2019-10-10 9:31 UTC (permalink / raw)
To: libc-alpha
To determine the load offset of the DT_STRTAB section search for the
segment containing it, instead of using the load offset of the first
segment.
[BZ #25087]
* elf/readelflib.c (process_elf_file): Use containing segment for
DT_STRTAB load offset.
---
elf/readelflib.c | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/elf/readelflib.c b/elf/readelflib.c
index 09f5858426..23a045a582 100644
--- a/elf/readelflib.c
+++ b/elf/readelflib.c
@@ -45,7 +45,6 @@ process_elf_file (const char *file_name, const char *lib, int *flag,
{
int i;
unsigned int j;
- ElfW(Addr) loadaddr;
unsigned int dynamic_addr;
size_t dynamic_size;
char *program_interpreter;
@@ -87,7 +86,6 @@ process_elf_file (const char *file_name, const char *lib, int *flag,
libc5/libc6. */
*flag = FLAG_ELF;
- loadaddr = -1;
dynamic_addr = 0;
dynamic_size = 0;
program_interpreter = NULL;
@@ -98,11 +96,6 @@ process_elf_file (const char *file_name, const char *lib, int *flag,
switch (segment->p_type)
{
- case PT_LOAD:
- if (loadaddr == (ElfW(Addr)) -1)
- loadaddr = segment->p_vaddr - segment->p_offset;
- break;
-
case PT_DYNAMIC:
if (dynamic_addr)
error (0, 0, _("more than one dynamic segment\n"));
@@ -176,11 +169,6 @@ process_elf_file (const char *file_name, const char *lib, int *flag,
}
}
- if (loadaddr == (ElfW(Addr)) -1)
- {
- /* Very strange. */
- loadaddr = 0;
- }
/* Now we can read the dynamic sections. */
if (dynamic_size == 0)
@@ -197,7 +185,27 @@ process_elf_file (const char *file_name, const char *lib, int *flag,
check_ptr (dyn_entry);
if (dyn_entry->d_tag == DT_STRTAB)
{
- dynamic_strings = (char *) (file_contents + dyn_entry->d_un.d_val - loadaddr);
+ /* Find the file offset of the segment containing the dynamic
+ string table. */
+ ElfW(Off) loadoff = -1;
+ for (i = 0, segment = elf_pheader;
+ i < elf_header->e_phnum; i++, segment++)
+ {
+ if (segment->p_type == PT_LOAD
+ && dyn_entry->d_un.d_val >= segment->p_vaddr
+ && dyn_entry->d_un.d_val < segment->p_vaddr + segment->p_filesz)
+ {
+ loadoff = segment->p_vaddr - segment->p_offset;
+ break;
+ }
+ }
+ if (loadoff == (ElfW(Off)) -1)
+ {
+ /* Very strange. */
+ loadoff = 0;
+ }
+
+ dynamic_strings = (char *) (file_contents + dyn_entry->d_un.d_val - loadoff);
check_ptr (dynamic_strings);
break;
}
--
2.23.0
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ldconfig: handle .dynstr located in separate segment (bug 25087)
2019-10-10 9:31 [PATCH] ldconfig: handle .dynstr located in separate segment (bug 25087) Andreas Schwab
@ 2019-10-16 10:34 ` Florian Weimer
2019-10-16 13:25 ` Andreas Schwab
0 siblings, 1 reply; 4+ messages in thread
From: Florian Weimer @ 2019-10-16 10:34 UTC (permalink / raw)
To: Andreas Schwab; +Cc: libc-alpha
* Andreas Schwab:
> + && dyn_entry->d_un.d_val < segment->p_vaddr + segment->p_filesz)
> + dynamic_strings = (char *) (file_contents + dyn_entry->d_un.d_val - loadoff);
I think these lines are too long. I also suspect that the condition
should be written as
dyn_entry->d_un.d_val - segment->p_vaddr < segment->p_filesz
But in principle, the change looks fine.
Is it possible to write a test case for bug 25087?
Thanks,
Florian
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ldconfig: handle .dynstr located in separate segment (bug 25087)
2019-10-16 10:34 ` Florian Weimer
@ 2019-10-16 13:25 ` Andreas Schwab
2019-10-16 13:42 ` Florian Weimer
0 siblings, 1 reply; 4+ messages in thread
From: Andreas Schwab @ 2019-10-16 13:25 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On Okt 16 2019, Florian Weimer <fweimer@redhat.com> wrote:
> * Andreas Schwab:
>
>> + && dyn_entry->d_un.d_val < segment->p_vaddr + segment->p_filesz)
>
>> + dynamic_strings = (char *) (file_contents + dyn_entry->d_un.d_val - loadoff);
>
> I think these lines are too long. I also suspect that the condition
> should be written as
>
> dyn_entry->d_un.d_val - segment->p_vaddr < segment->p_filesz
Ok.
> Is it possible to write a test case for bug 25087?
The broken layout is created by patchelf, but I have no idea how to
replicate it without that.
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ldconfig: handle .dynstr located in separate segment (bug 25087)
2019-10-16 13:25 ` Andreas Schwab
@ 2019-10-16 13:42 ` Florian Weimer
0 siblings, 0 replies; 4+ messages in thread
From: Florian Weimer @ 2019-10-16 13:42 UTC (permalink / raw)
To: Andreas Schwab; +Cc: libc-alpha
* Andreas Schwab:
> On Okt 16 2019, Florian Weimer <fweimer@redhat.com> wrote:
>
>> * Andreas Schwab:
>>
>>> + && dyn_entry->d_un.d_val < segment->p_vaddr + segment->p_filesz)
>>
>>> + dynamic_strings = (char *) (file_contents + dyn_entry->d_un.d_val - loadoff);
>>
>> I think these lines are too long. I also suspect that the condition
>> should be written as
>>
>> dyn_entry->d_un.d_val - segment->p_vaddr < segment->p_filesz
>
> Ok.
>
>> Is it possible to write a test case for bug 25087?
>
> The broken layout is created by patchelf, but I have no idea how to
> replicate it without that.
I guess in this case, checking this in without a regression test is
fine.
Thanks,
Florian
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-10-16 13:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-10 9:31 [PATCH] ldconfig: handle .dynstr located in separate segment (bug 25087) Andreas Schwab
2019-10-16 10:34 ` Florian Weimer
2019-10-16 13:25 ` Andreas Schwab
2019-10-16 13:42 ` Florian Weimer
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).