[patch] Workaround bfd_elf_bfd_from_remote_memory() for some Linux kernels

[patch] Workaround bfd_elf_bfd_from_remote_memory() for some Linux kernels

[Jan Kratochvil]

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

Hi,

on recent x86_64 kernels (at least the Fedora ones; 2.6.23-0.189.rc6.git8.fc8)
GDB starts to print
	warning: no loadable sections found in added symbol-file system-supplied DSO at 0x7fff00f68000
Fortunately it has no real functionality defect as AFAIK the VDSO decoding is
not needed on x86_64.

Unaware if the new kernel VDSO layout (8KB) is present in the upstream kernels.

It cannot find the sections in the kernel VDSO.  New VDSO is 8KB:
	7ffff43fd000-7ffff43ff000 r-xp 7ffff43fd000 00:00 0                      [vdso]

while the older VDSOs (such as linux-2.6.22-rc4-git7.x86_64) were only 4KB:
	ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vdso]

The new VDSO has:
	last segment end < up-aligned(last segment end) < section headers end
	0xb38 < 0x1000 < 0x1008
which leads in the current BFD code to the statement:
	1760        contents_size = last_phdr->p_offset + last_phdr->p_filesz;
as it will set CONTENTS_SIZE to 0xb38 stripping out completely the (already
partially stripped out) section headers.

This patch is wrong as it expects availability of a memory not covereted by the
PT_LOAD segments (nor even their alignment-extended areas).

The functionality of this fix is not dependent on my previous
BFD_ELF_BFD_FROM_REMOTE_MEMORY patches.  Still if the patch
	http://sourceware.org/ml/binutils/2007-08/msg00368.html
for decoding of the symbols content from PHDRs (instead of just the current
SHDRs) would get accepted, the patch attached below makes no longer sense.

The largest part of this patch
	/* Section header string table is usually before the section headers
	   so this check is here has usually no effect.  */
can be safely stripped out as its functionality may never be needed.



Regards,
Jan

[-- Attachment #2: bfd-vdso8k.patch --]
[-- Type: text/plain, Size: 3144 bytes --]

2007-09-23  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* elfcode.h (NAME(_bfd_elf,bfd_from_remote_memory)): New variables
	X_SHDR_SHSTRTAB and I_SHDR_SHSTRTAB.  Fixed the CONTENTS_SIZE trimming
	check for its aligned size between the last segment and still before
	the section header end.  Added variables check to cover also the
	section header string table.

--- ./bfd/elfcode.h	14 Aug 2007 08:04:47 -0000	1.86
+++ ./bfd/elfcode.h	23 Sep 2007 14:24:39 -0000
@@ -1628,6 +1628,8 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   Elf_Internal_Ehdr i_ehdr;	/* Elf file header, internal form */
   Elf_External_Phdr *x_phdrs;
   Elf_Internal_Phdr *i_phdrs, *last_phdr;
+  Elf_External_Shdr *x_shdr_shstrtab;
+  Elf_Internal_Shdr *i_shdr_shstrtab;
   bfd *nbfd;
   struct bfd_in_memory *bim;
   int contents_size;
@@ -1746,19 +1748,49 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 
   /* Trim the last segment so we don't bother with zeros in the last page
      that are off the end of the file.  However, if the extra bit in that
-     page includes the section headers, keep them.  */
-  if ((bfd_vma) contents_size > last_phdr->p_offset + last_phdr->p_filesz
-      && (bfd_vma) contents_size >= (i_ehdr.e_shoff
-				     + i_ehdr.e_shnum * i_ehdr.e_shentsize))
-    {
-      contents_size = last_phdr->p_offset + last_phdr->p_filesz;
-      if ((bfd_vma) contents_size < (i_ehdr.e_shoff
-				     + i_ehdr.e_shnum * i_ehdr.e_shentsize))
-	contents_size = i_ehdr.e_shoff + i_ehdr.e_shnum * i_ehdr.e_shentsize;
-    }
-  else
+     page includes the section headers os the section header string table,
+     keep them.  */
+  if ((bfd_vma) contents_size > last_phdr->p_offset + last_phdr->p_filesz)
     contents_size = last_phdr->p_offset + last_phdr->p_filesz;
 
+  if ((bfd_vma) contents_size < i_ehdr.e_shoff
+				+ i_ehdr.e_shnum * i_ehdr.e_shentsize)
+    contents_size = i_ehdr.e_shoff + i_ehdr.e_shnum * i_ehdr.e_shentsize;
+
+  /* Section header string table is usually before the section headers
+     so this check is here has usually no effect.  */
+  if (i_ehdr.e_shstrndx < i_ehdr.e_shnum)
+    {
+      x_shdr_shstrtab = bfd_malloc (sizeof *x_shdr_shstrtab
+				    + sizeof *i_shdr_shstrtab);
+      if (x_shdr_shstrtab == NULL)
+	{
+	  free (x_phdrs);
+	  bfd_set_error (bfd_error_no_memory);
+	  return NULL;
+	}
+      err = target_read_memory (ehdr_vma + i_ehdr.e_shoff
+				+ i_ehdr.e_shstrndx * sizeof *x_shdr_shstrtab,
+				(bfd_byte *) x_shdr_shstrtab,
+				sizeof *x_shdr_shstrtab);
+      if (err)
+	{
+	  free (x_shdr_shstrtab);
+	  free (x_phdrs);
+	  bfd_set_error (bfd_error_system_call);
+	  errno = err;
+	  return NULL;
+	}
+      i_shdr_shstrtab = (Elf_Internal_Shdr *) &x_shdr_shstrtab[1];
+      elf_swap_shdr_in (templ, x_shdr_shstrtab, i_shdr_shstrtab);
+
+      if ((bfd_vma) contents_size < i_shdr_shstrtab->sh_offset
+				    + i_shdr_shstrtab->sh_size)
+        contents_size = i_shdr_shstrtab->sh_offset + i_shdr_shstrtab->sh_size;
+
+      free (x_shdr_shstrtab);
+    }
+
   /* Now we know the size of the whole image we want read in.  */
   contents = bfd_zmalloc (contents_size);
   if (contents == NULL)

[-- Attachment #3: kernel-2.6.23-0.189.rc6.git8.fc8.x86_64-vdso.bin.gz --]
[-- Type: application/x-gzip, Size: 1649 bytes --]

Re: [patch] Workaround bfd_elf_bfd_from_remote_memory() for some Linux kernels

[Roland McGrath]

I'm not saying anything in particular about your bfd change (I haven't
really analyzed it).  But, the recent change in the size of the x86_64 vdso
image is just a bug in the kernel build (it's not getting stripped).


Thanks,
Roland

Re: [patch] Workaround bfd_elf_bfd_from_remote_memory() for some Linux kernels - cancelled

[Jan Kratochvil]

Hi,

in such case please ignore this patch.  It is incorrect and now it even has no
practical usecase.


Sorry,
Jan


On Mon, 24 Sep 2007 04:54:33 +0200, Roland McGrath wrote:
> I'm not saying anything in particular about your bfd change (I haven't
> really analyzed it).  But, the recent change in the size of the x86_64 vdso
> image is just a bug in the kernel build (it's not getting stripped).
> 
> 
> Thanks,
> Roland