| Message ID | 20190424152502.14246-9-mathieu.desnoyers@efficios.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,for,5.2,01/10] rseq/selftests: Add __rseq_exit_point_array section for debuggers | expand |
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 <mathieu.desnoyers@efficios.com> > CC: Peter Zijlstra <peterz@infradead.org> > CC: Thomas Gleixner <tglx@linutronix.de> > CC: Joel Fernandes <joelaf@google.com> > CC: Catalin Marinas <catalin.marinas@arm.com> > CC: Dave Watson <davejwatson@fb.com> > CC: Will Deacon <will.deacon@arm.com> > CC: Shuah Khan <shuah@kernel.org> > CC: Andi Kleen <andi@firstfloor.org> > CC: linux-kselftest@vger.kernel.org > CC: "H . Peter Anvin" <hpa@zytor.com> > CC: Chris Lameter <cl@linux.com> > CC: Russell King <linux@arm.linux.org.uk> > CC: Michael Kerrisk <mtk.manpages@gmail.com> > CC: "Paul E . McKenney" <paulmck@linux.vnet.ibm.com> > CC: Paul Turner <pjt@google.com> > CC: Boqun Feng <boqun.feng@gmail.com> > CC: Josh Triplett <josh@joshtriplett.org> > CC: Steven Rostedt <rostedt@goodmis.org> > CC: Ben Maurer <bmaurer@fb.com> > CC: linux-api@vger.kernel.org > CC: Andy Lutomirski <luto@amacapital.net> > CC: Andrew Morton <akpm@linux-foundation.org> > CC: Linus Torvalds <torvalds@linux-foundation.org> > --- > 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 <will.deacon@arm.com> > */ > > -#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? Thanks, Mark.
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 <mathieu.desnoyers@efficios.com> > > CC: Peter Zijlstra <peterz@infradead.org> > > CC: Thomas Gleixner <tglx@linutronix.de> > > CC: Joel Fernandes <joelaf@google.com> > > CC: Catalin Marinas <catalin.marinas@arm.com> > > CC: Dave Watson <davejwatson@fb.com> > > CC: Will Deacon <will.deacon@arm.com> > > CC: Shuah Khan <shuah@kernel.org> > > CC: Andi Kleen <andi@firstfloor.org> > > CC: linux-kselftest@vger.kernel.org > > CC: "H . Peter Anvin" <hpa@zytor.com> > > CC: Chris Lameter <cl@linux.com> > > CC: Russell King <linux@arm.linux.org.uk> > > CC: Michael Kerrisk <mtk.manpages@gmail.com> > > CC: "Paul E . McKenney" <paulmck@linux.vnet.ibm.com> > > CC: Paul Turner <pjt@google.com> > > CC: Boqun Feng <boqun.feng@gmail.com> > > CC: Josh Triplett <josh@joshtriplett.org> > > CC: Steven Rostedt <rostedt@goodmis.org> > > CC: Ben Maurer <bmaurer@fb.com> > > CC: linux-api@vger.kernel.org > > CC: Andy Lutomirski <luto@amacapital.net> > > CC: Andrew Morton <akpm@linux-foundation.org> > > CC: Linus Torvalds <torvalds@linux-foundation.org> > > --- > > 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 <will.deacon@arm.com> > > */ > > > > -#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
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: > > +/* > > + * 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). Now I see the RSEQ_CODE value isn't endian-swapped as teh RSEQ_DATA value is, so the code above is fine. Sory for the noise. Thanks, Mark.
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 <mathieu.desnoyers@efficios.com> > CC: Peter Zijlstra <peterz@infradead.org> > CC: Thomas Gleixner <tglx@linutronix.de> > CC: Joel Fernandes <joelaf@google.com> > CC: Catalin Marinas <catalin.marinas@arm.com> > CC: Dave Watson <davejwatson@fb.com> > CC: Will Deacon <will.deacon@arm.com> > CC: Shuah Khan <shuah@kernel.org> > CC: Andi Kleen <andi@firstfloor.org> > CC: linux-kselftest@vger.kernel.org > CC: "H . Peter Anvin" <hpa@zytor.com> > CC: Chris Lameter <cl@linux.com> > CC: Russell King <linux@arm.linux.org.uk> > CC: Michael Kerrisk <mtk.manpages@gmail.com> > CC: "Paul E . McKenney" <paulmck@linux.vnet.ibm.com> > CC: Paul Turner <pjt@google.com> > CC: Boqun Feng <boqun.feng@gmail.com> > CC: Josh Triplett <josh@joshtriplett.org> > CC: Steven Rostedt <rostedt@goodmis.org> > CC: Ben Maurer <bmaurer@fb.com> > CC: linux-api@vger.kernel.org > CC: Andy Lutomirski <luto@amacapital.net> > CC: Andrew Morton <akpm@linux-foundation.org> > CC: Linus Torvalds <torvalds@linux-foundation.org> > --- > 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 <will.deacon@arm.com> > */ > > -#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. */ It would be neater to implement swab32 and use that with RSEQ_SIG_CODE, but either way: Acked-by: Will Deacon <will.deacon@arm.com> Will
On Wed, Apr 24, 2019 at 05:45:38PM +0100, Will Deacon wrote: > On Wed, Apr 24, 2019 at 11:25:00AM -0400, Mathieu Desnoyers wrote: > > +/* > > + * 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. */ > > It would be neater to implement swab32 and use that with RSEQ_SIG_CODE, If possible, marginally neater than that would be using le32_to_cpu(RSEQ_SIG_CODE), without any ifdeffery necessary. It looks like that's defined in tools/include/linux/kernel.h, but I'm not sure if that gets pulled into your include path. Thanks, Mark.
----- On Apr 24, 2019, at 12:51 PM, Mark Rutland mark.rutland@arm.com wrote: > On Wed, Apr 24, 2019 at 05:45:38PM +0100, Will Deacon wrote: >> On Wed, Apr 24, 2019 at 11:25:00AM -0400, Mathieu Desnoyers wrote: >> > +/* >> > + * 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. */ >> >> It would be neater to implement swab32 and use that with RSEQ_SIG_CODE, > > If possible, marginally neater than that would be using > le32_to_cpu(RSEQ_SIG_CODE), without any ifdeffery necessary. > > It looks like that's defined in tools/include/linux/kernel.h, but I'm > not sure if that gets pulled into your include path. Considering that those RSEQ_SIG* define will end up in public bits/rseq.h headers within glibc, I'm tempted to keep the amount of dependencies on external headers to a minimum, if it's OK with you. Thanks, Mathieu
----- On Apr 24, 2019, at 1:00 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote: > ----- On Apr 24, 2019, at 12:51 PM, Mark Rutland mark.rutland@arm.com wrote: > >> On Wed, Apr 24, 2019 at 05:45:38PM +0100, Will Deacon wrote: >>> On Wed, Apr 24, 2019 at 11:25:00AM -0400, Mathieu Desnoyers wrote: >>> > +/* >>> > + * 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. */ >>> >>> It would be neater to implement swab32 and use that with RSEQ_SIG_CODE, >> >> If possible, marginally neater than that would be using >> le32_to_cpu(RSEQ_SIG_CODE), without any ifdeffery necessary. >> >> It looks like that's defined in tools/include/linux/kernel.h, but I'm >> not sure if that gets pulled into your include path. > > Considering that those RSEQ_SIG* define will end up in public bits/rseq.h > headers within glibc, I'm tempted to keep the amount of dependencies on > external headers to a minimum, if it's OK with you. Also, I'm not sure the le32_to_cpu() approach would work in __ASSEMBLER__ builds. Thanks, Mathieu > > Thanks, > > Mathieu > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com
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 <will.deacon@arm.com> */ -#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" \ __rseq_str(label) ":\n" \ " b %l[" __rseq_str(abort_label) "]\n" \ "222:\n"
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 <mathieu.desnoyers@efficios.com> CC: Peter Zijlstra <peterz@infradead.org> CC: Thomas Gleixner <tglx@linutronix.de> CC: Joel Fernandes <joelaf@google.com> CC: Catalin Marinas <catalin.marinas@arm.com> CC: Dave Watson <davejwatson@fb.com> CC: Will Deacon <will.deacon@arm.com> CC: Shuah Khan <shuah@kernel.org> CC: Andi Kleen <andi@firstfloor.org> CC: linux-kselftest@vger.kernel.org CC: "H . Peter Anvin" <hpa@zytor.com> CC: Chris Lameter <cl@linux.com> CC: Russell King <linux@arm.linux.org.uk> CC: Michael Kerrisk <mtk.manpages@gmail.com> CC: "Paul E . McKenney" <paulmck@linux.vnet.ibm.com> CC: Paul Turner <pjt@google.com> CC: Boqun Feng <boqun.feng@gmail.com> CC: Josh Triplett <josh@joshtriplett.org> CC: Steven Rostedt <rostedt@goodmis.org> CC: Ben Maurer <bmaurer@fb.com> CC: linux-api@vger.kernel.org CC: Andy Lutomirski <luto@amacapital.net> CC: Andrew Morton <akpm@linux-foundation.org> CC: Linus Torvalds <torvalds@linux-foundation.org> --- tools/testing/selftests/rseq/rseq-arm64.h | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)