[RFC,AArch64] Add support for system register based stack protector canary access

For quite sometime the kernel guys, (more specifically Ard) have been 
talking about using a system register (sp_el0) and an offset from that 
for a canary based access. This patchset adds support for a new set of
command line options similar to how powerpc has done this.

I don't intend to change the defaults in userland, we've discussed this 
for user-land in the past and as far as glibc and userland is concerned 
we stick to the options as currently existing. The system register 
option is really for the kernel to use along with an offset as they 
control their ABI and this is a decision for them to make.

I did consider sticking this all under a mcmodel=kernel-small option but
thought that would be a bit too aggressive. There is very little error
checking I can do in terms of the system register being used and really
the assembler would barf quite quickly in case things go wrong. I've
managed to rebuild Ard's kernel tree with an additional patch that
I will send to him. I haven't managed to boot this kernel.

There was an additional question asked about the performance 
characteristics of this but it's a security feature and the kernel 
doesn't have the luxury of a hidden symbol. Further since the kernel 
uses sp_el0 for access everywhere and if they choose to use the same 
register I don't think the performance characteristics would be too bad, 
but that's a decision for the kernel folks to make when taking in the 
feature into the kernel.

I still need to add some tests and documentation in invoke.texi but
this is at the stage where it would be nice for some other folks
to look at this.

The difference in code generated is as below.

extern void bar (char *);
int foo (void)
{
   char a[100];
   bar (&a);
}

$GCC -O2  -fstack-protector-strong  vs 
-mstack-protector-guard-reg=sp_el0 -mstack-protector-guard=sysreg 
-mstack-protector-guard-offset=1024 -fstack-protector-strong

	



I will be afk tomorrow and day after but this is to elicit some comments 
and for Ard to try this out with his kernel patches.

Thoughts ?

regards
Ramana

gcc/ChangeLog:

2018-11-23  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

         * config/aarch64/aarch64-opts.h (enum stack_protector_guard): New
         * config/aarch64/aarch64.c (aarch64_override_options_internal): 
Handle
         and put in error checks for stack protector guard options.
         (aarch64_stack_protect_guard): New.
         (TARGET_STACK_PROTECT_GUARD): Define.
         * config/aarch64/aarch64.md (UNSPEC_SSP_SYSREG): New.
         (reg_stack_protect_address<mode>): New.
         (stack_protect_set): Adjust for SSP_GLOBAL.
         (stack_protect_test): Likewise.
         * config/aarch64/aarch64.opt (-mstack-protector-guard-reg): New.
         (-mstack-protector-guard): Likewise.
         (-mstack-protector-guard-offset): Likewise.
         * doc/invoke.texi: Document new AArch64 options.
commit 9febaa23c114e598ddc9a2406ad96d8fa3ebe0c6
Author: Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>
Date:   Mon Nov 19 10:12:12 2018 +0000

diff --git a/gcc/config/aarch64/aarch64-opts.h b/gcc/config/aarch64/aarch64-opts.h
index 7a5c6d7664f..2f06f3e0e5a 100644
--- a/gcc/config/aarch64/aarch64-opts.h
+++ b/gcc/config/aarch64/aarch64-opts.h
@@ -91,4 +91,10 @@ enum aarch64_sve_vector_bits_enum {
   SVE_2048 = 2048
 };
 
+/* Where to get the canary for the stack protector.  */
+enum stack_protector_guard {
+  SSP_SYSREG,			/* per-thread canary in special system register */
+  SSP_GLOBAL			/* global canary */
+};
+
 #endif
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 0d89ba27e4a..a56b2166542 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10955,6 +10955,41 @@ aarch64_override_options_internal (struct gcc_options *opts)
   if (opts->x_flag_strict_volatile_bitfields < 0 && abi_version_at_least (2))
     opts->x_flag_strict_volatile_bitfields = 1;
 
