Christophe Lyon <christophe.l...@linaro.org> writes: > On Wed, 5 Aug 2020 at 16:33, Richard Sandiford > <richard.sandif...@arm.com> wrote: >> >> The stack_protect_test patterns were leaving the canary value in the >> temporary register, meaning that it was often still in registers on >> return from the function. An attacker might therefore have been >> able to use it to defeat stack-smash protection for a later function. >> >> Tested on arm-linux-gnueabi, arm-linux-gnueabihf and armeb-eabi. >> I tested the thumb1.md part using arm-linux-gnueabi with the >> test flags -march=armv5t -mthumb. OK for trunk and branches? >> >> As I mentioned in the corresponding aarch64 patch, this is needed >> to make arm conform to GCC's current -fstack-protector implementation. >> However, I think we should reconsider whether the zeroing is actually >> necessary and what it's actually protecting against. I'll send a >> separate message about that to gcc@. But since the port isn't even >> self-consistent (the *set patterns do clear the registers), I think >> we should do this first rather than wait for any outcome of that >> discussion. >> >> Richard >> >> >> gcc/ >> PR target/96191 >> * config/arm/arm.md (arm_stack_protect_test_insn): Zero out >> operand 2 after use. >> * config/arm/thumb1.md (thumb1_stack_protect_test_insn): Likewise. >> >> gcc/testsuite/ >> * gcc.target/arm/stack-protector-1.c: New test. >> * gcc.target/arm/stack-protector-2.c: Likewise. > > Hi Richard, > > The new tests fail when compiled with -mcpu=cortex-mXX because gas complains: > use of r13 is deprecated > It has a comment saying: "In the Thumb-2 ISA, use of R13 as Rm is > deprecated, but valid." > > It's a minor nuisance, I'm not sure what the best way of getting rid of it? > Add #ifndef __thumb2__ around CHECK(r13) ?
Hmm, maybe we should just drop that line altogether. It wasn't exactly likely that r13 would be the register to leak the value :-) Should I post a patch or do you already have one ready? Thanks, Richard
