diff mbox series

arm64: vdso: Fix ldflags for -Bsymbolic

Message ID 20200428150854.33130-1-vincenzo.frascino@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: vdso: Fix ldflags for -Bsymbolic | expand

Commit Message

Vincenzo Frascino April 28, 2020, 3:08 p.m. UTC
The vDSO library adds -Bsymbolic to the ldflags because when the shared
library is built it adds a flag in the dynamic section of the binary
called DT_SYMBOLIC which alters the dynamic linker's symbol resolution
algorithm to search for references for a symbol inside the library first
and then into the executable.

In the current implementation of the library for arm64 the flag has been
wrongly added to VDSO_LDFLAGS which is never used.

Restore the correct behavior adding the flag to ldflags.

Note: -Bsymbolic has been added for consistency with arm and future
proof.

Fixes: 28b1a824a4f44 ("arm64: vdso: Substitute gettimeofday() with C implementation")
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Reported-by: Geoff Levand <geoff@infradead.org>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 arch/arm64/kernel/vdso/Makefile | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Will Deacon April 28, 2020, 4:25 p.m. UTC | #1
On Tue, Apr 28, 2020 at 04:08:54PM +0100, Vincenzo Frascino wrote:
> The vDSO library adds -Bsymbolic to the ldflags because when the shared
> library is built it adds a flag in the dynamic section of the binary
> called DT_SYMBOLIC which alters the dynamic linker's symbol resolution
> algorithm to search for references for a symbol inside the library first
> and then into the executable.
> 
> In the current implementation of the library for arm64 the flag has been
> wrongly added to VDSO_LDFLAGS which is never used.
> 
> Restore the correct behavior adding the flag to ldflags.
> 
> Note: -Bsymbolic has been added for consistency with arm and future
> proof.
> 
> Fixes: 28b1a824a4f44 ("arm64: vdso: Substitute gettimeofday() with C implementation")
> Cc: Will Deacon <will@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Reported-by: Geoff Levand <geoff@infradead.org>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
>  arch/arm64/kernel/vdso/Makefile | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Thanks. I'll queue this, but I've tweaked the commit message and the comment
so that they make more sense to me (see below)

Will

--->8

Subject: [PATCH] arm64: vdso: Add '-Bsymbolic' to ldflags

Commit 28b1a824a4f44 ("arm64: vdso: Substitute gettimeofday() with C
implementation") introduced an unused 'VDSO_LDFLAGS' variable to the
vdso Makefile, suggesting that we should be passing '-Bsymbolic' to the
linker, as we do when linking the compat vDSO.

Although it's not strictly necessary to pass this flag, it would be
required if we were to add any internal references to the exported
symbols. It's also consistent with how we link the compat vdso so, since
there's no real downside from passing it, add '-Bsymbolic' to the ldflags
for the native vDSO.

Fixes: 28b1a824a4f44 ("arm64: vdso: Substitute gettimeofday() with C implementation")
Reported-by: Geoff Levand <geoff@infradead.org>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Link: https://lore.kernel.org/r/20200428150854.33130-1-vincenzo.frascino@arm.com
Signed-off-by: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/vdso/Makefile | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
index dd2514bb1511..abf61c96edbc 100644
--- a/arch/arm64/kernel/vdso/Makefile
+++ b/arch/arm64/kernel/vdso/Makefile
@@ -17,14 +17,16 @@ obj-vdso := vgettimeofday.o note.o sigreturn.o
 targets := $(obj-vdso) vdso.so vdso.so.dbg
 obj-vdso := $(addprefix $(obj)/, $(obj-vdso))
 
+# -Bsymbolic has been added for consistency with arm, the compat vDSO and
+# potential future proofing if we end up with internal calls to the exported
+# 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 \
-		--build-id -n -T
+		-Bsymbolic --build-id -n -T
 
 ccflags-y := -fno-common -fno-builtin -fno-stack-protector -ffixed-x18
 ccflags-y += -DDISABLE_BRANCH_PROFILING
 
-VDSO_LDFLAGS := -Bsymbolic
-
 CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) -Os
 KBUILD_CFLAGS			+= $(DISABLE_LTO)
 KASAN_SANITIZE			:= n
diff mbox series

Patch

diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
index abb3347a925b..6ba222ad8cc2 100644
--- a/arch/arm64/kernel/vdso/Makefile
+++ b/arch/arm64/kernel/vdso/Makefile
@@ -17,14 +17,17 @@  obj-vdso := vgettimeofday.o note.o sigreturn.o
 targets := $(obj-vdso) vdso.so vdso.so.dbg
 obj-vdso := $(addprefix $(obj)/, $(obj-vdso))
 
+# -Bsymbolic has been added for consistency with arm and future proof.
+# It adds a flag in the dynamic section of the binary called DT_SYMBOLIC
+# which alters the dynamic linker's symbol resolution algorithm to search
+# for references for a symbol inside the library first and then into the
+# executable.
 ldflags-y := -shared -nostdlib -soname=linux-vdso.so.1 --hash-style=sysv \
-		--build-id -n -T
+		-Bsymbolic --build-id -n -T
 
 ccflags-y := -fno-common -fno-builtin -fno-stack-protector -ffixed-x18
 ccflags-y += -DDISABLE_BRANCH_PROFILING
 
-VDSO_LDFLAGS := -Bsymbolic
-
 CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) -Os
 KBUILD_CFLAGS			+= $(DISABLE_LTO)
 KASAN_SANITIZE			:= n