Re: [PATCH, ARM, ping3] PR85434: Prevent spilling of stack protector guard's address on ARM

[Thomas Preudhomme <thomas dot preudhomme at linaro dot org>]

I'm talking about the PIC access to the guard's variable. See for
example the pr85434.c testcase contributed with this patch when
compiled for aarch64 with -Os -fpic -march=armv8-a
-fstack-protector-strong:

(insn 227 226 228 33 (set (reg:DI 90)
        (high:DI (symbol_ref:DI ("_GLOBAL_OFFSET_TABLE_"))))
"/data/dev/checkouts/private/linaro/gcc/gcc/testsuite/gcc.target/arm/pr85434.c":148:1
-1
     (nil))
(insn 228 227 229 33 (set (reg/f:DI 244)
        (unspec:DI [
                (mem/u/c:DI (lo_sum:DI (reg:DI 90)
                        (symbol_ref:DI ("__stack_chk_guard") [flags
0xc0]  <var_decl 0x7f93f8778750 __stack_chk_guard>)) [0  S8 A8])
            ] UNSPEC_GOTSMALLPIC28K))
"/data/dev/checkouts/private/linaro/gcc/gcc/testsuite/gcc.target/arm/pr85434.c":148:1
-1
     (expr_list:REG_EQUAL (symbol_ref:DI ("__stack_chk_guard") [flags
0xc0]  <var_decl 0x7f93f8778750 __stack_chk_guard>)
        (nil)))
(insn 229 228 230 33 (parallel [
            (set (reg:DI 245)
                (unspec:DI [
                        (mem/v/f/c:DI (plus:DI (reg/f:DI 85 virtual-stack-vars)
                                (const_int -8 [0xfffffffffffffff8]))
[4 D.3715+0 S8 A64])
                        (mem/v/f/c:DI (reg/f:DI 244) [4
__stack_chk_guard+0 S8 A64])
                    ] UNSPEC_SP_TEST))
            (clobber (scratch:DI))
        ]) "/data/dev/checkouts/private/linaro/gcc/gcc/testsuite/gcc.target/arm/pr85434.c":148:1
-1
     (nil))

The unspec in insn 228 is not CSEd in my experiment despite the same
instruction happening in the prologue to set the canary. In arm
backend it was but the PIC access is of the form (mem (reg) (unspec
offset)), ie the outermost rtx in the source is not an unspec.

Best regards,

Thomas
On Wed, 21 Nov 2018 at 17:54, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Fri, Nov 16, 2018 at 02:56:46PM +0000, Thomas Preudhomme wrote:
> > In case of high register pressure in PIC mode, address of the stack
> > protector's guard can be spilled on ARM targets as shown in PR85434,
> > thus allowing an attacker to control what the canary would be compared
> > against. ARM does lack stack_protect_set and stack_protect_test insn
> > patterns, defining them does not help as the address is expanded
> > regularly and the patterns only deal with the copy and test of the
> > guard with the canary.
> >
> > This problem does not occur for x86 targets because the PIC access and
> > the test can be done in the same instruction. Aarch64 is exempt too
> > because PIC access insn pattern are mov of UNSPEC which prevents it from
> > the second access in the epilogue being CSEd in cse_local pass with the
> > first access in the prologue.
>
> The unspecs are not CSEd because they are *different* unspecs (UNSPEC_SP_SET
> vs. UNSPEC_SP_TEST; they have different args too, different number of args
> even).  Two the same unspecs can be CSEd just fine.
>
>
> Segher