linux-kernel - Re: [PATCH 2/2] Discard .note.gnu.property sections in generic NOTES

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YyimOW229By98Dn7@relinquished.localdomain>
Date:   Mon, 19 Sep 2022 10:26:17 -0700
From:   Omar Sandoval <osandov@...ndov.com>
To:     Christophe Leroy <christophe.leroy@...roup.eu>,
        Mark Brown <broonie@...nel.org>
Cc:     "H.J. Lu" <hjl.tools@...il.com>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        Yu-cheng Yu <yu-cheng.yu@...el.com>,
        Kees Cook <keescook@...omium.org>,
        Arnd Bergmann <arnd@...db.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Paul Mackerras <paulus@...ba.org>,
        "Naveen N . Rao" <naveen.n.rao@...ux.vnet.ibm.com>,
        Borislav Petkov <bp@...e.de>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [PATCH 2/2] Discard .note.gnu.property sections in generic NOTES

On Sat, Sep 17, 2022 at 06:31:20AM +0000, Christophe Leroy wrote:
> 
> 
> Le 16/09/2022 à 21:40, Omar Sandoval a écrit :
> > [Vous ne recevez pas souvent de courriers de osandov@...ndov.com. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ]
> > 
> > On Tue, Apr 28, 2020 at 06:21:05AM -0700, H.J. Lu wrote:
> >> With the command-line option, -mx86-used-note=yes, the x86 assembler
> >> in binutils 2.32 and above generates a program property note in a note
> >> section, .note.gnu.property, to encode used x86 ISAs and features.  But
> >> kernel linker script only contains a single NOTE segment:
> >>
> >> PHDRS {
> >>   text PT_LOAD FLAGS(5);
> >>   data PT_LOAD FLAGS(6);
> >>   percpu PT_LOAD FLAGS(6);
> >>   init PT_LOAD FLAGS(7);
> >>   note PT_NOTE FLAGS(0);
> >> }
> >> SECTIONS
> >> {
> >> ...
> >>   .notes : AT(ADDR(.notes) - 0xffffffff80000000) { __start_notes = .; KEEP(*(.not
> >> e.*)) __stop_notes = .; } :text :note
> >> ...
> >> }
> >>
> >> The NOTE segment generated by kernel linker script is aligned to 4 bytes.
> >> But .note.gnu.property section must be aligned to 8 bytes on x86-64 and
> >> we get
> >>
> >> [hjl@...-skx-1 linux]$ readelf -n vmlinux
> >>
> >> Displaying notes found in: .notes
> >>    Owner                Data size Description
> >>    Xen                  0x00000006 Unknown note type: (0x00000006)
> >>     description data: 6c 69 6e 75 78 00
> >>    Xen                  0x00000004 Unknown note type: (0x00000007)
> >>     description data: 32 2e 36 00
> >>    xen-3.0              0x00000005 Unknown note type: (0x006e6558)
> >>     description data: 08 00 00 00 03
> >> readelf: Warning: note with invalid namesz and/or descsz found at offset 0x50
> >> readelf: Warning:  type: 0xffffffff, namesize: 0x006e6558, descsize:
> >> 0x80000000, alignment: 8
> >> [hjl@...-skx-1 linux]$
> >>
> >> Since note.gnu.property section in kernel image is never used, this patch
> >> discards .note.gnu.property sections in kernel linker script by adding
> >>
> >> /DISCARD/ : {
> >>    *(.note.gnu.property)
> >> }
> >>
> >> before kernel NOTE segment in generic NOTES.
> >>
> >> Signed-off-by: H.J. Lu <hjl.tools@...il.com>
> >> Reviewed-by: Kees Cook <keescook@...omium.org>
> >> ---
> >>   include/asm-generic/vmlinux.lds.h | 7 +++++++
> >>   1 file changed, 7 insertions(+)
> >>
> >> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> >> index 71e387a5fe90..95cd678428f4 100644
> >> --- a/include/asm-generic/vmlinux.lds.h
> >> +++ b/include/asm-generic/vmlinux.lds.h
> >> @@ -833,7 +833,14 @@
> >>   #define TRACEDATA
> >>   #endif
> >>
> >> +/*
> >> + * Discard .note.gnu.property sections which are unused and have
> >> + * different alignment requirement from kernel note sections.
> >> + */
> >>   #define NOTES                                                                \
> >> +     /DISCARD/ : {                                                   \
> >> +             *(.note.gnu.property)                                   \
> >> +     }                                                               \
> >>        .notes : AT(ADDR(.notes) - LOAD_OFFSET) {                       \
> >>                __start_notes = .;                                      \
> >>                KEEP(*(.note.*))                                        \
> >> --
> >> 2.25.4
> >>
> > 
> > Hi, H.J.,
> > 
> > I recently ran into this same .notes corruption when building kernels on
> > Arch Linux.
> > 
> > What ended up happening to this patch? It doesn't appear to have been
> > merged, and I couldn't find any further discussion about it. I'm happy
> > to resend it for you if you need a hand.
> 
> As far as I can see, ARM64 is doing something with that section, see 
> arch/arm64/include/asm/assembler.h
> 
> Instead of discarding that section, would it be enough to force 
> alignment of .notes to 8 bytes ?
> 
> Thanks
> Christophe

Unfortunately, "alignment requirement" here isn't just the starting
alignment of the .notes section; it also refers to internal padding in
the note metadata to keep things aligned. Changing this would break
anyone who parses /sys/kernel/notes (e.g., perf).

Here is a little more context around this mess:

The System V gABI [1] says that the note header and descriptor should be
aligned to 4 bytes for 32-bit files and 8 bytes for 64-bit files.
However, Linux never followed this, and 4-byte alignment is used for
both 32-bit and 64-bit files; see elf(5) [2].

The only exception as of 2022 is
.note.gnu.property/NT_GNU_PROPERTY_TYPE_0, which is defined to follow
the gABI alignment. There was a long thread discussing this back in 2018
with the subject "PT_NOTE alignment, NT_GNU_PROPERTY_TYPE_0, glibc and
gold" [3].

According to the gABI Linux Extensions [4], consumers are now supposed
to use the p_align of the PT_NOTE segment instead of assuming an
alignment.

There are a few issues with this for the kernel:

* The vmlinux linker script squishes together all of the notes sections
  with different alignments without adjusting their internal padding,
  but sets p_align to the maximum required alignment. This is what
  confuses readelf and co: they expect 8-byte alignment, but most of the
  note entries are only padded for 4-byte alignment.
* The vmlinux .notes section is exported as /sys/kernel/notes. This is
  stable ABI and has always had 4-byte alignment; all existing parsers
  assume this.
* /sys/kernel/notes doesn't currently have a way to specify an alternate
  alignment anyways.

My suggestion would be to keep .note.gnu.property in its own section in
vmlinux, or create a new .notes8 section with 8-byte alignment, and
leave .notes and /sys/kernel/notes alone.

I'm not sure what exactly arch/arm64/include/asm/assembler.h is doing
with this file. Perhaps the author, Mark Brown, can clarify?

1: http://www.sco.com/developers/gabi/latest/ch5.pheader.html#note_section
2: https://man7.org/linux/man-pages/man5/elf.5.html#:~:text=Notes%20(Nhdr)
3: https://public-inbox.org/libc-alpha/13a92cb0-a993-f684-9a96-e02e4afb1bef@redhat.com/
4: https://gitlab.com/x86-psABIs/Linux-ABI