[PATCH] arm64: vdso: disable .eh_frame_hdr via /DISCARD/ instead of --no-eh-frame-hdr

[PATCH] arm64: vdso: disable .eh_frame_hdr via /DISCARD/ instead of --no-eh-frame-hdr

[Peter Collingbourne]

Currently with ld.lld we emit an empty .eh_frame_hdr section (and a
corresponding program header) into the vDSO. With ld.bfd the section
is not emitted but the program header is, with p_vaddr set to 0. This
can lead to unwinders attempting to interpret the data at whichever
location the program header happens to point to as an unwind info
header. This happens to be mostly harmless as long as the byte at
that location (interpreted as a version number) has a value other
than 1, causing both libgcc and LLVM libunwind to ignore the section
(in libunwind's case, after printing an error message to stderr),
but it could lead to worse problems if the byte happened to be 1 or
the program header points to non-readable memory (e.g. if the empty
section was placed at a page boundary).

Instead of disabling .eh_frame_hdr via --no-eh-frame-hdr (which
also has the downside of being unsupported by older versions of GNU
binutils), disable it by discarding the section, and stop emitting
the program header that points to it.

I understand that we intend to emit valid unwind info for the vDSO
at some point. Once that happens this patch can be reverted.

Signed-off-by: Peter Collingbourne <pcc@google.com>
Link: https://linux-review.googlesource.com/id/If745fd9cadcb31b4010acbf5693727fe111b0863
---
 arch/arm64/kernel/vdso/Makefile   | 3 +--
 arch/arm64/kernel/vdso/vdso.lds.S | 5 +----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
index a8f8e409e2bf..cd9c3fa25902 100644
--- a/arch/arm64/kernel/vdso/Makefile
+++ b/arch/arm64/kernel/vdso/Makefile
@@ -24,8 +24,7 @@ btildflags-$(CONFIG_ARM64_BTI_KERNEL) += -z force-bti
 # routines, as x86 does (see 6f121e548f83 ("x86, vdso: Reimplement vdso.so
 # preparation in build-time C")).
 ldflags-y := -shared -nostdlib -soname=linux-vdso.so.1 --hash-style=sysv	\
-	     -Bsymbolic $(call ld-option, --no-eh-frame-hdr) --build-id=sha1 -n	\
-	     $(btildflags-y) -T
+	     -Bsymbolic --build-id=sha1 -n $(btildflags-y) -T
 
 ccflags-y := -fno-common -fno-builtin -fno-stack-protector -ffixed-x18
 ccflags-y += -DDISABLE_BRANCH_PROFILING -DBUILD_VDSO
diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
index d808ad31e01f..61dbb4c838ef 100644
--- a/arch/arm64/kernel/vdso/vdso.lds.S
+++ b/arch/arm64/kernel/vdso/vdso.lds.S
@@ -40,9 +40,6 @@ SECTIONS
 	PROVIDE (_etext = .);
 	PROVIDE (etext = .);
 
-	.eh_frame_hdr	: { *(.eh_frame_hdr) }		:text	:eh_frame_hdr
-	.eh_frame	: { KEEP (*(.eh_frame)) }	:text
-
 	.dynamic	: { *(.dynamic) }		:text	:dynamic
 
 	.rodata		: { *(.rodata*) }		:text
@@ -54,6 +51,7 @@ SECTIONS
 		*(.note.GNU-stack)
 		*(.data .data.* .gnu.linkonce.d.* .sdata*)
 		*(.bss .sbss .dynbss .dynsbss)
+		*(.eh_frame .eh_frame_hdr)
 	}
 }
 
