|
| 1 | +From 3202535093c445463fa259983149fea8d97e2cfd Mon Sep 17 00:00:00 2001 |
| 2 | +From: Richard Sandiford < [email protected]> |
| 3 | +Date: Fri, 7 Aug 2020 12:15:02 +0100 |
| 4 | +Subject: [PATCH 19/49] arm: Clear canary value after stack_protect_test |
| 5 | + [PR96191] |
| 6 | + |
| 7 | +The stack_protect_test patterns were leaving the canary value in the |
| 8 | +temporary register, meaning that it was often still in registers on |
| 9 | +return from the function. An attacker might therefore have been |
| 10 | +able to use it to defeat stack-smash protection for a later function. |
| 11 | + |
| 12 | +gcc/ |
| 13 | + PR target/96191 |
| 14 | + * config/arm/arm.md (arm_stack_protect_test_insn): Zero out |
| 15 | + operand 2 after use. |
| 16 | + * config/arm/thumb1.md (thumb1_stack_protect_test_insn): Likewise. |
| 17 | + |
| 18 | +gcc/testsuite/ |
| 19 | + * gcc.target/arm/stack-protector-1.c: New test. |
| 20 | + * gcc.target/arm/stack-protector-2.c: Likewise. |
| 21 | + |
| 22 | +(cherry picked from commit 6a3f3e08723063ea2dadb7ddf503f02972a724e2) |
| 23 | +--- |
| 24 | + gcc/config/arm/arm.md | 6 +- |
| 25 | + gcc/config/arm/thumb1.md | 8 ++- |
| 26 | + .../gcc.target/arm/stack-protector-1.c | 63 +++++++++++++++++++ |
| 27 | + .../gcc.target/arm/stack-protector-2.c | 6 ++ |
| 28 | + 4 files changed, 78 insertions(+), 5 deletions(-) |
| 29 | + create mode 100644 gcc/testsuite/gcc.target/arm/stack-protector-1.c |
| 30 | + create mode 100644 gcc/testsuite/gcc.target/arm/stack-protector-2.c |
| 31 | + |
| 32 | +diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md |
| 33 | +index 9367be59083..0fa044b5ac0 100644 |
| 34 | +--- a/gcc/config/arm/arm.md |
| 35 | ++++ b/gcc/config/arm/arm.md |
| 36 | +@@ -9320,6 +9320,8 @@ |
| 37 | + [(set_attr "arch" "t1,32")] |
| 38 | + ) |
| 39 | + |
| 40 | ++;; DO NOT SPLIT THIS PATTERN. It is important for security reasons that the |
| 41 | ++;; canary value does not live beyond the end of this sequence. |
| 42 | + (define_insn "arm_stack_protect_test_insn" |
| 43 | + [(set (reg:CC_Z CC_REGNUM) |
| 44 | + (compare:CC_Z (unspec:SI [(match_operand:SI 1 "memory_operand" "m,m") |
| 45 | +@@ -9329,8 +9331,8 @@ |
| 46 | + (clobber (match_operand:SI 0 "register_operand" "=&l,&r")) |
| 47 | + (clobber (match_dup 2))] |
| 48 | + "TARGET_32BIT" |
| 49 | +- "ldr\t%0, [%2]\;ldr\t%2, %1\;eors\t%0, %2, %0" |
| 50 | +- [(set_attr "length" "8,12") |
| 51 | ++ "ldr\t%0, [%2]\;ldr\t%2, %1\;eors\t%0, %2, %0\;mov\t%2, #0" |
| 52 | ++ [(set_attr "length" "12,16") |
| 53 | + (set_attr "conds" "set") |
| 54 | + (set_attr "type" "multiple") |
| 55 | + (set_attr "arch" "t,32")] |
| 56 | +diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md |
| 57 | +index 24861635fa5..0ff819090d9 100644 |
| 58 | +--- a/gcc/config/arm/thumb1.md |
| 59 | ++++ b/gcc/config/arm/thumb1.md |
| 60 | +@@ -2020,6 +2020,8 @@ |
| 61 | + [(set_attr "type" "mov_reg")] |
| 62 | + ) |
| 63 | + |
| 64 | ++;; DO NOT SPLIT THIS PATTERN. It is important for security reasons that the |
| 65 | ++;; canary value does not live beyond the end of this sequence. |
| 66 | + (define_insn "thumb1_stack_protect_test_insn" |
| 67 | + [(set (match_operand:SI 0 "register_operand" "=&l") |
| 68 | + (unspec:SI [(match_operand:SI 1 "memory_operand" "m") |
| 69 | +@@ -2027,9 +2029,9 @@ |
| 70 | + UNSPEC_SP_TEST)) |
| 71 | + (clobber (match_dup 2))] |
| 72 | + "TARGET_THUMB1" |
| 73 | +- "ldr\t%0, [%2]\;ldr\t%2, %1\;eors\t%0, %2, %0" |
| 74 | +- [(set_attr "length" "8") |
| 75 | +- (set_attr "conds" "set") |
| 76 | ++ "ldr\t%0, [%2]\;ldr\t%2, %1\;eors\t%0, %2, %0\;movs\t%2, #0" |
| 77 | ++ [(set_attr "length" "10") |
| 78 | ++ (set_attr "conds" "clob") |
| 79 | + (set_attr "type" "multiple")] |
| 80 | + ) |
| 81 | + |
| 82 | +diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-1.c b/gcc/testsuite/gcc.target/arm/stack-protector-1.c |
| 83 | +new file mode 100644 |
| 84 | +index 00000000000..b03ea14c4e2 |
| 85 | +--- /dev/null |
| 86 | ++++ b/gcc/testsuite/gcc.target/arm/stack-protector-1.c |
| 87 | +@@ -0,0 +1,63 @@ |
| 88 | ++/* { dg-do run } */ |
| 89 | ++/* { dg-require-effective-target fstack_protector } */ |
| 90 | ++/* { dg-options "-fstack-protector-all -O2" } */ |
| 91 | ++ |
| 92 | ++extern volatile long *stack_chk_guard_ptr; |
| 93 | ++ |
| 94 | ++volatile long * |
| 95 | ++get_ptr (void) |
| 96 | ++{ |
| 97 | ++ return stack_chk_guard_ptr; |
| 98 | ++} |
| 99 | ++ |
| 100 | ++void __attribute__ ((noipa)) |
| 101 | ++f (void) |
| 102 | ++{ |
| 103 | ++ volatile int x; |
| 104 | ++ x = 1; |
| 105 | ++ x += 1; |
| 106 | ++} |
| 107 | ++ |
| 108 | ++#define CHECK(REG) "\tcmp\tr0, " #REG "\n\tbeq\t1f\n" |
| 109 | ++ |
| 110 | ++asm ( |
| 111 | ++" .data\n" |
| 112 | ++" .align 3\n" |
| 113 | ++" .globl stack_chk_guard_ptr\n" |
| 114 | ++"stack_chk_guard_ptr:\n" |
| 115 | ++" .word __stack_chk_guard\n" |
| 116 | ++" .weak __stack_chk_guard\n" |
| 117 | ++"__stack_chk_guard:\n" |
| 118 | ++" .word 0xdead4321\n" |
| 119 | ++" .text\n" |
| 120 | ++" .globl main\n" |
| 121 | ++" .type main, %function\n" |
| 122 | ++"main:\n" |
| 123 | ++" bl get_ptr\n" |
| 124 | ++" str r0, [sp, #-8]!\n" |
| 125 | ++" bl f\n" |
| 126 | ++" str r0, [sp, #4]\n" |
| 127 | ++" ldr r0, [sp]\n" |
| 128 | ++" ldr r0, [r0]\n" |
| 129 | ++ CHECK (r1) |
| 130 | ++ CHECK (r2) |
| 131 | ++ CHECK (r3) |
| 132 | ++ CHECK (r4) |
| 133 | ++ CHECK (r5) |
| 134 | ++ CHECK (r6) |
| 135 | ++ CHECK (r7) |
| 136 | ++ CHECK (r8) |
| 137 | ++ CHECK (r9) |
| 138 | ++ CHECK (r10) |
| 139 | ++ CHECK (r11) |
| 140 | ++ CHECK (r12) |
| 141 | ++ CHECK (r13) |
| 142 | ++ CHECK (r14) |
| 143 | ++" ldr r1, [sp, #4]\n" |
| 144 | ++ CHECK (r1) |
| 145 | ++" mov r0, #0\n" |
| 146 | ++" b exit\n" |
| 147 | ++"1:\n" |
| 148 | ++" b abort\n" |
| 149 | ++" .size main, .-main" |
| 150 | ++); |
| 151 | +diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-2.c b/gcc/testsuite/gcc.target/arm/stack-protector-2.c |
| 152 | +new file mode 100644 |
| 153 | +index 00000000000..266c36fdbc6 |
| 154 | +--- /dev/null |
| 155 | ++++ b/gcc/testsuite/gcc.target/arm/stack-protector-2.c |
| 156 | +@@ -0,0 +1,6 @@ |
| 157 | ++/* { dg-do run } */ |
| 158 | ++/* { dg-require-effective-target fstack_protector } */ |
| 159 | ++/* { dg-require-effective-target fpic } */ |
| 160 | ++/* { dg-options "-fstack-protector-all -O2 -fpic" } */ |
| 161 | ++ |
| 162 | ++#include "stack-protector-1.c" |
| 163 | +-- |
| 164 | +2.29.2 |
| 165 | + |
0 commit comments