Re: vdso handling

[Alan Modra <amodra@gmail.com>]

On Fri, Mar 28, 2014 at 01:37:52PM +0000, Pedro Alves wrote:
> Hmm.  Indeed.  With current mainline, and with your patch as is,
> the command still fails for me.  In fact, it turns out
> exactly related to p_align vs page size.
> 
> $ cat /proc/30669/maps | grep ncurses
> 324d000000-324d023000 r-xp 00000000 fd:01 315662                         /usr/lib64/libncurses.so.5.9
> 324d023000-324d222000 ---p 00023000 fd:01 315662                         /usr/lib64/libncurses.so.5.9
> 324d222000-324d223000 r--p 00022000 fd:01 315662                         /usr/lib64/libncurses.so.5.9
> 324d223000-324d224000 rw-p 00023000 fd:01 315662                         /usr/lib64/libncurses.so.5.9
> 
> So when trying to read the second PT_LOAD with p_vmaddr 324d222cf8
> and p_vmaddr+p_filesz 324d2236b4, (the 3rd and 4th region above),
> we'd end up reading from 324d200000 to 324d2236b4:
> 
> (top-gdb) p /x loadbase + vaddr
> $5 = 0x324d200000
> (top-gdb) p /x end
> $6 = 0x236b4
> (top-gdb) p /x loadbase + vaddr + end
> $8 = 0x324d2236b4
> 
> which fails as it hits the (324d023000-324d222000) region,
> which has no permissions.

Ah ha!  What's more, if the read did happen to succeed you'd overwrite
contents written when processing the first PT_LOAD.  I believe that
will still happen with your fixup patch.  Not that it's a problem,
since ld.so reads the page holding the end of the first PT_LOAD and
the beginning of the second PT_LOAD twice, but I think it would be
better if BFD didn't rely on this ld.so behaviour (and an exact match
between BFD and ld.so's page size).

I believe the intent of rounding to a page was to pick up the file
and program headers at the start of a file and section headers at the
end, so let's do just that.  On top of my last patch:

diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 31f67a8..a005948 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -1612,7 +1612,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   Elf_External_Ehdr x_ehdr;	/* Elf file header, external form */
   Elf_Internal_Ehdr i_ehdr;	/* Elf file header, internal form */
   Elf_External_Phdr *x_phdrs;
-  Elf_Internal_Phdr *i_phdrs, *last_phdr;
+  Elf_Internal_Phdr *i_phdrs, *last_phdr, *first_phdr;
   bfd *nbfd;
   struct bfd_in_memory *bim;
   bfd_byte *contents;
@@ -1621,7 +1621,6 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   bfd_vma high_offset;
   bfd_vma shdr_end;
   bfd_vma loadbase;
-  bfd_boolean loadbase_set;
 
   /* Read in the ELF header in external format.  */
   err = target_read_memory (ehdr_vma, (bfd_byte *) &x_ehdr, sizeof x_ehdr);
@@ -1694,9 +1693,9 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   i_phdrs = (Elf_Internal_Phdr *) &x_phdrs[i_ehdr.e_phnum];
 
   high_offset = 0;
-  last_phdr = NULL;
   loadbase = 0;
-  loadbase_set = FALSE;
+  first_phdr = NULL;
+  last_phdr = NULL;
   for (i = 0; i < i_ehdr.e_phnum; ++i)
     {
       elf_swap_phdr_in (templ, &x_phdrs[i], &i_phdrs[i]);
@@ -1712,7 +1711,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 
 	  /* If this program header covers offset zero, where the file
 	     header sits, then we can figure out the loadbase.  */
-	  if (!loadbase_set)
+	  if (first_phdr == NULL)
 	    {
 	      bfd_vma p_offset = i_phdrs[i].p_offset;
 	      bfd_vma p_vaddr = i_phdrs[i].p_vaddr;
@@ -1725,7 +1724,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 	      if (p_offset == 0)
 		{
 		  loadbase = ehdr_vma - p_vaddr;
-		  loadbase_set = TRUE;
+		  first_phdr = &i_phdrs[i];
 		}
 	    }
 	}
@@ -1784,13 +1783,15 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 	bfd_vma end = start + i_phdrs[i].p_filesz;
 	bfd_vma vaddr = i_phdrs[i].p_vaddr;
 
-	if (i_phdrs[i].p_align > 1)
+	/* Extend the beginning of the first pt_load to cover file
+	   header and program headers.  */
+	if (first_phdr == &i_phdrs[i])
 	  {
-	    start &= -i_phdrs[i].p_align;
-	    end = (end + i_phdrs[i].p_align - 1) & -i_phdrs[i].p_align;
-	    vaddr &= -i_phdrs[i].p_align;
+	    vaddr -= start;
+	    start = 0;
 	  }
-	if (end > high_offset)
+	/* Extend the end of the last pt_load to cover section headers.  */
+	if (last_phdr == &i_phdrs[i])
 	  end = high_offset;
 	err = target_read_memory (loadbase + vaddr,
 				  contents + start, end - start);

-- 
Alan Modra
Australia Development Lab, IBM