+  if (aarch64_stack_protector_guard == SSP_GLOBAL
+      && opts->x_aarch64_stack_protector_guard_offset_str)
+    {
+      error ("Incompatible options -mstack-protector-guard=global and"
+	     "-mstack-protector-guard-offset=%qs",
+	     aarch64_stack_protector_guard_offset_str);
+    }
+
+  if (aarch64_stack_protector_guard == SSP_SYSREG
+      && !(opts->x_aarch64_stack_protector_guard_offset_str
+	   && opts->x_aarch64_stack_protector_guard_reg_str))
+    {
+      error ("Both -mstack-protector-guard-offset and "
+	     "-mstack-protector-guard-reg must be used "
+	     "with -mstack-protector-guard=sysreg");
+    }
+
+  if (opts->x_aarch64_stack_protector_guard_reg_str)
+    {
+      if (strlen (opts->x_aarch64_stack_protector_guard_reg_str) > 100)
+	  error ("Specify a system register with a small string length.");
+    }
+
+  if (opts->x_aarch64_stack_protector_guard_offset_str)
+    {
+      char *end;
+      const char *str = aarch64_stack_protector_guard_offset_str;
+      errno = 0;
+      long offs = strtol (aarch64_stack_protector_guard_offset_str, &end, 0);
+      if (!*str || *end || errno)
+	error ("%qs is not a valid offset in %qs", str,
+	       "-mstack-protector-guard-offset=");
+      aarch64_stack_protector_guard_offset = offs;
+    }
+
   initialize_aarch64_code_model (opts);
   initialize_aarch64_tls_size (opts);
 
@@ -17872,8 +17907,24 @@ aarch64_run_selftests (void)
 
 } // namespace selftest
 
+/* Implement TARGET_STACK_PROTECT_GUARD. In case of a
+   global variable based guard use the default else
+   return a null tree.  */
+static tree
+aarch64_stack_protect_guard (void)
+{
+  if (aarch64_stack_protector_guard == SSP_GLOBAL)
+    return default_stack_protect_guard ();
+
+  return NULL_TREE;
+}
+
+
 #endif /* #if CHECKING_P */
 
+#undef TARGET_STACK_PROTECT_GUARD
+#define TARGET_STACK_PROTECT_GUARD aarch64_stack_protect_guard
+
 #undef TARGET_ADDRESS_COST
 #define TARGET_ADDRESS_COST aarch64_address_cost
 
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 82af4d47f78..8b0eb9e4382 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -194,6 +194,7 @@
     UNSPEC_UCVTF
     UNSPEC_USHL_2S
     UNSPEC_VSTRUCTDUMMY
+    UNSPEC_SSP_SYSREG
     UNSPEC_SP_SET
     UNSPEC_SP_TEST
     UNSPEC_RSQRT
@@ -6561,13 +6562,46 @@
   ""
 {
   machine_mode mode = GET_MODE (operands[0]);
+  if (aarch64_stack_protector_guard != SSP_GLOBAL)
+  {
+    /* Generate access through the system register.  */
+    rtx tmp_reg = gen_reg_rtx (mode);
+    if (mode == DImode)
+    {
+        emit_insn (gen_reg_stack_protect_address_di (tmp_reg));
+        emit_insn (gen_adddi3 (tmp_reg, tmp_reg,
+			       GEN_INT (aarch64_stack_protector_guard_offset)));
+    }
+    else
+    {
+	emit_insn (gen_reg_stack_protect_address_si (tmp_reg));
+	emit_insn (gen_addsi3 (tmp_reg, tmp_reg,
+			       GEN_INT (aarch64_stack_protector_guard_offset)));
 
+    }
+    operands[1] = gen_rtx_MEM (mode, tmp_reg);
+  }
+  
   emit_insn ((mode == DImode
 	      ? gen_stack_protect_set_di
 	      : gen_stack_protect_set_si) (operands[0], operands[1]));
   DONE;
 })
 
