[PATCH] ELF: correct .note.* alignment handling

[Jan Beulich <jbeulich@xxxxxxxx>]

The present uniform 4-byte alignment hasn't been right for, I think, a
very long time (albeit not forever). As per e.g. [1], 8-byte alignment
is required in 64-bit ELF containers (and assembler-generated
.note.gnu.property, for example, is 8-byte aligned, while - oddly -
linker-generated .note.gnu.build-id is only 4-byte aligned [2]). Sadly
libelf is also affected, and hence going strictly by the spec would
break kernels also getting it wrong (e.g. Linux). Apply the same
heuristic as GNU readelf does: If section alignment is 4 or less, assume
only 4-byte padding.

[1] 
https://refspecs.linuxfoundation.org/elf/gabi4+/ch5.pheader.html#note_section
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=33259

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
x86's mkelf32 also isn't dealing right with .note.* sections: Due to the
different padding rules for 32- and 64-bit ELF, what is correct in
xen-syms would need adjustment in xen. Question is whether we care
enough, as long as it's only a cosmetic problem: In the shim, the Xen
note comes first, and that's the only thing that really needs looking at
from the outside, aiui. (Since we actively tail-pad .note.Xen entries,
.note.gnu-build-id ends up correctly placed anyway, despite GNU ld only
aligning it at a 4-byte boundary.)

Arguably the spec text is ambiguous as to the width of namesz, descsz,
and type: They could well be meant to be 8-byte quantities in 64-bit
ELF as per "each entry is an array of 8-byte words". Yet with everyone
using 4-byte fields, that's the defacto standard now anyway.

--- a/xen/arch/x86/include/asm/asm_defns.h
+++ b/xen/arch/x86/include/asm/asm_defns.h
@@ -334,14 +334,14 @@ static always_inline void stac(void)
 
 #define ELFNOTE(name, type, desc)           \
     .pushsection .note.name, "a", @note   ; \
-    .p2align 2                            ; \
+    .balign BYTES_PER_LONG                ; \
     .long 2f - 1f       /* namesz */      ; \
     .long 4f - 3f       /* descsz */      ; \
     .long type          /* type   */      ; \
 1:  .asciz #name        /* name   */      ; \
-2:  .p2align 2                            ; \
+2:  .balign BYTES_PER_LONG                ; \
 3:  desc                /* desc   */      ; \
-4:  .p2align 2                            ; \
+4:  .balign BYTES_PER_LONG                ; \
     .popsection
 
 #define ASM_CONSTANT(name, value)                \
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -583,6 +583,8 @@ elf_errorstatus elf_xen_parse(struct elf
     count = elf_phdr_count(elf);
     for ( i = 0; i < count; i++ )
     {
+        unsigned orig_align = elf->note_align;
+
         phdr = elf_phdr_by_index(elf, i);
         if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(phdr), 1) )
             /* input has an insane program header count field */
