On Wed, Apr 24, 2019 at 05:40:33PM +0100, Mark Rutland wrote:
> On Wed, Apr 24, 2019 at 11:25:00AM -0400, Mathieu Desnoyers wrote:
> > Handle compiling with -mbig-endian on aarch64, which generates binaries
> > with mixed code vs data endianness (little endian code, big endian
> > data).
> > 
> > Else mismatch between code endianness for the generated signatures and
> > data endianness for the RSEQ_SIG parameter passed to the rseq
> > registration will trigger application segmentation faults when the
> > kernel try to abort rseq critical sections.
> > 
> > Signed-off-by: Mathieu Desnoyers <[email protected]>
> > CC: Peter Zijlstra <[email protected]>
> > CC: Thomas Gleixner <[email protected]>
> > CC: Joel Fernandes <[email protected]>
> > CC: Catalin Marinas <[email protected]>
> > CC: Dave Watson <[email protected]>
> > CC: Will Deacon <[email protected]>
> > CC: Shuah Khan <[email protected]>
> > CC: Andi Kleen <[email protected]>
> > CC: [email protected]
> > CC: "H . Peter Anvin" <[email protected]>
> > CC: Chris Lameter <[email protected]>
> > CC: Russell King <[email protected]>
> > CC: Michael Kerrisk <[email protected]>
> > CC: "Paul E . McKenney" <[email protected]>
> > CC: Paul Turner <[email protected]>
> > CC: Boqun Feng <[email protected]>
> > CC: Josh Triplett <[email protected]>
> > CC: Steven Rostedt <[email protected]>
> > CC: Ben Maurer <[email protected]>
> > CC: [email protected]
> > CC: Andy Lutomirski <[email protected]>
> > CC: Andrew Morton <[email protected]>
> > CC: Linus Torvalds <[email protected]>
> > ---
> >  tools/testing/selftests/rseq/rseq-arm64.h | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/rseq/rseq-arm64.h 
> > b/tools/testing/selftests/rseq/rseq-arm64.h
> > index b41a2a48e965..200dae9e4208 100644
> > --- a/tools/testing/selftests/rseq/rseq-arm64.h
> > +++ b/tools/testing/selftests/rseq/rseq-arm64.h
> > @@ -6,7 +6,20 @@
> >   * (C) Copyright 2018 - Will Deacon <[email protected]>
> >   */
> >  
> > -#define RSEQ_SIG   0xd428bc00      /* BRK #0x45E0 */
> > +/*
> > + * aarch64 -mbig-endian generates mixed endianness code vs data:
> > + * little-endian code and big-endian data. Ensure the RSEQ_SIG signature
> > + * matches code endianness.
> > + */
> > +#define RSEQ_SIG_CODE      0xd428bc00      /* BRK #0x45E0.  */
> > +
> > +#ifdef __AARCH64EB__
> > +#define RSEQ_SIG_DATA      0x00bc28d4      /* BRK #0x45E0.  */
> > +#else
> > +#define RSEQ_SIG_DATA      RSEQ_SIG_CODE
> > +#endif
> > +
> > +#define RSEQ_SIG   RSEQ_SIG_DATA
> >  
> >  #define rseq_smp_mb()      __asm__ __volatile__ ("dmb ish" ::: "memory")
> >  #define rseq_smp_rmb()     __asm__ __volatile__ ("dmb ishld" ::: "memory")
> > @@ -121,7 +134,7 @@ do {                                                    
> >                         \
> >  
> >  #define RSEQ_ASM_DEFINE_ABORT(label, abort_label)                          
> > \
> >     "       b       222f\n"                                                 
> > \
> > -   "       .inst   "       __rseq_str(RSEQ_SIG) "\n"                       
> > \
> > +   "       .inst   "       __rseq_str(RSEQ_SIG_CODE) "\n"                  
> > \
> 
> I don't think this is right; the .inst directive _should_ emit the value
> in the instruction stream endianness (i.e. LE, regardless of the data
> endianness).
> 
> That's certainly the case with the kernel.org crosstool GCC:
> 
> [mark@lakrids:/mnt/data/tests/inst-test]% cat test.c                          
>      
> void func(void)
> {
>         asm volatile(".inst 0xd4000001");
> }
> [mark@lakrids:/mnt/data/tests/inst-test]% usekorg 8.1.0 aarch64-linux-gcc -c 
> test.c
> [mark@lakrids:/mnt/data/tests/inst-test]% usekorg 8.1.0 aarch64-linux-objdump 
> -d test.o
> 
> test.o:     file format elf64-littleaarch64
> 
> 
> Disassembly of section .text:
> 
> 0000000000000000 <func>:
>    0:   d4000001        svc     #0x0
>    4:   d503201f        nop
>    8:   d65f03c0        ret
> [mark@lakrids:/mnt/data/tests/inst-test]% usekorg 8.1.0 aarch64-linux-gcc 
> -mbig-endian -c test.c
> [mark@lakrids:/mnt/data/tests/inst-test]% usekorg 8.1.0 aarch64-linux-objdump 
> -d test.o         
> 
> test.o:     file format elf64-bigaarch64
> 
> 
> Disassembly of section .text:
> 
> 0000000000000000 <func>:
>    0:   d4000001        svc     #0x0
>    4:   d503201f        nop
>    8:   d65f03c0        ret
> 
> 
> 
> Have you tested this? Is there some toolchain that doesn't get this
> right?

I think that the issue is that the kernel loads the thing to check the
signature. RSEQ_SIG_CODE isn't byte-swapped explicitly and is used with
.inst. RSEG_SIG_DATA is byte-swapped to ensure that the value passed into
the syscall is consistent with what the kernel will load.

But yeah, I've just spent the last ten minutes confusing myself with this.

Will

Reply via email to