@@ -66,7 +64,6 @@ 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 */
-	eh_frame_hdr	PT_GNU_EH_FRAME;
 }
 
 /*
-- 
2.29.2.729.g45daf8777d-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: vdso: disable .eh_frame_hdr via /DISCARD/ instead of --no-eh-frame-hdr

[Ard Biesheuvel]

On Wed, 30 Dec 2020 at 23:20, Peter Collingbourne <pcc@google.com> wrote:
>
> Currently with ld.lld we emit an empty .eh_frame_hdr section (and a
> corresponding program header) into the vDSO. With ld.bfd the section
> is not emitted but the program header is, with p_vaddr set to 0. This
> can lead to unwinders attempting to interpret the data at whichever
> location the program header happens to point to as an unwind info
> header. This happens to be mostly harmless as long as the byte at
> that location (interpreted as a version number) has a value other
> than 1, causing both libgcc and LLVM libunwind to ignore the section
> (in libunwind's case, after printing an error message to stderr),
> but it could lead to worse problems if the byte happened to be 1 or
> the program header points to non-readable memory (e.g. if the empty
> section was placed at a page boundary).
>
> Instead of disabling .eh_frame_hdr via --no-eh-frame-hdr (which
> also has the downside of being unsupported by older versions of GNU
> binutils), disable it by discarding the section, and stop emitting
> the program header that points to it.
>
> I understand that we intend to emit valid unwind info for the vDSO
> at some point. Once that happens this patch can be reverted.
>
> Signed-off-by: Peter Collingbourne <pcc@google.com>
> Link: https://linux-review.googlesource.com/id/If745fd9cadcb31b4010acbf5693727fe111b0863

Acked-by: Ard Biesheuvel <ardb@kernel.org>

although I am slighty concerned that we haven't been able to converge
on a proper fix yet.


> ---
>  arch/arm64/kernel/vdso/Makefile   | 3 +--
>  arch/arm64/kernel/vdso/vdso.lds.S | 5 +----
>  2 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> index a8f8e409e2bf..cd9c3fa25902 100644
> --- a/arch/arm64/kernel/vdso/Makefile
> +++ b/arch/arm64/kernel/vdso/Makefile
> @@ -24,8 +24,7 @@ btildflags-$(CONFIG_ARM64_BTI_KERNEL) += -z force-bti
>  # routines, as x86 does (see 6f121e548f83 ("x86, vdso: Reimplement vdso.so
>  # preparation in build-time C")).
>  ldflags-y := -shared -nostdlib -soname=linux-vdso.so.1 --hash-style=sysv       \
> -            -Bsymbolic $(call ld-option, --no-eh-frame-hdr) --build-id=sha1 -n \
> -            $(btildflags-y) -T
> +            -Bsymbolic --build-id=sha1 -n $(btildflags-y) -T
>
>  ccflags-y := -fno-common -fno-builtin -fno-stack-protector -ffixed-x18
>  ccflags-y += -DDISABLE_BRANCH_PROFILING -DBUILD_VDSO
> diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
> index d808ad31e01f..61dbb4c838ef 100644
> --- a/arch/arm64/kernel/vdso/vdso.lds.S
> +++ b/arch/arm64/kernel/vdso/vdso.lds.S
> @@ -40,9 +40,6 @@ SECTIONS
>         PROVIDE (_etext = .);
>         PROVIDE (etext = .);
>
> -       .eh_frame_hdr   : { *(.eh_frame_hdr) }          :text   :eh_frame_hdr
> -       .eh_frame       : { KEEP (*(.eh_frame)) }       :text
> -
>         .dynamic        : { *(.dynamic) }               :text   :dynamic
>
>         .rodata         : { *(.rodata*) }               :text
> @@ -54,6 +51,7 @@ SECTIONS
>                 *(.note.GNU-stack)
>                 *(.data .data.* .gnu.linkonce.d.* .sdata*)
>                 *(.bss .sbss .dynbss .dynsbss)
> +               *(.eh_frame .eh_frame_hdr)
>         }
>  }
>
> @@ -66,7 +64,6 @@ 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 */
> -       eh_frame_hdr    PT_GNU_EH_FRAME;
>  }
>
>  /*
> --
> 2.29.2.729.g45daf8777d-goog
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: vdso: disable .eh_frame_hdr via /DISCARD/ instead of --no-eh-frame-hdr

[Vincenzo Frascino]

On 12/30/20 10:19 PM, Peter Collingbourne wrote:
> Currently with ld.lld we emit an empty .eh_frame_hdr section (and a
> corresponding program header) into the vDSO. With ld.bfd the section
> is not emitted but the program header is, with p_vaddr set to 0. This
> can lead to unwinders attempting to interpret the data at whichever
> location the program header happens to point to as an unwind info
> header. This happens to be mostly harmless as long as the byte at
> that location (interpreted as a version number) has a value other
> than 1, causing both libgcc and LLVM libunwind to ignore the section
> (in libunwind's case, after printing an error message to stderr),
> but it could lead to worse problems if the byte happened to be 1 or
> the program header points to non-readable memory (e.g. if the empty
> section was placed at a page boundary).
> 
> Instead of disabling .eh_frame_hdr via --no-eh-frame-hdr (which
> also has the downside of being unsupported by older versions of GNU
> binutils), disable it by discarding the section, and stop emitting
> the program header that points to it.
> 
> I understand that we intend to emit valid unwind info for the vDSO
> at some point. Once that happens this patch can be reverted.
> 

Based on this:

Acked-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

> Signed-off-by: Peter Collingbourne <pcc@google.com>
> Link: https://linux-review.googlesource.com/id/If745fd9cadcb31b4010acbf5693727fe111b0863
> ---
>  arch/arm64/kernel/vdso/Makefile   | 3 +--
>  arch/arm64/kernel/vdso/vdso.lds.S | 5 +----
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> index a8f8e409e2bf..cd9c3fa25902 100644
> --- a/arch/arm64/kernel/vdso/Makefile
> +++ b/arch/arm64/kernel/vdso/Makefile
> @@ -24,8 +24,7 @@ btildflags-$(CONFIG_ARM64_BTI_KERNEL) += -z force-bti
>  # routines, as x86 does (see 6f121e548f83 ("x86, vdso: Reimplement vdso.so
>  # preparation in build-time C")).
>  ldflags-y := -shared -nostdlib -soname=linux-vdso.so.1 --hash-style=sysv	\
> -	     -Bsymbolic $(call ld-option, --no-eh-frame-hdr) --build-id=sha1 -n	\
> -	     $(btildflags-y) -T
> +	     -Bsymbolic --build-id=sha1 -n $(btildflags-y) -T
>  
>  ccflags-y := -fno-common -fno-builtin -fno-stack-protector -ffixed-x18
>  ccflags-y += -DDISABLE_BRANCH_PROFILING -DBUILD_VDSO
> diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
> index d808ad31e01f..61dbb4c838ef 100644
> --- a/arch/arm64/kernel/vdso/vdso.lds.S
> +++ b/arch/arm64/kernel/vdso/vdso.lds.S
> @@ -40,9 +40,6 @@ SECTIONS
>  	PROVIDE (_etext = .);
>  	PROVIDE (etext = .);
>  
> -	.eh_frame_hdr	: { *(.eh_frame_hdr) }		:text	:eh_frame_hdr
> -	.eh_frame	: { KEEP (*(.eh_frame)) }	:text
> -
>  	.dynamic	: { *(.dynamic) }		:text	:dynamic
>  
>  	.rodata		: { *(.rodata*) }		:text
> @@ -54,6 +51,7 @@ SECTIONS
>  		*(.note.GNU-stack)
>  		*(.data .data.* .gnu.linkonce.d.* .sdata*)
>  		*(.bss .sbss .dynbss .dynsbss)
> +		*(.eh_frame .eh_frame_hdr)
>  	}
>  }
>  
> @@ -66,7 +64,6 @@ 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 */
> -	eh_frame_hdr	PT_GNU_EH_FRAME;
>  }
>  
>  /*
> 

-- 
Regards,
Vincenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: vdso: disable .eh_frame_hdr via /DISCARD/ instead of --no-eh-frame-hdr

[Catalin Marinas]

On Wed, 30 Dec 2020 14:19:54 -0800, Peter Collingbourne wrote:
> Currently with ld.lld we emit an empty .eh_frame_hdr section (and a
> corresponding program header) into the vDSO. With ld.bfd the section
> is not emitted but the program header is, with p_vaddr set to 0. This
> can lead to unwinders attempting to interpret the data at whichever
> location the program header happens to point to as an unwind info
> header. This happens to be mostly harmless as long as the byte at
> that location (interpreted as a version number) has a value other
> than 1, causing both libgcc and LLVM libunwind to ignore the section
> (in libunwind's case, after printing an error message to stderr),
> but it could lead to worse problems if the byte happened to be 1 or
> the program header points to non-readable memory (e.g. if the empty
> section was placed at a page boundary).
> 
> [...]

Applied to arm64 (for-next/fixes), thanks!

[1/1] arm64: vdso: disable .eh_frame_hdr via /DISCARD/ instead of --no-eh-frame-hdr
      https://git.kernel.org/arm64/c/e2bba5f92354

-- 
Catalin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel