2021-04-23 20:55:15

by Bill Wendling

[permalink] [raw]
Subject: [PATCH] arm64/vdso: Discard .note.gnu.property sections in vDSO

The arm64 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 the kernel linker script only contains a single
NOTE segment:

PHDRS
{
text PT_LOAD FLAGS(5) FILEHDR PHDRS; /* PF_R|PF_X */
dynamic PT_DYNAMIC FLAGS(4); /* PF_R */
note PT_NOTE FLAGS(4); /* PF_R */
}

The NOTE segment generated by the vDSO linker script is aligned to 4 bytes.
But the .note.gnu.property section must be aligned to 8 bytes on arm64.

$ readelf -n vdso64.so

Displaying notes found in: .note
Owner Data size Description
Linux 0x00000004 Unknown note type: (0x00000000)
description data: 06 00 00 00
readelf: Warning: note with invalid namesz and/or descsz found at offset 0x20
readelf: Warning: type: 0x78, namesize: 0x00000100, descsize: 0x756e694c, alignment: 8

Since the note.gnu.property section in the vDSO is not checked by the
dynamic linker, discard the .note.gnu.property sections in the vDSO.

Similar to commit 4caffe6a28d31 ("x86/vdso: Discard .note.gnu.property
sections in vDSO"), but for arm64.

Signed-off-by: Bill Wendling <[email protected]>
---
arch/arm64/kernel/vdso/vdso.lds.S | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
index 61dbb4c838ef..a5e61e09ea92 100644
--- a/arch/arm64/kernel/vdso/vdso.lds.S
+++ b/arch/arm64/kernel/vdso/vdso.lds.S
@@ -31,6 +31,13 @@ SECTIONS
.gnu.version_d : { *(.gnu.version_d) }
.gnu.version_r : { *(.gnu.version_r) }

+ /*
+ * Discard .note.gnu.property sections which are unused and have
+ * different alignment requirement from vDSO note sections.
+ */
+ /DISCARD/ : {
+ *(.note.GNU-stack .note.gnu.property)
+ }
.note : { *(.note.*) } :text :note

. = ALIGN(16);
@@ -48,7 +55,6 @@ SECTIONS
PROVIDE(end = .);

/DISCARD/ : {
- *(.note.GNU-stack)
*(.data .data.* .gnu.linkonce.d.* .sdata*)
*(.bss .sbss .dynbss .dynsbss)
*(.eh_frame .eh_frame_hdr)
--
2.31.1.498.g6c1eba8ee3d-goog


2021-04-27 23:00:20

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH] arm64/vdso: Discard .note.gnu.property sections in vDSO

On Fri, Apr 23, 2021 at 01:51:59PM -0700, Bill Wendling wrote:
> The arm64 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 the kernel linker script only contains a single
> NOTE segment:
>
> PHDRS
> {
> text PT_LOAD FLAGS(5) FILEHDR PHDRS; /* PF_R|PF_X */
> dynamic PT_DYNAMIC FLAGS(4); /* PF_R */
> note PT_NOTE FLAGS(4); /* PF_R */
> }
>
> The NOTE segment generated by the vDSO linker script is aligned to 4 bytes.
> But the .note.gnu.property section must be aligned to 8 bytes on arm64.
>
> $ readelf -n vdso64.so
>
> Displaying notes found in: .note
> Owner Data size Description
> Linux 0x00000004 Unknown note type: (0x00000000)
> description data: 06 00 00 00
> readelf: Warning: note with invalid namesz and/or descsz found at offset 0x20
> readelf: Warning: type: 0x78, namesize: 0x00000100, descsize: 0x756e694c, alignment: 8
>
> Since the note.gnu.property section in the vDSO is not checked by the
> dynamic linker, discard the .note.gnu.property sections in the vDSO.
>
> Similar to commit 4caffe6a28d31 ("x86/vdso: Discard .note.gnu.property
> sections in vDSO"), but for arm64.
>
> Signed-off-by: Bill Wendling <[email protected]>

Seems good to me. If we ever need the BTI markings, etc, for the vDSO,
we can revisit it then.

Reviewed-by: Kees Cook <[email protected]>

-Kees

--
Kees Cook

2021-04-28 08:53:33

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH] arm64/vdso: Discard .note.gnu.property sections in vDSO

On Wed, 28 Apr 2021 at 00:58, Kees Cook <[email protected]> wrote:
>
> On Fri, Apr 23, 2021 at 01:51:59PM -0700, Bill Wendling wrote:
> > The arm64 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 the kernel linker script only contains a single
> > NOTE segment:
> >
> > PHDRS
> > {
> > text PT_LOAD FLAGS(5) FILEHDR PHDRS; /* PF_R|PF_X */
> > dynamic PT_DYNAMIC FLAGS(4); /* PF_R */
> > note PT_NOTE FLAGS(4); /* PF_R */
> > }
> >
> > The NOTE segment generated by the vDSO linker script is aligned to 4 bytes.
> > But the .note.gnu.property section must be aligned to 8 bytes on arm64.
> >
> > $ readelf -n vdso64.so
> >
> > Displaying notes found in: .note
> > Owner Data size Description
> > Linux 0x00000004 Unknown note type: (0x00000000)
> > description data: 06 00 00 00
> > readelf: Warning: note with invalid namesz and/or descsz found at offset 0x20
> > readelf: Warning: type: 0x78, namesize: 0x00000100, descsize: 0x756e694c, alignment: 8
> >
> > Since the note.gnu.property section in the vDSO is not checked by the
> > dynamic linker, discard the .note.gnu.property sections in the vDSO.
> >
> > Similar to commit 4caffe6a28d31 ("x86/vdso: Discard .note.gnu.property
> > sections in vDSO"), but for arm64.
> >
> > Signed-off-by: Bill Wendling <[email protected]>
>
> Seems good to me. If we ever need the BTI markings, etc, for the vDSO,
> we can revisit it then.
>
> Reviewed-by: Kees Cook <[email protected]>
>

Acked-by: Ard Biesheuvel <[email protected]>

2021-04-28 20:51:40

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH] arm64/vdso: Discard .note.gnu.property sections in vDSO

On Fri, Apr 23, 2021 at 01:51:59PM -0700, Bill Wendling wrote:
> The arm64 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 the kernel linker script only contains a single
> NOTE segment:
>
> PHDRS
> {
> text PT_LOAD FLAGS(5) FILEHDR PHDRS; /* PF_R|PF_X */
> dynamic PT_DYNAMIC FLAGS(4); /* PF_R */
> note PT_NOTE FLAGS(4); /* PF_R */
> }
>
> The NOTE segment generated by the vDSO linker script is aligned to 4 bytes.
> But the .note.gnu.property section must be aligned to 8 bytes on arm64.
>
> $ readelf -n vdso64.so
>
> Displaying notes found in: .note
> Owner Data size Description
> Linux 0x00000004 Unknown note type: (0x00000000)
> description data: 06 00 00 00
> readelf: Warning: note with invalid namesz and/or descsz found at offset 0x20
> readelf: Warning: type: 0x78, namesize: 0x00000100, descsize: 0x756e694c, alignment: 8
>
> Since the note.gnu.property section in the vDSO is not checked by the
> dynamic linker, discard the .note.gnu.property sections in the vDSO.
>
> Similar to commit 4caffe6a28d31 ("x86/vdso: Discard .note.gnu.property
> sections in vDSO"), but for arm64.

Can we not instead fix the linker script to preserve the
.note.gnu.property, correctly aligned? It doesn't take much space and
while we don't use it now, it has the BTI information about the binary.

Cc'ing a few others who were involved in the BTI support.

> Signed-off-by: Bill Wendling <[email protected]>
> ---
> arch/arm64/kernel/vdso/vdso.lds.S | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
> index 61dbb4c838ef..a5e61e09ea92 100644
> --- a/arch/arm64/kernel/vdso/vdso.lds.S
> +++ b/arch/arm64/kernel/vdso/vdso.lds.S
> @@ -31,6 +31,13 @@ SECTIONS
> .gnu.version_d : { *(.gnu.version_d) }
> .gnu.version_r : { *(.gnu.version_r) }
>
> + /*
> + * Discard .note.gnu.property sections which are unused and have
> + * different alignment requirement from vDSO note sections.
> + */
> + /DISCARD/ : {
> + *(.note.GNU-stack .note.gnu.property)
> + }
> .note : { *(.note.*) } :text :note
>
> . = ALIGN(16);
> @@ -48,7 +55,6 @@ SECTIONS
> PROVIDE(end = .);
>
> /DISCARD/ : {
> - *(.note.GNU-stack)
> *(.data .data.* .gnu.linkonce.d.* .sdata*)
> *(.bss .sbss .dynbss .dynsbss)
> *(.eh_frame .eh_frame_hdr)
> --
> 2.31.1.498.g6c1eba8ee3d-goog

--
Catalin

2021-04-28 20:52:32

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] arm64/vdso: Discard .note.gnu.property sections in vDSO

On Wed, Apr 28, 2021 at 06:28:47PM +0100, Catalin Marinas wrote:
> On Fri, Apr 23, 2021 at 01:51:59PM -0700, Bill Wendling wrote:

> > Since the note.gnu.property section in the vDSO is not checked by the
> > dynamic linker, discard the .note.gnu.property sections in the vDSO.

> Can we not instead fix the linker script to preserve the
> .note.gnu.property, correctly aligned? It doesn't take much space and
> while we don't use it now, it has the BTI information about the binary.

> Cc'ing a few others who were involved in the BTI support.

Not just BTI, we also flag PAC usage in there too and could add other
extensions going forwards. While the note isn't actively used by
anything right now due to the kernel mapping the vDSO prior to userspace
starting it is part of the ABI and something could end up wanting to use
it and getting confused if it's not there. It would be much better to
fix the alignment issue.


Attachments:
(No filename) (942.00 B)
signature.asc (499.00 B)
Download all attachments

2021-04-28 20:54:54

by Bill Wendling

[permalink] [raw]
Subject: Re: [PATCH] arm64/vdso: Discard .note.gnu.property sections in vDSO

On Wed, Apr 28, 2021 at 12:21 PM Bill Wendling <[email protected]> wrote:
> On Wed, Apr 28, 2021 at 10:40 AM Mark Brown <[email protected]> wrote:
> > On Wed, Apr 28, 2021 at 06:28:47PM +0100, Catalin Marinas wrote:
> > > On Fri, Apr 23, 2021 at 01:51:59PM -0700, Bill Wendling wrote:
> >
> > > > Since the note.gnu.property section in the vDSO is not checked by the
> > > > dynamic linker, discard the .note.gnu.property sections in the vDSO.
> >
> > > Can we not instead fix the linker script to preserve the
> > > .note.gnu.property, correctly aligned? It doesn't take much space and
> > > while we don't use it now, it has the BTI information about the binary.
> >
> > > Cc'ing a few others who were involved in the BTI support.
> >
> > Not just BTI, we also flag PAC usage in there too and could add other
> > extensions going forwards. While the note isn't actively used by
> > anything right now due to the kernel mapping the vDSO prior to userspace
> > starting it is part of the ABI and something could end up wanting to use
> > it and getting confused if it's not there. It would be much better to
> > fix the alignment issue.
>
> If there's only one of the 8-byte aligned sections guaranteed, we
> could place it first in the note. Otherwise, we will have to change
> the alignment of the note (or somehow merge multiple notes).
>
I should have clarified that there's only one *entry* in the
.note.gnu.properties section, and if not then is it possible to merge
multiple entries into one. (Excuse my ignorance if this is already the
case.)

-bw

2021-04-28 20:56:51

by Bill Wendling

[permalink] [raw]
Subject: Re: [PATCH] arm64/vdso: Discard .note.gnu.property sections in vDSO

On Wed, Apr 28, 2021 at 10:40 AM Mark Brown <[email protected]> wrote:
> On Wed, Apr 28, 2021 at 06:28:47PM +0100, Catalin Marinas wrote:
> > On Fri, Apr 23, 2021 at 01:51:59PM -0700, Bill Wendling wrote:
>
> > > Since the note.gnu.property section in the vDSO is not checked by the
> > > dynamic linker, discard the .note.gnu.property sections in the vDSO.
>
> > Can we not instead fix the linker script to preserve the
> > .note.gnu.property, correctly aligned? It doesn't take much space and
> > while we don't use it now, it has the BTI information about the binary.
>
> > Cc'ing a few others who were involved in the BTI support.
>
> Not just BTI, we also flag PAC usage in there too and could add other
> extensions going forwards. While the note isn't actively used by
> anything right now due to the kernel mapping the vDSO prior to userspace
> starting it is part of the ABI and something could end up wanting to use
> it and getting confused if it's not there. It would be much better to
> fix the alignment issue.

If there's only one of the 8-byte aligned sections guaranteed, we
could place it first in the note. Otherwise, we will have to change
the alignment of the note (or somehow merge multiple notes).

-bw

2021-04-29 07:57:33

by Szabolcs Nagy

[permalink] [raw]
Subject: Re: [PATCH] arm64/vdso: Discard .note.gnu.property sections in vDSO

The 04/28/2021 12:31, Bill Wendling wrote:
> On Wed, Apr 28, 2021 at 12:21 PM Bill Wendling <[email protected]> wrote:
> > On Wed, Apr 28, 2021 at 10:40 AM Mark Brown <[email protected]> wrote:
> > > On Wed, Apr 28, 2021 at 06:28:47PM +0100, Catalin Marinas wrote:
> > > > On Fri, Apr 23, 2021 at 01:51:59PM -0700, Bill Wendling wrote:
> > >
> > > > > Since the note.gnu.property section in the vDSO is not checked by the
> > > > > dynamic linker, discard the .note.gnu.property sections in the vDSO.
> > >
> > > > Can we not instead fix the linker script to preserve the
> > > > .note.gnu.property, correctly aligned? It doesn't take much space and
> > > > while we don't use it now, it has the BTI information about the binary.
> > >
> > > > Cc'ing a few others who were involved in the BTI support.
> > >
> > > Not just BTI, we also flag PAC usage in there too and could add other
> > > extensions going forwards. While the note isn't actively used by
> > > anything right now due to the kernel mapping the vDSO prior to userspace
> > > starting it is part of the ABI and something could end up wanting to use
> > > it and getting confused if it's not there. It would be much better to
> > > fix the alignment issue.
> >
> > If there's only one of the 8-byte aligned sections guaranteed, we
> > could place it first in the note. Otherwise, we will have to change
> > the alignment of the note (or somehow merge multiple notes).
> >
> I should have clarified that there's only one *entry* in the
> .note.gnu.properties section, and if not then is it possible to merge
> multiple entries into one. (Excuse my ignorance if this is already the
> case.)

.note.gnu.property should go to PT_GNU_PROPERTY and it
should be merged following rules specified in
https://github.com/hjl-tools/linux-abi/wiki/Linux-Extensions-to-gABI
and
https://github.com/ARM-software/abi-aa/blob/master/aaelf64/aaelf64.rst#program-property

it may also be covered by a (8byte aligned) PT_NOTE, but
that's not a requirement on aarch64 (x86 requires it for
compatibility with old dynamic linker, but since the vdso
is handled specially that may not be relevant either).

i don't know how this works in linker scripts.

2021-04-29 09:27:19

by Bill Wendling

[permalink] [raw]
Subject: Re: [PATCH] arm64/vdso: Discard .note.gnu.property sections in vDSO

On Thu, Apr 29, 2021 at 12:55 AM Szabolcs Nagy <[email protected]> wrote:
> The 04/28/2021 12:31, Bill Wendling wrote:
> > On Wed, Apr 28, 2021 at 12:21 PM Bill Wendling <[email protected]> wrote:
> > > On Wed, Apr 28, 2021 at 10:40 AM Mark Brown <[email protected]> wrote:
> > > > On Wed, Apr 28, 2021 at 06:28:47PM +0100, Catalin Marinas wrote:
> > > > > On Fri, Apr 23, 2021 at 01:51:59PM -0700, Bill Wendling wrote:
> > > >
> > > > > > Since the note.gnu.property section in the vDSO is not checked by the
> > > > > > dynamic linker, discard the .note.gnu.property sections in the vDSO.
> > > >
> > > > > Can we not instead fix the linker script to preserve the
> > > > > .note.gnu.property, correctly aligned? It doesn't take much space and
> > > > > while we don't use it now, it has the BTI information about the binary.
> > > >
> > > > > Cc'ing a few others who were involved in the BTI support.
> > > >
> > > > Not just BTI, we also flag PAC usage in there too and could add other
> > > > extensions going forwards. While the note isn't actively used by
> > > > anything right now due to the kernel mapping the vDSO prior to userspace
> > > > starting it is part of the ABI and something could end up wanting to use
> > > > it and getting confused if it's not there. It would be much better to
> > > > fix the alignment issue.
> > >
> > > If there's only one of the 8-byte aligned sections guaranteed, we
> > > could place it first in the note. Otherwise, we will have to change
> > > the alignment of the note (or somehow merge multiple notes).
> > >
> > I should have clarified that there's only one *entry* in the
> > .note.gnu.properties section, and if not then is it possible to merge
> > multiple entries into one. (Excuse my ignorance if this is already the
> > case.)
>
> .note.gnu.property should go to PT_GNU_PROPERTY and it
> should be merged following rules specified in
> https://github.com/hjl-tools/linux-abi/wiki/Linux-Extensions-to-gABI
> and
> https://github.com/ARM-software/abi-aa/blob/master/aaelf64/aaelf64.rst#program-property
>
> it may also be covered by a (8byte aligned) PT_NOTE, but
> that's not a requirement on aarch64 (x86 requires it for
> compatibility with old dynamic linker, but since the vdso
> is handled specially that may not be relevant either).
>
> i don't know how this works in linker scripts.

