linux-kernel - Re: [PATCH] arm64: Fix off-by-one vdso trampoline return value

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 11 Nov 2020 17:07:43 -0800
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Will McVicker <willmcvicker@...gle.com>
Cc:     Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Nathan Chancellor <natechancellor@...il.com>,
        Vincenzo Frascino <vincenzo.frascino@....com>,
        Andrei Vagin <avagin@...il.com>,
        Dmitry Safonov <0x7f454c46@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        clang-built-linux <clang-built-linux@...glegroups.com>,
        kernel-team <kernel-team@...roid.com>
Subject: Re: [PATCH] arm64: Fix off-by-one vdso trampoline return value

On Wed, Nov 11, 2020 at 5:00 PM Nick Desaulniers
<ndesaulniers@...gle.com> wrote:
>
> On Wed, Nov 11, 2020 at 4:14 PM Will McVicker <willmcvicker@...gle.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@...gle.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