Re: PATCH] Properly handle note sections and segments

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Sat, Nov 25, 2017 at 9:22 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sat, Nov 25, 2017 at 6:17 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, Nov 24, 2017 at 3:49 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> gABI specifies that PT_NOTE alignment should be aligned to 4 bytes
>>> for 32-bit objects and to 8 bytes for 64-bit objects.  But on Linux,
>>> .note.ABI-tag and .note.gnu.build-id notes are always aligned to 4
>>> bytes.  We allow either 4 byte or 8 byte alignments.
>>>
>>> OK for master?
>>>
>>> H.J.
>>> ---
>>>         * readelf.c (process_notes_at): Return FALSE if alignment of
>>>         note section isn't 4 nor 8 bytes.
>>
>> This patch is incorrect.  Here is the updated patch, which also fixes
>> incorrect note section alignment:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=22492
>>
>
> This updated patch also fixed:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=22490
>
> and dumps CORE PT_NOTE segments.
>
> OK for master?

Here is the updated patch against master branch.


-- 
H.J.

From 8a1d35e7c28afe518370c82172f257b7809326c2 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 24 Nov 2017 15:47:10 -0800
Subject: [PATCH] Properly handle note sections and segments

When dumping notes, get note alignment from either note section or note
segment.  To support notes generated by assemblers with

https://sourceware.org/bugzilla/show_bug.cgi?id=22492

we treate alignment as 4 bytes if it is less than 4.  Otherwise, we skip
notes if alignment isn't 4 nor 8 bytes.

We should call load_separate_debug_file only if e_shstrndx != SHN_UNDEF.

	PR binutils/22490
	* readelf.c (process_notes_at): Add an argument for note
	alignment.  If note alignment is less than 4, use 4 byte
	alignment.  Otherwise, skip notes if alignment isn't 4 nor
	8 bytes.
	(process_corefile_note_segments): Pass segment alignment to
	process_notes_at.
	(process_note_sections): Pass section alignment to
	process_notes_at.
	(process_object): Call load_separate_debug_file only if
	e_shstrndx != SHN_UNDEF.
---
 binutils/readelf.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 53896ae647..a1f43e612a 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -17904,13 +17904,13 @@ static bfd_boolean
 process_notes_at (Filedata *           filedata,
 		  Elf_Internal_Shdr *  section,
 		  bfd_vma              offset,
-		  bfd_vma              length)
+		  bfd_vma              length,
+		  bfd_vma              align)
 {
   Elf_External_Note * pnotes;
   Elf_External_Note * external;
   char *              end;
   bfd_boolean         res = TRUE;
-  size_t              align;
 
   if (length <= 0)
     return FALSE;
@@ -17923,21 +17923,11 @@ process_notes_at (Filedata *           filedata,
 	  if (! apply_relocations (filedata, section, (unsigned char *) pnotes, length, NULL, NULL))
 	    return FALSE;
 	}
-      align = section->sh_addralign;
     }
   else
-    {
-      pnotes = (Elf_External_Note *) get_data (NULL, filedata, offset, 1, length,
+    pnotes = (Elf_External_Note *) get_data (NULL, filedata, offset, 1, length,
 					     _("notes"));
 
-      /* FIXME: Core notes seem to be produced with
-	 4-byte alignment even on 64-bit systems.  */
-      if (filedata->file_header.e_type == ET_CORE)
-	align = 4;
-      else
-	align = is_32bit_elf ? 4 : 4;
-    }
-
   if (pnotes == NULL)
     return FALSE;
 
@@ -17949,6 +17939,20 @@ process_notes_at (Filedata *           filedata,
     printf (_("\nDisplaying notes found at file offset 0x%08lx with length 0x%08lx:\n"),
 	    (unsigned long) offset, (unsigned long) length);
 
+  /* NB: Some note sections may have alignment value of 0 or 1.  gABI
+     specifies that notes should be aligned to 4 bytes in 32-bit
+     objects and to 8 bytes in 64-bit objects.  As a Linux extension,
+     we also support 4 byte alignment in 64-bit objects.  If section
+     alignment is less than 4, we treate alignment as 4 bytes.   */
+  if (align < 4)
+    align = 4;
+  else if (align != 4 && align != 8)
+    {
+      warn (_("Corrupt note: alignment %ld, expecting 4 or 8\n"),
+	    (long) align);
+      return FALSE;
+    }
+
   printf (_("  %-20s %10s\tDescription\n"), _("Owner"), _("Data size"));
 
   end = (char *) pnotes + length;
@@ -18087,7 +18091,8 @@ process_corefile_note_segments (Filedata * filedata)
       if (segment->p_type == PT_NOTE)
 	if (! process_notes_at (filedata, NULL,
 				(bfd_vma) segment->p_offset,
-				(bfd_vma) segment->p_filesz))
+				(bfd_vma) segment->p_filesz,
+				(bfd_vma) segment->p_align))
 	  res = FALSE;
     }
 
@@ -18191,7 +18196,8 @@ process_note_sections (Filedata * filedata)
 	{
 	  if (! process_notes_at (filedata, section,
 				  (bfd_vma) section->sh_offset,
-				  (bfd_vma) section->sh_size))
+				  (bfd_vma) section->sh_size,
+				  (bfd_vma) section->sh_addralign))
 	    res = FALSE;
 	  n++;
 	}
@@ -18566,7 +18572,10 @@ process_object (Filedata * filedata)
   if (! process_version_sections (filedata))
     res = FALSE;
 
-  separates = load_separate_debug_file (filedata, filedata->file_name);
+  if (filedata->file_header.e_shstrndx != SHN_UNDEF)
+    separates = load_separate_debug_file (filedata, filedata->file_name);
+  else
+    separates = NULL;
 
   if (! process_section_contents (filedata))
     res = FALSE;
-- 
2.14.3