This is a potential patch. I haven't had time to test it though.
However, it does appear to format the section in the "expected" way,
so one's able to grab the build IDs. Thoughts?

$ objdump -s -j .note ./arch/arm64/kernel/vdso/vdso.so

./arch/arm64/kernel/vdso/vdso.so: file format elf64-little

Contents of section .note:
0288 04000000 10000000 05000000 474e5500 ............GNU.
0298 000000c0 04000000 03000000 00000000 ................
02a8 06000000 04000000 00000000 4c696e75 ............Linu
02b8 78000000 000a0500 06000000 01000000 x...............
02c8 00010000 4c696e75 78000000 00000000 ....Linux.......
02d8 04000000 14000000 03000000 474e5500 ............GNU.
02e8 958db149 af5156cb 45309896 7a53ae8a ...I.QV.E0..zS..
02f8 ef34e95c .4.\

diff --git a/arch/arm64/kernel/vdso/vdso.lds.S
b/arch/arm64/kernel/vdso/vdso.lds.S
index d808ad31e01f..d51e886c6223 100644
--- a/arch/arm64/kernel/vdso/vdso.lds.S
+++ b/arch/arm64/kernel/vdso/vdso.lds.S
@@ -31,7 +31,13 @@ SECTIONS
.gnu.version_d : { *(.gnu.version_d) }
.gnu.version_r : { *(.gnu.version_r) }

