| 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
| ||
|
Message-ID: <716499178.40175.1560009165920.JavaMail.zimbra@efficios.com>
Date: Sat, 8 Jun 2019 11:52:45 -0400 (EDT)
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: Will Deacon <will.deacon@....com>,
Russell King <linux@....linux.org.uk>
Cc: linux-kernel <linux-kernel@...r.kernel.org>,
linux-api <linux-api@...r.kernel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>,
"Paul E . McKenney" <paulmck@...ux.vnet.ibm.com>,
Boqun Feng <boqun.feng@...il.com>, shuah <shuah@...nel.org>,
Andy Lutomirski <luto@...capital.net>,
Dave Watson <davejwatson@...com>, Paul Turner <pjt@...gle.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>, Andi Kleen <andi@...stfloor.org>,
Chris Lameter <cl@...ux.com>, Ben Maurer <bmaurer@...com>,
rostedt <rostedt@...dmis.org>,
Josh Triplett <josh@...htriplett.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Catalin Marinas <catalin.marinas@....com>,
Michael Kerrisk <mtk.manpages@...il.com>,
Joel Fernandes <joelaf@...gle.com>,
linux-kselftest <linux-kselftest@...r.kernel.org>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 for 5.2 08/12] rseq/selftests: arm: use udf
instruction for RSEQ_SIG
----- On Jun 6, 2019, at 8:02 PM, Mathieu Desnoyers mathieu.desnoyers@...icios.com wrote:
> ----- On May 3, 2019, at 3:38 PM, Mathieu Desnoyers
> mathieu.desnoyers@...icios.com wrote:
>
>> Use udf as the guard instruction for the restartable sequence abort
>> handler.
>>
>> Previously, the chosen signature was not a valid instruction, based
>> on the assumption that it could always sit in a literal pool. However,
>> there are compilation environments in which literal pools are not
>> available, for instance execute-only code. Therefore, we need to
>> choose a signature value that is also a valid instruction.
>>
>> Handle compiling with -mbig-endian on ARMv6+, 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.
>>
>> Prior to ARMv6, -mbig-endian generates big-endian code and data, so
>> endianness should not be reversed in that case.
>
> And of course it cannot be that easy. This breaks when building in
> thumb mode (-mthumb). Output from librseq arm32 build [1] (code similar
> to what is found in the rseq selftests):
>
> CC rseq.lo
> /tmp/ccu6Jw1b.s: Assembler messages:
> /tmp/ccu6Jw1b.s:297: Error: cannot determine Thumb instruction size. Use
> .inst.n/.inst.w instead
> /tmp/ccu6Jw1b.s:490: Error: cannot determine Thumb instruction size. Use
> .inst.n/.inst.w instead
> Makefile:460: recipe for target 'rseq.lo' failed
>
> This appears to be caused by a missing .arm directive in RSEQ_SIG_DATA.
> Fixing with:
>
> - asm volatile ("b 2f\n\t" \
> + asm volatile (".arm\n\t" \
> + "b 2f\n\t" \
>
> gets the build to go further, but breaks at:
>
> CC basic_percpu_ops_test.o
> /tmp/ccpHOMHZ.s: Assembler messages:
> /tmp/ccpHOMHZ.s:148: Error: misaligned branch destination
> /tmp/ccpHOMHZ.s:956: Error: misaligned branch destination
> Makefile:378: recipe for target 'basic_percpu_ops_test.o' failed
>
> I suspect it's caused by the change from:
>
> - ".word " __rseq_str(RSEQ_SIG) "\n\t" \
>
> to
>
> + ".arm\n\t" \
> + ".inst " __rseq_str(RSEQ_SIG_CODE) "\n\t" \
>
> which changes the mode from thumb to arm for the rest of the
> inline asm within __RSEQ_ASM_DEFINE_ABORT. Better yet, there appears
> to be no way to save the arm/thumb state and restore it afterwards.
>
> I'm really starting to wonder if we should go our of our way to try
> to get this signature to be a valid instruction on arm32. Perhaps
> we should consider going back to use ".word" on arm32 so it ensures
> it uses data endianness (which matches the parameter received by the
> sys_rseq system call), let objdump and friends print it as a literal
> pool (which it is), and just choose an instruction which has little
> chances to appear for the cases we care about between ARM32 BE, LE
> and THUMB. Perhaps a 32-bit palindrome ? Bonus points if this is a
> trap instruction in common configurations for odd-cases-debugging
> purposes.
So I'm not particularly proud of the result, but I found a rather
ugly way to figure out if we are currently in thumb mode within an
inline asm, and restore that mode: test the length of a nop
instruction with a ".if" asm statement.
Do we want to go for this kind of approach, or should we revert
back to a ".word" and accept that the rseq signature before the
abort handler will be seen as data rather than an instruction
on arm32 ?
Is there a better way to do this ?
Thanks,
Mathieu
diff --git a/include/rseq/rseq-arm.h b/include/rseq/rseq-arm.h
index 1ce9231..b6c36dd 100644
--- a/include/rseq/rseq-arm.h
+++ b/include/rseq/rseq-arm.h
@@ -43,7 +43,14 @@
({ \
int sig; \
asm volatile ("b 2f\n\t" \
+ "3:\n\t" \
+ "nop\n\t" \
+ "4:\n\t" \
+ ".arm\n\t" \
"1: .inst " __rseq_str(RSEQ_SIG_CODE) "\n\t" \
+ ".if ((4b - 3b) == 2)\n\t" \
+ ".thumb\n\t" \
+ ".endif\n\t" \
"2:\n\t" \
"ldr %[sig], 1b\n\t" \
: [sig] "=r" (sig)); \
@@ -125,8 +132,14 @@ do { \
__rseq_str(table_label) ":\n\t" \
".word " __rseq_str(version) ", " __rseq_str(flags) "\n\t" \
".word " __rseq_str(start_ip) ", 0x0, " __rseq_str(post_commit_offset) ", 0x0, " __rseq_str(abort_ip) ", 0x0\n\t" \
+ "333:\n\t" \
+ "nop\n\t" \
+ "444:\n\t" \
".arm\n\t" \
".inst " __rseq_str(RSEQ_SIG_CODE) "\n\t" \
+ ".if ((444b - 333b) == 2)\n\t" \
+ ".thumb\n\t" \
+ ".endif\n\t" \
__rseq_str(label) ":\n\t" \
teardown \
"b %l[" __rseq_str(abort_label) "]\n\t"
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Powered by blists - more mailing lists