| Message ID | 20201230221954.2007257-1-pcc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: vdso: disable .eh_frame_hdr via /DISCARD/ instead of --no-eh-frame-hdr | expand |
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 >
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; > } > > /* >
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
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; } /*
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(-)