- .note : { *(.note.*) } :text :note
+ /*
+ * Add the .note.gnu.property section first, as it's aligned to
+ * 8-bytes, while other notes are aligned to 4-bytes.
+ */
+ . = ALIGN(8);
+
+ .note : { *(.note.gnu.property) *(.note.*) } :text :note

. = ALIGN(16);

2021-04-29 09:52:59

by Szabolcs Nagy

[permalink] [raw]
Subject: Re: [PATCH] arm64/vdso: Discard .note.gnu.property sections in vDSO

The 04/29/2021 02:23, Bill Wendling wrote:
> On Thu, Apr 29, 2021 at 12:55 AM Szabolcs Nagy <[email protected]> wrote:
> > The 04/28/2021 12:31, Bill Wendling wrote:
> > > On Wed, Apr 28, 2021 at 12:21 PM Bill Wendling <[email protected]> wrote:
> > > > On Wed, Apr 28, 2021 at 10:40 AM Mark Brown <[email protected]> wrote:
> > > > > On Wed, Apr 28, 2021 at 06:28:47PM +0100, Catalin Marinas wrote:
> > > > > > On Fri, Apr 23, 2021 at 01:51:59PM -0700, Bill Wendling wrote:
> > > > >
> > > > > > > Since the note.gnu.property section in the vDSO is not checked by the
> > > > > > > dynamic linker, discard the .note.gnu.property sections in the vDSO.
> > > > >
> > > > > > Can we not instead fix the linker script to preserve the
> > > > > > .note.gnu.property, correctly aligned? It doesn't take much space and
> > > > > > while we don't use it now, it has the BTI information about the binary.
> > > > >
> > > > > > Cc'ing a few others who were involved in the BTI support.
> > > > >
> > > > > Not just BTI, we also flag PAC usage in there too and could add other
> > > > > extensions going forwards. While the note isn't actively used by
> > > > > anything right now due to the kernel mapping the vDSO prior to userspace
> > > > > starting it is part of the ABI and something could end up wanting to use
> > > > > it and getting confused if it's not there. It would be much better to
> > > > > fix the alignment issue.
> > > >
> > > > If there's only one of the 8-byte aligned sections guaranteed, we
> > > > could place it first in the note. Otherwise, we will have to change
> > > > the alignment of the note (or somehow merge multiple notes).
> > > >
> > > I should have clarified that there's only one *entry* in the
> > > .note.gnu.properties section, and if not then is it possible to merge
> > > multiple entries into one. (Excuse my ignorance if this is already the
> > > case.)
> >
> > .note.gnu.property should go to PT_GNU_PROPERTY and it
> > should be merged following rules specified in
> > https://github.com/hjl-tools/linux-abi/wiki/Linux-Extensions-to-gABI
> > and
> > https://github.com/ARM-software/abi-aa/blob/master/aaelf64/aaelf64.rst#program-property
> >
> > it may also be covered by a (8byte aligned) PT_NOTE, but
> > that's not a requirement on aarch64 (x86 requires it for
> > compatibility with old dynamic linker, but since the vdso
> > is handled specially that may not be relevant either).
> >
> > i don't know how this works in linker scripts.
>
> This is a potential patch. I haven't had time to test it though.
> However, it does appear to format the section in the "expected" way,
> so one's able to grab the build IDs. Thoughts?


