Re: rseq/x86: choosing rseq code signature

[Mathieu Desnoyers]

----- On Apr 9, 2019, at 3:32 PM, Mathieu Desnoyers mathieu.desnoyers@xxxxxxxxxxxx wrote:

> Hi,
> 
> We are about to include the code signature required prior to restartable
> sequences abort handlers into glibc, which will make this ABI choice final.
> We need architecture maintainer input on that signature value.
> 
> That code signature is placed before each abort handler, so the kernel can
> validate that it is indeed jumping to an abort handler (and not some
> arbitrary attacker-chosen code). The signature is never executed.
> 
> Currently, tools/testing/selftests/rseq/rseq-x86.h defines RSEQ_SIG
> as 0x53053053, and uses it as an immediate operand to the following
> instruction opcodes (as suggested by Andy Lutomirski):
> 
> x86-32:
> - .byte 0x0f, 0x1f, 0x05: nopl <sig>
> 
> x86-64:
> - .byte 0x0f, 0x1f, 0x05: nopl <sig>(%rip)
> 
> The current discussion thread on the glibc mailing list leads us towards
> using a trap with uncommon immediate operand, which simplifies integration
> with disassemblers, emulators, makes it easier to debug if the control
> flow gets redirected there by mistake, and is nicer for some architecture's
> speculative execution.
> 
> The main advantage of choosing a trap instruction over a no-op is to ensure the
> program traps if the execution flow gets redirected to the signature by mistake
> (makes it easier to debug). It's not a hard requirement, but it would be a
> bonus.
> 
> Are there trap instructions that take an uncommon 4-byte immediate
> operand you would recommend on x86 32/64 ? Or is the current choice of
> nopl confirmed to be right one ?
> 
> Here is an example of rseq signature definition template:
> 
> /*
> * TODO: document trap instruction objdump output on each sub-architecture
> * instruction sets, as well as instruction set extensions.
> */
> #define RSEQ_SIG 0x########

Peter Zijlstra suggested to use "invlpg" in user-space, which should generate
a trap. The only concern would be emulators, but ideally they would not try to
decode an instruction that is never executed. This would lead to the following
patch. Any objections/ack ?


diff --git a/tools/testing/selftests/rseq/rseq-x86.h b/tools/testing/selftests/rseq/rseq-x86.h
index 2d4887b5d3f0..e9c8a9879e18 100644
--- a/tools/testing/selftests/rseq/rseq-x86.h
+++ b/tools/testing/selftests/rseq/rseq-x86.h
@@ -7,6 +7,11 @@
 
 #include <stdint.h>
 
+/*
+ * RSEQ_SIG is used with the following privileged instructions, which trap in user-space:
+ * x86-32:    0f 01 3d 53 30 05 53      invlpg 0x53053053
+ * x86-64:    0f 01 3d 53 30 05 53      invlpg 0x53053053(%rip)
+ */
 #define RSEQ_SIG       0x53053053
 
 #ifdef __x86_64__
@@ -78,8 +83,8 @@ do {                                                                  \
 
 #define RSEQ_ASM_DEFINE_ABORT(label, teardown, abort_label)            \
                ".pushsection __rseq_failure, \"ax\"\n\t"               \
-               /* Disassembler-friendly signature: nopl <sig>(%rip). */\
-               ".byte 0x0f, 0x1f, 0x05\n\t"                            \
+               /* Disassembler-friendly signature: invlpg <sig>(%rip). */\
+               ".byte 0x0f, 0x01, 0x3d\n\t"                            \
                ".long " __rseq_str(RSEQ_SIG) "\n\t"                    \
                __rseq_str(label) ":\n\t"                               \
                teardown                                                \
@@ -605,8 +610,8 @@ do {                                                                        \
 
 #define RSEQ_ASM_DEFINE_ABORT(label, teardown, abort_label)            \
                ".pushsection __rseq_failure, \"ax\"\n\t"               \
-               /* Disassembler-friendly signature: nopl <sig>. */      \
-               ".byte 0x0f, 0x1f, 0x05\n\t"                            \
+               /* Disassembler-friendly signature: invlpg <sig>. */    \
+               ".byte 0x0f, 0x1f, 0x3d\n\t"                            \
                ".long " __rseq_str(RSEQ_SIG) "\n\t"                    \
                __rseq_str(label) ":\n\t"                               \
                teardown                                                \


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com