public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/96191] New: aarch64 stack_protect_test canary leak
@ 2020-07-14 2:49 wilson at gcc dot gnu.org
2020-07-14 8:22 ` [Bug target/96191] " rsandifo at gcc dot gnu.org
` (9 more replies)
0 siblings, 10 replies; 11+ messages in thread
From: wilson at gcc dot gnu.org @ 2020-07-14 2:49 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96191
Bug ID: 96191
Summary: aarch64 stack_protect_test canary leak
Product: gcc
Version: 11.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: wilson at gcc dot gnu.org
Target Milestone: ---
Given a simple testcase
extern int sub (int);
int
main (void)
{
sub (10);
return 0;
}
commpiling with -O -S -fstack-protector-all -mstack-protector-guard=global
in the epilogue for the canary check I see
ldr x1, [sp, 40]
ldr x0, [x19, #:lo12:__stack_chk_guard]
eor x0, x1, x0
cbnz x0, .L4
Both x0 and x1 have the stack protector canary loaded into them, and the eor
clobbers x0, but x1 is left alone. This means the value of the canary is
leaking from the epilogue. The canary value is never supposed to survive in a
register outside the stack protector patterns.
A powerpc64-linux toolchain build with the same testcase and options generates
lwz 9,28(1)
lwz 10,0(31)
xor. 9,9,10
li 10,0
bne- 0,.L4
and note that it clears the second register after the xor to prevent the canary
leak. The aarch64 stack_protect_test pattern should do the same thing.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/96191] aarch64 stack_protect_test canary leak
2020-07-14 2:49 [Bug target/96191] New: aarch64 stack_protect_test canary leak wilson at gcc dot gnu.org
@ 2020-07-14 8:22 ` rsandifo at gcc dot gnu.org
2020-07-14 18:58 ` wilco at gcc dot gnu.org
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: rsandifo at gcc dot gnu.org @ 2020-07-14 8:22 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96191
rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|unassigned at gcc dot gnu.org |rsandifo at gcc dot gnu.org
Ever confirmed|0 |1
Status|UNCONFIRMED |ASSIGNED
CC| |rsandifo at gcc dot gnu.org
Last reconfirmed| |2020-07-14
--- Comment #1 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Thanks for the report. I'm testing a patch.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/96191] aarch64 stack_protect_test canary leak
2020-07-14 2:49 [Bug target/96191] New: aarch64 stack_protect_test canary leak wilson at gcc dot gnu.org
2020-07-14 8:22 ` [Bug target/96191] " rsandifo at gcc dot gnu.org
@ 2020-07-14 18:58 ` wilco at gcc dot gnu.org
2020-07-14 22:30 ` wilson at gcc dot gnu.org
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: wilco at gcc dot gnu.org @ 2020-07-14 18:58 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96191
Wilco <wilco at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |wilco at gcc dot gnu.org
--- Comment #2 from Wilco <wilco at gcc dot gnu.org> ---
(In reply to Jim Wilson from comment #0)
> Given a simple testcase
> extern int sub (int);
>
> int
> main (void)
> {
> sub (10);
> return 0;
> }
> commpiling with -O -S -fstack-protector-all -mstack-protector-guard=global
> in the epilogue for the canary check I see
> ldr x1, [sp, 40]
> ldr x0, [x19, #:lo12:__stack_chk_guard]
> eor x0, x1, x0
> cbnz x0, .L4
> Both x0 and x1 have the stack protector canary loaded into them, and the eor
> clobbers x0, but x1 is left alone. This means the value of the canary is
> leaking from the epilogue. The canary value is never supposed to survive in
> a register outside the stack protector patterns.
>
> A powerpc64-linux toolchain build with the same testcase and options
> generates
> lwz 9,28(1)
> lwz 10,0(31)
> xor. 9,9,10
> li 10,0
> bne- 0,.L4
> and note that it clears the second register after the xor to prevent the
> canary leak. The aarch64 stack_protect_test pattern should do the same
> thing.
The canary value is not a secret. What would the purpose of clearing the
register be given the stack slot containing the canary is not cleared as well?
And register could potentially contain the address of the canary or that of a
global nearby, making reading the canary value really easy.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/96191] aarch64 stack_protect_test canary leak
2020-07-14 2:49 [Bug target/96191] New: aarch64 stack_protect_test canary leak wilson at gcc dot gnu.org
2020-07-14 8:22 ` [Bug target/96191] " rsandifo at gcc dot gnu.org
2020-07-14 18:58 ` wilco at gcc dot gnu.org
@ 2020-07-14 22:30 ` wilson at gcc dot gnu.org
2020-08-05 14:19 ` cvs-commit at gcc dot gnu.org
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: wilson at gcc dot gnu.org @ 2020-07-14 22:30 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96191
--- Comment #3 from Jim Wilson <wilson at gcc dot gnu.org> ---
The location of the canary is not known to the attacker. You are not supposed
to leak the address of the canary or the value of the canary. If you leak
either, then an attacker has a chance to restore the canary after clobbering
it.
See the descriptions of the stack_protect_set and stack_protect_test patterns
in gcc/doc/md.texi which make clear that no intermediate values should be
allowed to survive past the end of the pattern.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/96191] aarch64 stack_protect_test canary leak
2020-07-14 2:49 [Bug target/96191] New: aarch64 stack_protect_test canary leak wilson at gcc dot gnu.org
` (2 preceding siblings ...)
2020-07-14 22:30 ` wilson at gcc dot gnu.org
@ 2020-08-05 14:19 ` cvs-commit at gcc dot gnu.org
2020-08-06 18:20 ` cvs-commit at gcc dot gnu.org
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-08-05 14:19 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96191
--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Sandiford <rsandifo@gcc.gnu.org>:
https://gcc.gnu.org/g:fe1a26429038d7cd17abc53f96a6f3e2639b605f
commit r11-2576-gfe1a26429038d7cd17abc53f96a6f3e2639b605f
Author: Richard Sandiford <richard.sandiford@arm.com>
Date: Wed Aug 5 15:18:36 2020 +0100
aarch64: Clear canary value after stack_protect_test [PR96191]
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.
gcc/
PR target/96191
* config/aarch64/aarch64.md (stack_protect_test_<mode>): Set the
CC register directly, instead of a GPR. Replace the original GPR
destination with an extra scratch register. Zero out operand 3
after use.
(stack_protect_test): Update accordingly.
gcc/testsuite/
PR target/96191
* gcc.target/aarch64/stack-protector-1.c: New test.
* gcc.target/aarch64/stack-protector-2.c: Likewise.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/96191] aarch64 stack_protect_test canary leak
2020-07-14 2:49 [Bug target/96191] New: aarch64 stack_protect_test canary leak wilson at gcc dot gnu.org
` (3 preceding siblings ...)
2020-08-05 14:19 ` cvs-commit at gcc dot gnu.org
@ 2020-08-06 18:20 ` cvs-commit at gcc dot gnu.org
2020-08-07 11:15 ` cvs-commit at gcc dot gnu.org
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-08-06 18:20 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96191
--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Sandiford <rsandifo@gcc.gnu.org>:
https://gcc.gnu.org/g:6a3f3e08723063ea2dadb7ddf503f02972a724e2
commit r11-2598-g6a3f3e08723063ea2dadb7ddf503f02972a724e2
Author: Richard Sandiford <richard.sandiford@arm.com>
Date: Thu Aug 6 19:19:41 2020 +0100
arm: Clear canary value after stack_protect_test [PR96191]
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.
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/96191] aarch64 stack_protect_test canary leak
2020-07-14 2:49 [Bug target/96191] New: aarch64 stack_protect_test canary leak wilson at gcc dot gnu.org
` (4 preceding siblings ...)
2020-08-06 18:20 ` cvs-commit at gcc dot gnu.org
@ 2020-08-07 11:15 ` cvs-commit at gcc dot gnu.org
2020-08-07 11:15 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-08-07 11:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96191
--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Richard Sandiford
<rsandifo@gcc.gnu.org>:
https://gcc.gnu.org/g:bab5fdaf9abb1236a7a56258d1d36265068b4827
commit r10-8583-gbab5fdaf9abb1236a7a56258d1d36265068b4827
Author: Richard Sandiford <richard.sandiford@arm.com>
Date: Fri Aug 7 12:15:01 2020 +0100
aarch64: Clear canary value after stack_protect_test [PR96191]
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.
gcc/
PR target/96191
* config/aarch64/aarch64.md (stack_protect_test_<mode>): Set the
CC register directly, instead of a GPR. Replace the original GPR
destination with an extra scratch register. Zero out operand 3
after use.
(stack_protect_test): Update accordingly.
gcc/testsuite/
PR target/96191
* gcc.target/aarch64/stack-protector-1.c: New test.
* gcc.target/aarch64/stack-protector-2.c: Likewise.
(cherry picked from commit fe1a26429038d7cd17abc53f96a6f3e2639b605f)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/96191] aarch64 stack_protect_test canary leak
2020-07-14 2:49 [Bug target/96191] New: aarch64 stack_protect_test canary leak wilson at gcc dot gnu.org
` (5 preceding siblings ...)
2020-08-07 11:15 ` cvs-commit at gcc dot gnu.org
@ 2020-08-07 11:15 ` cvs-commit at gcc dot gnu.org
2020-08-07 11:18 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-08-07 11:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96191
--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Richard Sandiford
<rsandifo@gcc.gnu.org>:
https://gcc.gnu.org/g:8f6b7c9796035ad8b2cdfbce5d3d11dd4b81fad7
commit r10-8584-g8f6b7c9796035ad8b2cdfbce5d3d11dd4b81fad7
Author: Richard Sandiford <richard.sandiford@arm.com>
Date: Fri Aug 7 12:15:02 2020 +0100
arm: Clear canary value after stack_protect_test [PR96191]
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.
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.
(cherry picked from commit 6a3f3e08723063ea2dadb7ddf503f02972a724e2)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/96191] aarch64 stack_protect_test canary leak
2020-07-14 2:49 [Bug target/96191] New: aarch64 stack_protect_test canary leak wilson at gcc dot gnu.org
` (6 preceding siblings ...)
2020-08-07 11:15 ` cvs-commit at gcc dot gnu.org
@ 2020-08-07 11:18 ` cvs-commit at gcc dot gnu.org
2020-08-07 11:18 ` cvs-commit at gcc dot gnu.org
2020-11-17 18:17 ` cvs-commit at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-08-07 11:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96191
--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Richard Sandiford
<rsandifo@gcc.gnu.org>:
https://gcc.gnu.org/g:5380912a17ea09a8996720fb62b1a70c16c8f9f2
commit r9-8794-g5380912a17ea09a8996720fb62b1a70c16c8f9f2
Author: Richard Sandiford <richard.sandiford@arm.com>
Date: Fri Aug 7 12:17:37 2020 +0100
aarch64: Clear canary value after stack_protect_test [PR96191]
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.
gcc/
PR target/96191
* config/aarch64/aarch64.md (stack_protect_test_<mode>): Set the
CC register directly, instead of a GPR. Replace the original GPR
destination with an extra scratch register. Zero out operand 3
after use.
(stack_protect_test): Update accordingly.
gcc/testsuite/
PR target/96191
* gcc.target/aarch64/stack-protector-1.c: New test.
* gcc.target/aarch64/stack-protector-2.c: Likewise.
(cherry picked from commit fe1a26429038d7cd17abc53f96a6f3e2639b605f)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/96191] aarch64 stack_protect_test canary leak
2020-07-14 2:49 [Bug target/96191] New: aarch64 stack_protect_test canary leak wilson at gcc dot gnu.org
` (7 preceding siblings ...)
2020-08-07 11:18 ` cvs-commit at gcc dot gnu.org
@ 2020-08-07 11:18 ` cvs-commit at gcc dot gnu.org
2020-11-17 18:17 ` cvs-commit at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-08-07 11:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96191
--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Richard Sandiford
<rsandifo@gcc.gnu.org>:
https://gcc.gnu.org/g:3e40be9cc92d3fa117be7f4fab07cedeed8361a2
commit r9-8795-g3e40be9cc92d3fa117be7f4fab07cedeed8361a2
Author: Richard Sandiford <richard.sandiford@arm.com>
Date: Fri Aug 7 12:17:37 2020 +0100
arm: Clear canary value after stack_protect_test [PR96191]
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.
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.
(cherry picked from commit 6a3f3e08723063ea2dadb7ddf503f02972a724e2)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/96191] aarch64 stack_protect_test canary leak
2020-07-14 2:49 [Bug target/96191] New: aarch64 stack_protect_test canary leak wilson at gcc dot gnu.org
` (8 preceding siblings ...)
2020-08-07 11:18 ` cvs-commit at gcc dot gnu.org
@ 2020-11-17 18:17 ` cvs-commit at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-11-17 18:17 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96191
--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Richard Sandiford
<rsandifo@gcc.gnu.org>:
https://gcc.gnu.org/g:3ee527923b1ce92c6b16c587d072720a6c813c95
commit r8-10627-g3ee527923b1ce92c6b16c587d072720a6c813c95
Author: Richard Sandiford <richard.sandiford@arm.com>
Date: Tue Nov 17 18:16:45 2020 +0000
aarch64: Clear canary value after stack_protect_test [PR96191]
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.
gcc/
PR target/96191
* config/aarch64/aarch64.md (stack_protect_test_<mode>): Set the
CC register directly, instead of a GPR. Replace the original GPR
destination with an extra scratch register. Zero out operand 3
after use.
(stack_protect_test): Update accordingly.
gcc/testsuite/
PR target/96191
* gcc.target/aarch64/stack-protector-1.c: New test.
* gcc.target/aarch64/stack-protector-2.c: Likewise.
(cherry picked from commit fe1a26429038d7cd17abc53f96a6f3e2639b605f)
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-11-17 18:17 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-14 2:49 [Bug target/96191] New: aarch64 stack_protect_test canary leak wilson at gcc dot gnu.org
2020-07-14 8:22 ` [Bug target/96191] " rsandifo at gcc dot gnu.org
2020-07-14 18:58 ` wilco at gcc dot gnu.org
2020-07-14 22:30 ` wilson at gcc dot gnu.org
2020-08-05 14:19 ` cvs-commit at gcc dot gnu.org
2020-08-06 18:20 ` cvs-commit at gcc dot gnu.org
2020-08-07 11:15 ` cvs-commit at gcc dot gnu.org
2020-08-07 11:15 ` cvs-commit at gcc dot gnu.org
2020-08-07 11:18 ` cvs-commit at gcc dot gnu.org
2020-08-07 11:18 ` cvs-commit at gcc dot gnu.org
2020-11-17 18:17 ` cvs-commit at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).