please show the program headers.


>
> $ objdump -s -j .note ./arch/arm64/kernel/vdso/vdso.so
>
> ./arch/arm64/kernel/vdso/vdso.so: file format elf64-little
>
> Contents of section .note:
> 0288 04000000 10000000 05000000 474e5500 ............GNU.
> 0298 000000c0 04000000 03000000 00000000 ................
> 02a8 06000000 04000000 00000000 4c696e75 ............Linu
> 02b8 78000000 000a0500 06000000 01000000 x...............
> 02c8 00010000 4c696e75 78000000 00000000 ....Linux.......
> 02d8 04000000 14000000 03000000 474e5500 ............GNU.
> 02e8 958db149 af5156cb 45309896 7a53ae8a ...I.QV.E0..zS..
> 02f8 ef34e95c .4.\
>
> diff --git a/arch/arm64/kernel/vdso/vdso.lds.S
> b/arch/arm64/kernel/vdso/vdso.lds.S
> index d808ad31e01f..d51e886c6223 100644
> --- a/arch/arm64/kernel/vdso/vdso.lds.S
> +++ b/arch/arm64/kernel/vdso/vdso.lds.S
> @@ -31,7 +31,13 @@ SECTIONS
> .gnu.version_d : { *(.gnu.version_d) }
> .gnu.version_r : { *(.gnu.version_r) }
>
> - .note : { *(.note.*) } :text :note
> + /*
> + * Add the .note.gnu.property section first, as it's aligned to
> + * 8-bytes, while other notes are aligned to 4-bytes.
> + */
> + . = ALIGN(8);
> +
> + .note : { *(.note.gnu.property) *(.note.*) } :text :note
>
> . = ALIGN(16);

