* [PATCH] arm64: Fix off-by-one vdso trampoline return value
@ 2020-11-12 0:14 Will McVicker
2020-11-12 1:00 ` Nick Desaulniers
2020-11-12 10:12 ` Will Deacon
0 siblings, 2 replies; 7+ messages in thread
From: Will McVicker @ 2020-11-12 0:14 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon
Cc: Nathan Chancellor, Nick Desaulniers, Vincenzo Frascino,
Andrei Vagin, Dmitry Safonov, Thomas Gleixner, linux-arm-kernel,
linux-kernel, clang-built-linux, kernel-team, Will McVicker
Depending on your host nm version, the generated header
`include/generated/vdso32-offsets.h` may have the bottom bit set for the
thumb vdso offset addresses (as observed when using llvm-nm). This
results in an additional +1 for thumb vdso trampoline return values
since compat_setup_return() already includes `vdso_trampoline + thumb`.
As a result, I see a SIGBUS error when running the LTP test
syscalls.rt_sigaction01. To fix this, let's clear the bottom bit of the
vdso_offset in the VDSO_SYMBOL macro.
Test: LTP test syscalls.rt_sigaction01
Fixes: f01703b3d2e6 ("arm64: compat: Get sigreturn trampolines from vDSO")
Signed-off-by: Will McVicker <willmcvicker@google.com>
---
arch/arm64/include/asm/vdso.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h
index f99dcb94b438..a7384379e8e1 100644
--- a/arch/arm64/include/asm/vdso.h
+++ b/arch/arm64/include/asm/vdso.h
@@ -23,7 +23,7 @@
#define VDSO_SYMBOL(base, name) \
({ \
- (void *)(vdso_offset_##name - VDSO_LBASE + (unsigned long)(base)); \
+ (void *)((vdso_offset_##name & ~1UL) - VDSO_LBASE + (unsigned long)(base)); \
})
#endif /* !__ASSEMBLY__ */
--
2.29.2.299.gdc1121823c-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: Fix off-by-one vdso trampoline return value
2020-11-12 0:14 [PATCH] arm64: Fix off-by-one vdso trampoline return value Will McVicker
@ 2020-11-12 1:00 ` Nick Desaulniers
2020-11-12 1:07 ` Nick Desaulniers
2020-11-12 2:14 ` Nick Desaulniers
2020-11-12 10:12 ` Will Deacon
1 sibling, 2 replies; 7+ messages in thread
From: Nick Desaulniers @ 2020-11-12 1:00 UTC (permalink / raw)
To: Will McVicker
Cc: Catalin Marinas, Will Deacon, Nathan Chancellor,
Vincenzo Frascino, Andrei Vagin, Dmitry Safonov, Thomas Gleixner,
Linux ARM, LKML, clang-built-linux, kernel-team
On Wed, Nov 11, 2020 at 4:14 PM Will McVicker <willmcvicker@google.com> wrote:
>
> Depending on your host nm version, the generated header
> `include/generated/vdso32-offsets.h` may have the bottom bit set for the
> thumb vdso offset addresses (as observed when using llvm-nm). This
Sorry, the commit message seems to imply a bug in llvm-nm, but I don't
think that's the case. If it is, please, send us a bugreport.
$ aarch64-linux-gnu-nm arch/arm64/kernel/vdso32/vdso.so.raw | grep thumb
00000968 T __kernel_rt_sigreturn_thumb
00000960 T __kernel_sigreturn_thumb
00000968 t VDSO_compat_rt_sigreturn_thumb
00000960 t VDSO_compat_sigreturn_thumb
$ llvm-nm arch/arm64/kernel/vdso32/vdso.so.raw | grep thumb
00000968 t VDSO_compat_rt_sigreturn_thumb
00000960 t VDSO_compat_sigreturn_thumb
00000968 T __kernel_rt_sigreturn_thumb
00000960 T __kernel_sigreturn_thumb
$ /usr/bin/nm arch/arm64/kernel/vdso32/vdso.so.raw | grep thumb
00000969 T __kernel_rt_sigreturn_thumb
00000961 T __kernel_sigreturn_thumb
00000969 t VDSO_compat_rt_sigreturn_thumb
00000961 t VDSO_compat_sigreturn_thumb
$ /usr/bin/nm --version
GNU nm (GNU Binutils for Debian) 2.35.1
Would you mind amending the commit message to not imply that llvm-nm is broken?
It might be of interest to find out why the host `nm` was invoked,
rather than $(NM)/$(CROSS_COMPILE)nm.
> results in an additional +1 for thumb vdso trampoline return values
> since compat_setup_return() already includes `vdso_trampoline + thumb`.
> As a result, I see a SIGBUS error when running the LTP test
> syscalls.rt_sigaction01. To fix this, let's clear the bottom bit of the
> vdso_offset in the VDSO_SYMBOL macro.
>
> Test: LTP test syscalls.rt_sigaction01
> Fixes: f01703b3d2e6 ("arm64: compat: Get sigreturn trampolines from vDSO")
> Signed-off-by: Will McVicker <willmcvicker@google.com>
> ---
> arch/arm64/include/asm/vdso.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h
> index f99dcb94b438..a7384379e8e1 100644
> --- a/arch/arm64/include/asm/vdso.h
> +++ b/arch/arm64/include/asm/vdso.h
> @@ -23,7 +23,7 @@
>
> #define VDSO_SYMBOL(base, name) \
> ({ \
> - (void *)(vdso_offset_##name - VDSO_LBASE + (unsigned long)(base)); \
> + (void *)((vdso_offset_##name & ~1UL) - VDSO_LBASE + (unsigned long)(base)); \
> })
>
> #endif /* !__ASSEMBLY__ */
> --
> 2.29.2.299.gdc1121823c-goog
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: Fix off-by-one vdso trampoline return value
2020-11-12 1:00 ` Nick Desaulniers
@ 2020-11-12 1:07 ` Nick Desaulniers
2020-11-12 2:14 ` Nick Desaulniers
1 sibling, 0 replies; 7+ messages in thread
From: Nick Desaulniers @ 2020-11-12 1:07 UTC (permalink / raw)
To: Will McVicker
Cc: Catalin Marinas, Will Deacon, Nathan Chancellor,
Vincenzo Frascino, Andrei Vagin, Dmitry Safonov, Thomas Gleixner,
Linux ARM, LKML, clang-built-linux, kernel-team
On Wed, Nov 11, 2020 at 5:00 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, Nov 11, 2020 at 4:14 PM Will McVicker <willmcvicker@google.com> wrote:
> >
> > Depending on your host nm version, the generated header
> > `include/generated/vdso32-offsets.h` may have the bottom bit set for the
> > thumb vdso offset addresses (as observed when using llvm-nm). This
>
> Sorry, the commit message seems to imply a bug in llvm-nm, but I don't
> think that's the case. If it is, please, send us a bugreport.
>
> $ aarch64-linux-gnu-nm arch/arm64/kernel/vdso32/vdso.so.raw | grep thumb
> 00000968 T __kernel_rt_sigreturn_thumb
> 00000960 T __kernel_sigreturn_thumb
> 00000968 t VDSO_compat_rt_sigreturn_thumb
> 00000960 t VDSO_compat_sigreturn_thumb
> $ llvm-nm arch/arm64/kernel/vdso32/vdso.so.raw | grep thumb
> 00000968 t VDSO_compat_rt_sigreturn_thumb
> 00000960 t VDSO_compat_sigreturn_thumb
> 00000968 T __kernel_rt_sigreturn_thumb
> 00000960 T __kernel_sigreturn_thumb
> $ /usr/bin/nm arch/arm64/kernel/vdso32/vdso.so.raw | grep thumb
> 00000969 T __kernel_rt_sigreturn_thumb
> 00000961 T __kernel_sigreturn_thumb
> 00000969 t VDSO_compat_rt_sigreturn_thumb
> 00000961 t VDSO_compat_sigreturn_thumb
> $ /usr/bin/nm --version
> GNU nm (GNU Binutils for Debian) 2.35.1
(Noting that my host's GNU binutils are configured to target x86):
$ /usr/bin/nm -h
...
elf64-x86-64 elf32-i386 elf32-iamcu elf32-x86-64 pei-i386 pei-x86-64
elf64-l1om elf64-k1om elf64-little elf64-big elf32-little elf32-big
pe-x86-64 pe-bigobj-x86-64 pe-i386 srec symbolsrec verilog tekhex
binary ihex plugin
So it would seem when binutils is configured for x86, then it will
mistakenly decode thumb instructions as being off by one.
(Note to no one in particular: verilog? really?)
>
> Would you mind amending the commit message to not imply that llvm-nm is broken?
>
> It might be of interest to find out why the host `nm` was invoked,
> rather than $(NM)/$(CROSS_COMPILE)nm.
>
> > results in an additional +1 for thumb vdso trampoline return values
> > since compat_setup_return() already includes `vdso_trampoline + thumb`.
> > As a result, I see a SIGBUS error when running the LTP test
> > syscalls.rt_sigaction01. To fix this, let's clear the bottom bit of the
> > vdso_offset in the VDSO_SYMBOL macro.
> >
> > Test: LTP test syscalls.rt_sigaction01
> > Fixes: f01703b3d2e6 ("arm64: compat: Get sigreturn trampolines from vDSO")
> > Signed-off-by: Will McVicker <willmcvicker@google.com>
> > ---
> > arch/arm64/include/asm/vdso.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h
> > index f99dcb94b438..a7384379e8e1 100644
> > --- a/arch/arm64/include/asm/vdso.h
> > +++ b/arch/arm64/include/asm/vdso.h
> > @@ -23,7 +23,7 @@
> >
> > #define VDSO_SYMBOL(base, name) \
> > ({ \
> > - (void *)(vdso_offset_##name - VDSO_LBASE + (unsigned long)(base)); \
> > + (void *)((vdso_offset_##name & ~1UL) - VDSO_LBASE + (unsigned long)(base)); \
> > })
> >
> > #endif /* !__ASSEMBLY__ */
> > --
> > 2.29.2.299.gdc1121823c-goog
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: Fix off-by-one vdso trampoline return value
2020-11-12 1:00 ` Nick Desaulniers
2020-11-12 1:07 ` Nick Desaulniers
@ 2020-11-12 2:14 ` Nick Desaulniers
1 sibling, 0 replies; 7+ messages in thread
From: Nick Desaulniers @ 2020-11-12 2:14 UTC (permalink / raw)
To: Will McVicker
Cc: Catalin Marinas, Will Deacon, Nathan Chancellor,
Vincenzo Frascino, Andrei Vagin, Dmitry Safonov, Thomas Gleixner,
Linux ARM, LKML, clang-built-linux, kernel-team
On Wed, Nov 11, 2020 at 5:00 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, Nov 11, 2020 at 4:14 PM Will McVicker <willmcvicker@google.com> wrote:
> >
> > Depending on your host nm version, the generated header
> > `include/generated/vdso32-offsets.h` may have the bottom bit set for the
> > thumb vdso offset addresses (as observed when using llvm-nm). This
>
> Sorry, the commit message seems to imply a bug in llvm-nm, but I don't
> think that's the case. If it is, please, send us a bugreport.
>
> $ aarch64-linux-gnu-nm arch/arm64/kernel/vdso32/vdso.so.raw | grep thumb
> 00000968 T __kernel_rt_sigreturn_thumb
> 00000960 T __kernel_sigreturn_thumb
> 00000968 t VDSO_compat_rt_sigreturn_thumb
> 00000960 t VDSO_compat_sigreturn_thumb
> $ llvm-nm arch/arm64/kernel/vdso32/vdso.so.raw | grep thumb
> 00000968 t VDSO_compat_rt_sigreturn_thumb
> 00000960 t VDSO_compat_sigreturn_thumb
> 00000968 T __kernel_rt_sigreturn_thumb
> 00000960 T __kernel_sigreturn_thumb
> $ /usr/bin/nm arch/arm64/kernel/vdso32/vdso.so.raw | grep thumb
> 00000969 T __kernel_rt_sigreturn_thumb
> 00000961 T __kernel_sigreturn_thumb
> 00000969 t VDSO_compat_rt_sigreturn_thumb
> 00000961 t VDSO_compat_sigreturn_thumb
> $ /usr/bin/nm --version
> GNU nm (GNU Binutils for Debian) 2.35.1
>
> Would you mind amending the commit message to not imply that llvm-nm is broken?
Testing another set of configs:
$ aarch64-linux-android-nm arch/arm64/kernel/vdso32/vdso.so.dbg | grep thumb
00000950 T __kernel_rt_sigreturn_thumb
00000948 T __kernel_sigreturn_thumb
00000951 t VDSO_compat_rt_sigreturn_thumb
00000949 t VDSO_compat_sigreturn_thumb
$ /path/to/older/aarch64-linux-gnu-nm
arch/arm64/kernel/vdso32/vdso.so.dbg | grep thumb
00000950 T __kernel_rt_sigreturn_thumb
00000948 T __kernel_sigreturn_thumb
00000951 t VDSO_compat_rt_sigreturn_thumb
00000949 t VDSO_compat_sigreturn_thumb
$ /usr/bin/nm out/android-4.19-stable/common/arch/arm64/kernel/vdso32/vdso.so.dbg
| grep thumb
00000951 T __kernel_rt_sigreturn_thumb
00000949 T __kernel_sigreturn_thumb
00000951 t VDSO_compat_rt_sigreturn_thumb
00000949 t VDSO_compat_sigreturn_thumb
$ llvm-nm out/android-4.19-stable/common/arch/arm64/kernel/vdso32/vdso.so.dbg
| grep thumb
00000951 t VDSO_compat_rt_sigreturn_thumb
00000949 t VDSO_compat_sigreturn_thumb
00000950 T __kernel_rt_sigreturn_thumb
00000948 T __kernel_sigreturn_thumb
(That llvm-nm sorts the output makes this trickier to follow). But
shows that only host GNU `nm` differs.
>
> It might be of interest to find out why the host `nm` was invoked,
> rather than $(NM)/$(CROSS_COMPILE)nm.
Possibly commit 7b7891c7bdfd ("arm64: vdso32: Fix '--prefix=' value
for newer versions of clang") missing from your tree, but I fail to
see how that would mess up or invoke the incorrect $(NM).
>
> > results in an additional +1 for thumb vdso trampoline return values
> > since compat_setup_return() already includes `vdso_trampoline + thumb`.
> > As a result, I see a SIGBUS error when running the LTP test
> > syscalls.rt_sigaction01. To fix this, let's clear the bottom bit of the
> > vdso_offset in the VDSO_SYMBOL macro.
> >
> > Test: LTP test syscalls.rt_sigaction01
> > Fixes: f01703b3d2e6 ("arm64: compat: Get sigreturn trampolines from vDSO")
> > Signed-off-by: Will McVicker <willmcvicker@google.com>
> > ---
> > arch/arm64/include/asm/vdso.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h
> > index f99dcb94b438..a7384379e8e1 100644
> > --- a/arch/arm64/include/asm/vdso.h
> > +++ b/arch/arm64/include/asm/vdso.h
> > @@ -23,7 +23,7 @@
> >
> > #define VDSO_SYMBOL(base, name) \
> > ({ \
> > - (void *)(vdso_offset_##name - VDSO_LBASE + (unsigned long)(base)); \
> > + (void *)((vdso_offset_##name & ~1UL) - VDSO_LBASE + (unsigned long)(base)); \
> > })
> >
> > #endif /* !__ASSEMBLY__ */
> > --
> > 2.29.2.299.gdc1121823c-goog
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: Fix off-by-one vdso trampoline return value
2020-11-12 0:14 [PATCH] arm64: Fix off-by-one vdso trampoline return value Will McVicker
2020-11-12 1:00 ` Nick Desaulniers
@ 2020-11-12 10:12 ` Will Deacon
2020-11-12 18:51 ` William Mcvicker
1 sibling, 1 reply; 7+ messages in thread
From: Will Deacon @ 2020-11-12 10:12 UTC (permalink / raw)
To: Will McVicker
Cc: Catalin Marinas, Nathan Chancellor, Nick Desaulniers,
Vincenzo Frascino, Andrei Vagin, Dmitry Safonov, Thomas Gleixner,
linux-arm-kernel, linux-kernel, clang-built-linux, kernel-team
On Thu, Nov 12, 2020 at 12:14:22AM +0000, Will McVicker wrote:
> Depending on your host nm version, the generated header
> `include/generated/vdso32-offsets.h` may have the bottom bit set for the
> thumb vdso offset addresses (as observed when using llvm-nm). This
> results in an additional +1 for thumb vdso trampoline return values
> since compat_setup_return() already includes `vdso_trampoline + thumb`.
> As a result, I see a SIGBUS error when running the LTP test
> syscalls.rt_sigaction01. To fix this, let's clear the bottom bit of the
> vdso_offset in the VDSO_SYMBOL macro.
>
> Test: LTP test syscalls.rt_sigaction01
> Fixes: f01703b3d2e6 ("arm64: compat: Get sigreturn trampolines from vDSO")
> Signed-off-by: Will McVicker <willmcvicker@google.com>
> ---
> arch/arm64/include/asm/vdso.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h
> index f99dcb94b438..a7384379e8e1 100644
> --- a/arch/arm64/include/asm/vdso.h
> +++ b/arch/arm64/include/asm/vdso.h
> @@ -23,7 +23,7 @@
>
> #define VDSO_SYMBOL(base, name) \
> ({ \
> - (void *)(vdso_offset_##name - VDSO_LBASE + (unsigned long)(base)); \
> + (void *)((vdso_offset_##name & ~1UL) - VDSO_LBASE + (unsigned long)(base)); \
I don't think we need this in mainline, because the sigreturn trampoline
is just a bunch of .byte directives and I removed the sigreturn code from
the compat vdso in 2d071968a405 ("arm64: compat: Remove 32-bit sigreturn code
from the vDSO").
Might be needed in some stable kernels though (or we just backport the
patch I mentioned above)
Will
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: Fix off-by-one vdso trampoline return value
2020-11-12 10:12 ` Will Deacon
@ 2020-11-12 18:51 ` William Mcvicker
2020-11-16 22:55 ` William Mcvicker
0 siblings, 1 reply; 7+ messages in thread
From: William Mcvicker @ 2020-11-12 18:51 UTC (permalink / raw)
To: Will Deacon
Cc: Catalin Marinas, Nathan Chancellor, Nick Desaulniers,
Vincenzo Frascino, Andrei Vagin, Dmitry Safonov, Thomas Gleixner,
linux-arm-kernel, linux-kernel, clang-built-linux, kernel-team
Hi Nick,
Regarding llvm-nm, this extra thumb +1 is noticed after porting
https://lore.kernel.org/linux-arm-kernel/20201013033947.2257501-1-natechancellor@gmail.com/
to the Android Common Kernel android-4.19-stable. I'm not sure why using ld.lld
causes this difference, but this proposed patch ensures that we don't rely on
the nm tool used.
Will D.,
Regarding applying this to some stable kernels vs backporting 2d071968a405
("arm64: compat: Remove 32-bit sigreturn code from the vDSO"), I am hesitant to
backport commit 2d071968a405 due it's dependencies. For 4.19 at least, I would
also need to backport these:
8e411be6aad13 will@kernel.org arm64: compat: Always use sigpage for sigreturn trampoline
a39060b009ca0 will@kernel.org arm64: compat: Allow 32-bit vdso and sigpage to co-exist
1d09094aa6205 mark.rutland@arm.com arm64: vdso: use consistent 'map' nomenclature
d3418f3839b66 mark.rutland@arm.com arm64: vdso: use consistent 'abi' nomenclature
3ee16ff3437ca mark.rutland@arm.com arm64: vdso: simplify arch_vdso_type ifdeffery
74fc72e77dc5c mark.rutland@arm.com arm64: vdso: remove aarch32_vdso_pages[]
I have done this in my local tree and verified it fixes the SIGBUS error I'm
seeing; however, it seems a lot cleaner and safer to just patch the VDSO_SYMBOL
macro. Please let me know what route you prefer. I'm happy to backport all of
these if that's the recommended approach.
Thanks,
Will
On 11/12/2020, Will Deacon wrote:
> On Thu, Nov 12, 2020 at 12:14:22AM +0000, Will McVicker wrote:
> > Depending on your host nm version, the generated header
> > `include/generated/vdso32-offsets.h` may have the bottom bit set for the
> > thumb vdso offset addresses (as observed when using llvm-nm). This
> > results in an additional +1 for thumb vdso trampoline return values
> > since compat_setup_return() already includes `vdso_trampoline + thumb`.
> > As a result, I see a SIGBUS error when running the LTP test
> > syscalls.rt_sigaction01. To fix this, let's clear the bottom bit of the
> > vdso_offset in the VDSO_SYMBOL macro.
> >
> > Test: LTP test syscalls.rt_sigaction01
> > Fixes: f01703b3d2e6 ("arm64: compat: Get sigreturn trampolines from vDSO")
> > Signed-off-by: Will McVicker <willmcvicker@google.com>
> > ---
> > arch/arm64/include/asm/vdso.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h
> > index f99dcb94b438..a7384379e8e1 100644
> > --- a/arch/arm64/include/asm/vdso.h
> > +++ b/arch/arm64/include/asm/vdso.h
> > @@ -23,7 +23,7 @@
> >
> > #define VDSO_SYMBOL(base, name) \
> > ({ \
> > - (void *)(vdso_offset_##name - VDSO_LBASE + (unsigned long)(base)); \
> > + (void *)((vdso_offset_##name & ~1UL) - VDSO_LBASE + (unsigned long)(base)); \
>
> I don't think we need this in mainline, because the sigreturn trampoline
> is just a bunch of .byte directives and I removed the sigreturn code from
> the compat vdso in 2d071968a405 ("arm64: compat: Remove 32-bit sigreturn code
> from the vDSO").
>
> Might be needed in some stable kernels though (or we just backport the
> patch I mentioned above)
>
> Will
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: Fix off-by-one vdso trampoline return value
2020-11-12 18:51 ` William Mcvicker
@ 2020-11-16 22:55 ` William Mcvicker
0 siblings, 0 replies; 7+ messages in thread
From: William Mcvicker @ 2020-11-16 22:55 UTC (permalink / raw)
To: Will Deacon
Cc: Catalin Marinas, Nathan Chancellor, Nick Desaulniers,
Vincenzo Frascino, Andrei Vagin, Dmitry Safonov, Thomas Gleixner,
linux-arm-kernel, linux-kernel, clang-built-linux, kernel-team
Hi All,
After digging into this even deeper with Nick, we found that the underlying
issue is with ld.lld not carrying over the st_type for the VDSO_compat* symbol
assignments in `arch/arm64/kernel/vdso32/vdso.lds.S`.
From my Android Common Kernel 4.19 build:
$ llvm-readelf -s arch/arm64/kernel/vdso32/vdso.so.raw | grep thumb | sort
26: 0000094d 0 NOTYPE LOCAL DEFAULT 11 VDSO_compat_sigreturn_thumb
28: 00000955 0 NOTYPE LOCAL DEFAULT 11 VDSO_compat_rt_sigreturn_thumb
37: 0000094d 4 FUNC GLOBAL DEFAULT 11 __kernel_sigreturn_thumb
38: 00000955 4 FUNC GLOBAL DEFAULT 11 __kernel_rt_sigreturn_thumb
8: 0000094d 4 FUNC GLOBAL DEFAULT 11 __kernel_sigreturn_thumb@@LINUX_2.6
9: 00000955 4 FUNC GLOBAL DEFAULT 11 __kernel_rt_sigreturn_thumb@@LINUX_2.6
Fortunately, this has been fixed by llvm here: https://reviews.llvm.org/D86263.
So for kernels that use ld.lld and the VDSO compat sigreturn trampoline, they
need to make sure to upgrade their toolchain.
I hope this thread helps anyone running into this issue in the future.
Thanks,
Will
On 11/12/2020, William Mcvicker wrote:
> Hi Nick,
>
> Regarding llvm-nm, this extra thumb +1 is noticed after porting
> https://lore.kernel.org/linux-arm-kernel/20201013033947.2257501-1-natechancellor@gmail.com/
> to the Android Common Kernel android-4.19-stable. I'm not sure why using ld.lld
> causes this difference, but this proposed patch ensures that we don't rely on
> the nm tool used.
>
> Will D.,
> Regarding applying this to some stable kernels vs backporting 2d071968a405
> ("arm64: compat: Remove 32-bit sigreturn code from the vDSO"), I am hesitant to
> backport commit 2d071968a405 due it's dependencies. For 4.19 at least, I would
> also need to backport these:
>
> 8e411be6aad13 will@kernel.org arm64: compat: Always use sigpage for sigreturn trampoline
> a39060b009ca0 will@kernel.org arm64: compat: Allow 32-bit vdso and sigpage to co-exist
> 1d09094aa6205 mark.rutland@arm.com arm64: vdso: use consistent 'map' nomenclature
> d3418f3839b66 mark.rutland@arm.com arm64: vdso: use consistent 'abi' nomenclature
> 3ee16ff3437ca mark.rutland@arm.com arm64: vdso: simplify arch_vdso_type ifdeffery
> 74fc72e77dc5c mark.rutland@arm.com arm64: vdso: remove aarch32_vdso_pages[]
>
> I have done this in my local tree and verified it fixes the SIGBUS error I'm
> seeing; however, it seems a lot cleaner and safer to just patch the VDSO_SYMBOL
> macro. Please let me know what route you prefer. I'm happy to backport all of
> these if that's the recommended approach.
>
> Thanks,
> Will
>
> On 11/12/2020, Will Deacon wrote:
> > On Thu, Nov 12, 2020 at 12:14:22AM +0000, Will McVicker wrote:
> > > Depending on your host nm version, the generated header
> > > `include/generated/vdso32-offsets.h` may have the bottom bit set for the
> > > thumb vdso offset addresses (as observed when using llvm-nm). This
> > > results in an additional +1 for thumb vdso trampoline return values
> > > since compat_setup_return() already includes `vdso_trampoline + thumb`.
> > > As a result, I see a SIGBUS error when running the LTP test
> > > syscalls.rt_sigaction01. To fix this, let's clear the bottom bit of the
> > > vdso_offset in the VDSO_SYMBOL macro.
> > >
> > > Test: LTP test syscalls.rt_sigaction01
> > > Fixes: f01703b3d2e6 ("arm64: compat: Get sigreturn trampolines from vDSO")
> > > Signed-off-by: Will McVicker <willmcvicker@google.com>
> > > ---
> > > arch/arm64/include/asm/vdso.h | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h
> > > index f99dcb94b438..a7384379e8e1 100644
> > > --- a/arch/arm64/include/asm/vdso.h
> > > +++ b/arch/arm64/include/asm/vdso.h
> > > @@ -23,7 +23,7 @@
> > >
> > > #define VDSO_SYMBOL(base, name) \
> > > ({ \
> > > - (void *)(vdso_offset_##name - VDSO_LBASE + (unsigned long)(base)); \
> > > + (void *)((vdso_offset_##name & ~1UL) - VDSO_LBASE + (unsigned long)(base)); \
> >
> > I don't think we need this in mainline, because the sigreturn trampoline
> > is just a bunch of .byte directives and I removed the sigreturn code from
> > the compat vdso in 2d071968a405 ("arm64: compat: Remove 32-bit sigreturn code
> > from the vDSO").
> >
> > Might be needed in some stable kernels though (or we just backport the
> > patch I mentioned above)
> >
> > Will
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-11-16 22:56 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-12 0:14 [PATCH] arm64: Fix off-by-one vdso trampoline return value Will McVicker
2020-11-12 1:00 ` Nick Desaulniers
2020-11-12 1:07 ` Nick Desaulniers
2020-11-12 2:14 ` Nick Desaulniers
2020-11-12 10:12 ` Will Deacon
2020-11-12 18:51 ` William Mcvicker
2020-11-16 22:55 ` William Mcvicker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).