Re: [COMMITTED] libdwfl: Remove p_align sanity check from elf_from_memory.

Re: [COMMITTED] libdwfl: Remove p_align sanity check from elf_from_memory.

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 250 bytes --]

On Fri, 19 Dec 2014 18:13:04 +0100, Mark Wielaard wrote:
> But on some architectures the kernel inserts a vdso with
> a completely bogus p_align for some PT_LOAD segments.

It would be better to know what is "some" and what is "bogus".


Jan

Re: [COMMITTED] libdwfl: Remove p_align sanity check from elf_from_memory.

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 156 bytes --]

On Fri, 19 Dec 2014 19:06:33 +0100, Mark Wielaard wrote:
> At least aarch64 puts 0xA as p_align value in the phdrs.

OK, I agree 0xa is bogus.


Jan

Re: [COMMITTED] libdwfl: Remove p_align sanity check from elf_from_memory.

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

On Fri, Dec 19, 2014 at 06:30:22PM +0100, Jan Kratochvil wrote:
> On Fri, 19 Dec 2014 18:13:04 +0100, Mark Wielaard wrote:
> > But on some architectures the kernel inserts a vdso with
> > a completely bogus p_align for some PT_LOAD segments.
> 
> It would be better to know what is "some" and what is "bogus".

Yes, sorry, I was handling tests on different arches at the time.
And this wasn't the only issue with vdso/elf_from_memory.
At least aarch64 puts 0xA as p_align value in the phdrs.
Normally the glibc dynamic linker would at least sanity check
the p_align value, but then always uses the pagealign anyway.
But the kernel seems to not care about phdr sanity because
the dynamic linker won't touch the vdso ELF image.

Cheers,

Mark

[COMMITTED] libdwfl: Remove p_align sanity check from elf_from_memory.

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 2754 bytes --]

In commit f15bcd "elf_from_remote_memory should use pagesize, not p_align"
we already relaxed the p_align sanity check to allow alignment of the
segment against the pagesize since that is what the glibc dynamic linker
actually does. But on some architectures the kernel inserts a vdso with
a completely bogus p_align for some PT_LOAD segments. So just drop the
whole sanity check and allow anything since we won't use p_align, but
always already use pagesize anyway.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 libdwfl/ChangeLog         |  4 ++++
 libdwfl/elf-from-memory.c | 14 +++++---------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index f6db301..3d3edc5 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,7 @@
+2014-12-19  Mark Wielaard  <mjw@redhat.com>
+
+	* elf-from-memory.c (handle_segment): Remove palign sanity check.
+
 2014-12-18  Mark Wielaard  <mjw@redhat.com>
 
 	* relocate.c (resolve_symbol): Make sure symstrdata->d_buf != NULL.
diff --git a/libdwfl/elf-from-memory.c b/libdwfl/elf-from-memory.c
index df9fbe6..b35fac7 100644
--- a/libdwfl/elf-from-memory.c
+++ b/libdwfl/elf-from-memory.c
@@ -206,12 +206,10 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
 	 found_base yet).  Returns true if sanity checking failed,
 	 false otherwise.  */
       inline bool handle_segment (GElf_Addr vaddr, GElf_Off offset,
-				  GElf_Xword filesz, GElf_Xword memsz,
-				  GElf_Xword palign)
+				  GElf_Xword filesz, GElf_Xword memsz)
 	{
-	  /* Sanity check the alignment requirements.  */
-	  if ((palign & (pagesize - 1)) != 0
-	      || ((vaddr - offset) & (palign - 1)) != 0)
+	  /* Sanity check the segment load aligns with the pagesize.  */
+	  if (((vaddr - offset) & (pagesize - 1)) != 0)
 	    return true;
 
 	  GElf_Off segment_end = ((offset + filesz + pagesize - 1)
@@ -238,8 +236,7 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
       for (uint_fast16_t i = 0; i < phnum; ++i)
 	if (phdrs.p32[i].p_type == PT_LOAD)
 	  if (handle_segment (phdrs.p32[i].p_vaddr, phdrs.p32[i].p_offset,
-			      phdrs.p32[i].p_filesz, phdrs.p32[i].p_memsz,
-			      phdrs.p32[i].p_align))
+			      phdrs.p32[i].p_filesz, phdrs.p32[i].p_memsz))
 	    goto bad_elf;
       break;
 
@@ -250,8 +247,7 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
       for (uint_fast16_t i = 0; i < phnum; ++i)
 	if (phdrs.p64[i].p_type == PT_LOAD)
 	  if (handle_segment (phdrs.p64[i].p_vaddr, phdrs.p64[i].p_offset,
-			      phdrs.p64[i].p_filesz, phdrs.p64[i].p_memsz,
-			      phdrs.p64[i].p_align))
+			      phdrs.p64[i].p_filesz, phdrs.p64[i].p_memsz))
 	    goto bad_elf;
       break;
 
-- 
1.8.3.1