--

2021-04-29 18:53:30

by Bill Wendling

[permalink] [raw]
Subject: Re: [PATCH] arm64/vdso: Discard .note.gnu.property sections in vDSO

On Thu, Apr 29, 2021 at 2:50 AM Szabolcs Nagy <[email protected]> wrote:
> The 04/29/2021 02:23, Bill Wendling wrote:
> > On Thu, Apr 29, 2021 at 12:55 AM Szabolcs Nagy <[email protected]> wrote:
> > > The 04/28/2021 12:31, Bill Wendling wrote:
> > > > On Wed, Apr 28, 2021 at 12:21 PM Bill Wendling <[email protected]> wrote:
> > > > > On Wed, Apr 28, 2021 at 10:40 AM Mark Brown <[email protected]> wrote:
> > > > > > On Wed, Apr 28, 2021 at 06:28:47PM +0100, Catalin Marinas wrote:
> > > > > > > On Fri, Apr 23, 2021 at 01:51:59PM -0700, Bill Wendling wrote:
> > > > > >
> > > > > > > > Since the note.gnu.property section in the vDSO is not checked by the
> > > > > > > > dynamic linker, discard the .note.gnu.property sections in the vDSO.
> > > > > >
> > > > > > > Can we not instead fix the linker script to preserve the
> > > > > > > .note.gnu.property, correctly aligned? It doesn't take much space and
> > > > > > > while we don't use it now, it has the BTI information about the binary.
> > > > > >
> > > > > > > Cc'ing a few others who were involved in the BTI support.
> > > > > >
> > > > > > Not just BTI, we also flag PAC usage in there too and could add other
> > > > > > extensions going forwards. While the note isn't actively used by
> > > > > > anything right now due to the kernel mapping the vDSO prior to userspace
> > > > > > starting it is part of the ABI and something could end up wanting to use
> > > > > > it and getting confused if it's not there. It would be much better to
> > > > > > fix the alignment issue.
> > > > >
> > > > > If there's only one of the 8-byte aligned sections guaranteed, we
> > > > > could place it first in the note. Otherwise, we will have to change
> > > > > the alignment of the note (or somehow merge multiple notes).
> > > > >
> > > > I should have clarified that there's only one *entry* in the
> > > > .note.gnu.properties section, and if not then is it possible to merge
> > > > multiple entries into one. (Excuse my ignorance if this is already the
> > > > case.)
> > >
> > > .note.gnu.property should go to PT_GNU_PROPERTY and it
> > > should be merged following rules specified in
> > > https://github.com/hjl-tools/linux-abi/wiki/Linux-Extensions-to-gABI
> > > and
> > > https://github.com/ARM-software/abi-aa/blob/master/aaelf64/aaelf64.rst#program-property
> > >
> > > it may also be covered by a (8byte aligned) PT_NOTE, but
> > > that's not a requirement on aarch64 (x86 requires it for
> > > compatibility with old dynamic linker, but since the vdso
> > > is handled specially that may not be relevant either).
> > >
> > > i don't know how this works in linker scripts.
> >
> > This is a potential patch. I haven't had time to test it though.
> > However, it does appear to format the section in the "expected" way,
> > so one's able to grab the build IDs. Thoughts?
>
>
> please show the program headers.
>