@@ -597,10 +599,15 @@ elf_errorstatus elf_xen_parse(struct elf
         if (elf_uval(elf, phdr, p_offset) == 0)
              continue;
 
+        elf->note_align = elf_uval(elf, phdr, p_align);
+
         more_notes = elf_xen_parse_notes(elf, parms,
                                  elf_segment_start(elf, phdr),
                                  elf_segment_end(elf, phdr),
                                  &total_note_count);
+
+        elf->note_align = orig_align;
+
         if ( more_notes == ELF_NOTE_INVALID )
             return -1;
 
@@ -616,6 +623,8 @@ elf_errorstatus elf_xen_parse(struct elf
         count = elf_shdr_count(elf);
         for ( i = 1; i < count; i++ )
         {
+            unsigned orig_align = elf->note_align;
+
             shdr = elf_shdr_by_index(elf, i);
             if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1) )
                 /* input has an insane section header count field */
@@ -624,11 +633,15 @@ elf_errorstatus elf_xen_parse(struct elf
             if ( elf_uval(elf, shdr, sh_type) != SHT_NOTE )
                 continue;
 
+            elf->note_align = elf_uval(elf, shdr, sh_addralign);
+
             more_notes = elf_xen_parse_notes(elf, parms,
                                      elf_section_start(elf, shdr),
                                      elf_section_end(elf, shdr),
                                      &total_note_count);
 
+            elf->note_align = orig_align;
+
             if ( more_notes == ELF_NOTE_INVALID )
                 return -1;
 
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -72,6 +72,9 @@ elf_errorstatus elf_init(struct elf_bina
         return -1;
     }
 
+    /* Record default note alignment, as per EI_CLASS. */
+    elf->note_align = elf_64bit(elf) ? 8 : 4;
+
     /* Find section string table. */
     section = elf_uval(elf, elf->ehdr, e_shstrndx);
     shdr = elf_shdr_by_index(elf, section);
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -288,6 +288,20 @@ ELF_HANDLE_DECL(elf_sym) elf_sym_by_inde
     return sym;
 }
 
+/*
+ * Notes are special: Formally for a long time the spec has demanded that
+ * 64-bit ELF would have 8-byte padding at respective places.  However, many
+ * producers were never updated, so apply a heuristic GNU readelf also applies:
+ * Take section (or segment) alignment into consideration.  When alignment is 4
+ * or less, assume only 4-byte padding.
+ */
+static unsigned elf_note_round_up(const struct elf_binary *elf, unsigned pos)
+{
+    unsigned align = elf_32bit(elf) || elf->note_align <= 4 ? 4 : 8;
+
+    return (pos + align - 1) & ~(align - 1);
+}
+
 const char *elf_note_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) 
note)
 {
     return elf_strval(elf, ELF_HANDLE_PTRVAL(note) + elf_size(elf, note));
@@ -295,9 +309,9 @@ const char *elf_note_name(struct elf_bin
 
 elf_ptrval elf_note_desc(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) 
note)
 {
-    unsigned namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
-
-    return ELF_HANDLE_PTRVAL(note) + elf_size(elf, note) + namesz;
+    return ELF_HANDLE_PTRVAL(note) +
+           elf_note_round_up(elf,
+                             elf_size(elf, note) + elf_uval(elf, note, 
namesz));
 }
 
 uint64_t elf_note_numeric(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) 
note)
@@ -339,11 +353,9 @@ uint64_t elf_note_numeric_array(struct e
 
 ELF_HANDLE_DECL(elf_note) elf_note_next(struct elf_binary *elf, 
ELF_HANDLE_DECL(elf_note) note)
 {
-    unsigned namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
-    unsigned descsz = (elf_uval(elf, note, descsz) + 3) & ~3;
-
-    elf_ptrval ptrval = ELF_HANDLE_PTRVAL(note)
-        + elf_size(elf, note) + namesz + descsz;
+    elf_ptrval ptrval =
+        elf_note_desc(elf, note) +
+        elf_note_round_up(elf, elf_uval(elf, note, descsz));
 
     if ( ( ptrval <= ELF_HANDLE_PTRVAL(note) || /* wrapped or stuck */
            !elf_access_ok(elf, ELF_HANDLE_PTRVAL(note), 1) ) )
--- a/xen/include/xen/elf.h
+++ b/xen/include/xen/elf.h
@@ -29,7 +29,7 @@
 
 #include <xen/elfstructs.h>
 
-#define ELFNOTE_ALIGN(_n_) (((_n_)+3)&~3)
+#define ELFNOTE_ALIGN(_n_) ROUNDUP(_n_, BYTES_PER_LONG)
 #define ELFNOTE_NAME(_n_) ((char*)(_n_) + sizeof(*(_n_)))
 #define ELFNOTE_DESC(_n_) (ELFNOTE_NAME(_n_) + ELFNOTE_ALIGN((_n_)->namesz))
 #define ELFNOTE_NEXT(_n_) ((Elf_Note *)(ELFNOTE_DESC(_n_) + 
ELFNOTE_ALIGN((_n_)->descsz)))
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -181,6 +181,12 @@ struct elf_binary {
     char class;
     char data;
 
+    /*
+     * Note alignment is defaulted from EI_CLASS, but overridden by
+     * segment / section alignment.
+     */
+    unsigned note_align;
+
     ELF_HANDLE_DECL(elf_ehdr) ehdr;
     elf_ptrval sec_strtab;
     ELF_HANDLE_DECL(elf_shdr) sym_tab;