+(define_insn "reg_stack_protect_address_<mode>"
+ [(set (match_operand:PTR 0 "register_operand" "=r")
+       (unspec:PTR [(const_int 0)]
+	UNSPEC_SSP_SYSREG))]
+ "aarch64_stack_protector_guard != SSP_GLOBAL"
+ {
+   char buf[150];
+   snprintf (buf, 150, "mrs\\t%%<w>0, %s",
+	    aarch64_stack_protector_guard_reg_str);
+   output_asm_insn (buf, operands);
+   return "";
+ }
+ [(set_attr "type" "mrs")])
+
 (define_insn "stack_protect_set_<mode>"
   [(set (match_operand:PTR 0 "memory_operand" "=m")
 	(unspec:PTR [(match_operand:PTR 1 "memory_operand" "m")]
@@ -6588,12 +6622,34 @@
   machine_mode mode = GET_MODE (operands[0]);
 
   result = gen_reg_rtx(mode);
+  if (aarch64_stack_protector_guard != SSP_GLOBAL)
+  {
+    /* Generate access through the system register. The
+       sequence we want here is the access
+       of the stack offset to come with
+       mrs scratch_reg, <system_register>
+       add scratch_reg, scratch_reg, :lo12:offset. */
+    rtx tmp_reg = gen_reg_rtx (mode);
+    if (mode == DImode)
+    {
+       emit_insn (gen_reg_stack_protect_address_di (tmp_reg));
+       emit_insn (gen_adddi3 (tmp_reg, tmp_reg,
+       		              GEN_INT (aarch64_stack_protector_guard_offset)));
+    }
+    else
+    {
+	emit_insn (gen_reg_stack_protect_address_si (tmp_reg));
+	emit_insn (gen_addsi3 (tmp_reg, tmp_reg,
+			       GEN_INT (aarch64_stack_protector_guard_offset)));
 
+    }
+    operands[1] = gen_rtx_MEM (mode, tmp_reg);
+  }
   emit_insn ((mode == DImode
-	      ? gen_stack_protect_test_di
-	      : gen_stack_protect_test_si) (result,
-					    operands[0],
-					    operands[1]));
+		  ? gen_stack_protect_test_di
+		  : gen_stack_protect_test_si) (result,
+					        operands[0],
+					        operands[1]));
 
   if (mode == DImode)
     emit_jump_insn (gen_cbranchdi4 (gen_rtx_EQ (VOIDmode, result, const0_rtx),
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index b2e80cbf6f1..1aaf4beb329 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -218,3 +218,33 @@ Enables verbose cost model dumping in the debug dump files.
 mtrack-speculation
 Target Var(aarch64_track_speculation)
 Generate code to track when the CPU might be speculating incorrectly.
+
+mstack-protector-guard=
+Target RejectNegative Joined Enum(stack_protector_guard) Var(aarch64_stack_protector_guard) Init(SSP_GLOBAL)
+Use given stack-protector guard.
+
+Enum
+Name(stack_protector_guard) Type(enum stack_protector_guard)
+Valid arguments to -mstack-protector-guard=:
+
+EnumValue
+Enum(stack_protector_guard) String(sysreg) Value(SSP_SYSREG)
+
+EnumValue
+Enum(stack_protector_guard) String(global) Value(SSP_GLOBAL)
+
+mstack-protector-guard-reg=
+Target Joined RejectNegative String Var(aarch64_stack_protector_guard_reg_str)
+Use the system register specified on the command line as the stack protector
+guard register. This option is for use with fstack-protector-strong and
+not for use in user-land code.
+
+mstack-protector-guard-offset=
+Target Joined RejectNegative String Var(aarch64_stack_protector_guard_offset_str)
+Use an immediate to offset from the stack protector guard register, sp_el0.
+This option is for use with fstack-protector-strong and not for use in
+user-land code.
+
+TargetVariable
+long aarch64_stack_protector_guard_offset = 0
+
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d46ebd02c4e..dbe1ca42be4 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -629,7 +629,9 @@ Objective-C and Objective-C++ Dialects}.
 -mpc-relative-literal-loads @gol
 -msign-return-address=@var{scope} @gol
 -march=@var{name}  -mcpu=@var{name}  -mtune=@var{name}  @gol
--moverride=@var{string}  -mverbose-cost-dump  -mtrack-speculation} 
+-moverride=@var{string}  -mverbose-cost-dump @gol
+-mstack-protector-guard=@var{guard} -mstack-protector-guard-reg=@var{sysreg} @gol
+-mstack-protector-guard-offset=@var{offset} -mtrack-speculation }
 
 @emph{Adapteva Epiphany Options}
 @gccoptlist{-mhalf-reg-file  -mprefer-short-insn-regs @gol
@@ -15450,6 +15452,24 @@ object boundary as described in the architecture specification.
 Omit or keep the frame pointer in leaf functions.  The former behavior is the
 default.
 
+@item -mstack-protector-guard=@var{guard}
+@itemx -mstack-protector-guard-reg=@var{reg}
+@itemx -mstack-protector-guard-offset=@var{offset}
+@opindex mstack-protector-guard
+@opindex mstack-protector-guard-reg
+@opindex mstack-protector-guard-offset
+Generate stack protection code using canary at @var{guard}.  Supported
+locations are @samp{global} for a global canary or @samp{sysreg} for a
+canary in an appropriate system register.
+
+With the latter choice the options
+@option{-mstack-protector-guard-reg=@var{reg}} and
+@option{-mstack-protector-guard-offset=@var{offset}} furthermore specify
+which system register to use as base register for reading the canary,
+and from what offset from that base register. There is no default
+register or offset as this is entirely for use within the Linux
+kernel.
+
 @item -mtls-dialect=desc
 @opindex mtls-dialect=desc
 Use TLS descriptors as the thread-local storage mechanism for dynamic accesses


Message ID	7a5a57fa-629d-d2ff-6292-e0893647ec8a@arm.com
State	New
Headers	show Delivered-To: patch@linaro.org Received-SPF: pass (google.com: domain of gcc-patches-return-491488-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:content-type:mime-version; q=dns; s=default; b=Uy/nSfhA3lM0lAX1nLia7flE+tASV+16hpmrf1GD8fd7aSsZSA 1YafRYj2b84rhUmedMgT7Dvd4JzDjcE0DACxIOQ1Q1JPnCg/HmvW3zWGf4xRXUJb 5b1yl9M0Q/6SL2PorNacv0sWrhSdOQTjWCvvCaIsJIpcBCKqmq/pWcDWk= Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk Sender: gcc-patches-owner@gcc.gnu.org From: Ramana Radhakrishnan <Ramana.Radhakrishnan@arm.com> To: James Greenhalgh <James.Greenhalgh@arm.com>, Richard Earnshaw <Richard.Earnshaw@arm.com> CC: Marcus Shawcroft <Marcus.Shawcroft@arm.com>, "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>, Ard Biesheuvel <ard.biesheuvel@linaro.org>, Will Deacon <Will.Deacon@arm.com>, Mark Rutland <Mark.Rutland@arm.com>, nd <nd@arm.com> Subject: [RFC][AArch64] Add support for system register based stack protector canary access Date: Mon, 3 Dec 2018 09:55:36 +0000 Message-ID: <7a5a57fa-629d-d2ff-6292-e0893647ec8a@arm.com> user-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) Content-Type: multipart/mixed; boundary="_002_7a5a57fa629dd2ff6292e0893647ec8aarmcom_" MIME-Version: 1.0
Series	[RFC,AArch64] Add support for system register based stack protector canary access \| expand [RFC,AArch64] Add support for system register based stack protector canary access

[RFC,AArch64] Add support for system register based stack protector canary access

Commit Message

Comments

Patch