$ readelf -lW arch/arm64/kernel/vdso/vdso.so

Elf file type is DYN (Shared object file)
Entry point 0x300
There are 4 program headers, starting at offset 64

Program Headers:
Type Offset VirtAddr PhysAddr
FileSiz MemSiz Flg Align
LOAD 0x000000 0x0000000000000000 0x0000000000000000
0x000968 0x000968 R E 0x8
DYNAMIC 0x0008a8 0x00000000000008a8 0x00000000000008a8
0x0000c0 0x0000c0 R 0x8
NOTE 0x000288 0x0000000000000288 0x0000000000000288
0x000074 0x000074 R 0x8
GNU_EH_FRAME 0x0008a4 0x00000000000008a4 0x00000000000008a4
0x000000 0x000000 R E 0x1

Section to Segment mapping:
Segment Sections...
00 .hash .dynsym .dynstr .gnu.version .gnu.version_d .note
.text .eh_frame_hdr .eh_frame .dynamic
01 .dynamic
02 .note
03 .eh_frame_hdr .eh_frame

> >
> > $ objdump -s -j .note ./arch/arm64/kernel/vdso/vdso.so
> >
> > ./arch/arm64/kernel/vdso/vdso.so: file format elf64-little
> >
> > Contents of section .note:
> > 0288 04000000 10000000 05000000 474e5500 ............GNU.
> > 0298 000000c0 04000000 03000000 00000000 ................
> > 02a8 06000000 04000000 00000000 4c696e75 ............Linu
> > 02b8 78000000 000a0500 06000000 01000000 x...............
> > 02c8 00010000 4c696e75 78000000 00000000 ....Linux.......
> > 02d8 04000000 14000000 03000000 474e5500 ............GNU.
> > 02e8 958db149 af5156cb 45309896 7a53ae8a ...I.QV.E0..zS..
> > 02f8 ef34e95c .4.\
> >
> > diff --git a/arch/arm64/kernel/vdso/vdso.lds.S
> > b/arch/arm64/kernel/vdso/vdso.lds.S
> > index d808ad31e01f..d51e886c6223 100644
> > --- a/arch/arm64/kernel/vdso/vdso.lds.S
> > +++ b/arch/arm64/kernel/vdso/vdso.lds.S
> > @@ -31,7 +31,13 @@ SECTIONS
> > .gnu.version_d : { *(.gnu.version_d) }
> > .gnu.version_r : { *(.gnu.version_r) }
> >
> > - .note : { *(.note.*) } :text :note
> > + /*
> > + * Add the .note.gnu.property section first, as it's aligned to
> > + * 8-bytes, while other notes are aligned to 4-bytes.
> > + */
> > + . = ALIGN(8);
> > +
> > + .note : { *(.note.gnu.property) *(.note.*) } :text :note
> >
> > . = ALIGN(16);
>
> --

2021-04-30 08:18:20

by Szabolcs Nagy

[permalink] [raw]
Subject: Re: [PATCH] arm64/vdso: Discard .note.gnu.property sections in vDSO

The 04/29/2021 11:52, Bill Wendling wrote:
> $ readelf -lW arch/arm64/kernel/vdso/vdso.so
>
> Elf file type is DYN (Shared object file)
> Entry point 0x300
> There are 4 program headers, starting at offset 64
>
> Program Headers:
> Type Offset VirtAddr PhysAddr
> FileSiz MemSiz Flg Align
> LOAD 0x000000 0x0000000000000000 0x0000000000000000
> 0x000968 0x000968 R E 0x8
> DYNAMIC 0x0008a8 0x00000000000008a8 0x00000000000008a8
> 0x0000c0 0x0000c0 R 0x8
> NOTE 0x000288 0x0000000000000288 0x0000000000000288
> 0x000074 0x000074 R 0x8
> GNU_EH_FRAME 0x0008a4 0x00000000000008a4 0x00000000000008a4
> 0x000000 0x000000 R E 0x1

this is wrong because the abi specifically says properties
are in PT_GNU_PROPERTY, not in PT_NOTE.

so the original patch that removes them from notes is fine,
but ideally they should be readded under PT_GNU_PROPERTY.

(x86 needs it there too, but also needs it under PT_NOTE for
historical reasons. ideally the section would have been named
other than .note and was not marked as SHF_NOTE, because
properties are merged completely differently by the linker
than notes, so now linkers have to special case it by section
name. properties should have been designed like the
.ARM.attributes section except that is not in a load segment
and thus has no runtime semantics while the key feature of
properties is the runtime semantics. however accessing the
program headers of the vdso is not trivial at runtime from
user code so dropping the properties might work too, but can
bite us in the future depending on how they evolve: the ld.so
can look at vdso program headers.)

2021-04-30 08:33:04

by Bill Wendling

[permalink] [raw]
Subject: Re: [PATCH] arm64/vdso: Discard .note.gnu.property sections in vDSO

On Fri, Apr 30, 2021 at 1:17 AM Szabolcs Nagy <[email protected]> wrote:
> The 04/29/2021 11:52, Bill Wendling wrote:
> > $ readelf -lW arch/arm64/kernel/vdso/vdso.so
> >
> > Elf file type is DYN (Shared object file)
> > Entry point 0x300
> > There are 4 program headers, starting at offset 64
> >
> > Program Headers:
> > Type Offset VirtAddr PhysAddr
> > FileSiz MemSiz Flg Align
> > LOAD 0x000000 0x0000000000000000 0x0000000000000000
> > 0x000968 0x000968 R E 0x8
> > DYNAMIC 0x0008a8 0x00000000000008a8 0x00000000000008a8
> > 0x0000c0 0x0000c0 R 0x8
> > NOTE 0x000288 0x0000000000000288 0x0000000000000288
> > 0x000074 0x000074 R 0x8
> > GNU_EH_FRAME 0x0008a4 0x00000000000008a4 0x00000000000008a4
> > 0x000000 0x000000 R E 0x1
>
> this is wrong because the abi specifically says properties
> are in PT_GNU_PROPERTY, not in PT_NOTE.
>
Right. The issue is more complex than this fix is meant for, to be
honest. That is, it seems that there needs to be a generalized way of
handling the different .note sections.

-bw

> so the original patch that removes them from notes is fine,
> but ideally they should be readded under PT_GNU_PROPERTY.
>
> (x86 needs it there too, but also needs it under PT_NOTE for
> historical reasons. ideally the section would have been named
> other than .note and was not marked as SHF_NOTE, because
> properties are merged completely differently by the linker
> than notes, so now linkers have to special case it by section
> name. properties should have been designed like the
> .ARM.attributes section except that is not in a load segment
> and thus has no runtime semantics while the key feature of
> properties is the runtime semantics. however accessing the
> program headers of the vdso is not trivial at runtime from
> user code so dropping the properties might work too, but can
> bite us in the future depending on how they evolve: the ld.so
> can look at vdso program headers.)

2021-04-30 17:58:41

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH] arm64/vdso: Discard .note.gnu.property sections in vDSO

On Fri, 23 Apr 2021 13:51:59 -0700, Bill Wendling wrote:
> The arm64 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 the kernel linker script only contains a single
> NOTE segment:
>
> PHDRS
> {
> text PT_LOAD FLAGS(5) FILEHDR PHDRS; /* PF_R|PF_X */
> dynamic PT_DYNAMIC FLAGS(4); /* PF_R */
> note PT_NOTE FLAGS(4); /* PF_R */
> }
>
> [...]

Applied to arm64 (for-next/core). Thanks Bill and Szabolcs for
clarifying the need for this patch.

[1/1] arm64/vdso: Discard .note.gnu.property sections in vDSO
https://git.kernel.org/arm64/c/388708028e69

--
Catalin