[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <b7aad232-76e1-241f-00e2-77783ce30f87@linux.com>
Date: Wed, 14 Nov 2018 21:24:28 +0300
From: Alexander Popov <alex.popov@...ux.com>
To: Kees Cook <keescook@...omium.org>, Ingo Molnar <mingo@...nel.org>,
Andy Lutomirski <luto@...nel.org>, Tycho Andersen <tycho@...ho.ws>,
Laura Abbott <labbott@...hat.com>, Mark Rutland <mark.rutland@....com>,
Ard Biesheuvel <ard.biesheuvel@...aro.org>, Borislav Petkov <bp@...en8.de>,
Richard Sandiford <richard.sandiford@....com>,
Thomas Gleixner <tglx@...utronix.de>, "H . Peter Anvin" <hpa@...or.com>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>, Emese Revfy <re.emese@...il.com>,
Thomas Garnier <thgarnie@...gle.com>, Alexei Starovoitov <ast@...nel.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
"David S . Miller" <davem@...emloft.net>,
Steven Rostedt <rostedt@...dmis.org>,
Dave Hansen <dave.hansen@...ux.intel.com>, Will Deacon
<will.deacon@....com>, Florian Weimer <fweimer@...hat.com>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>,
linux-arm-kernel@...ts.infradead.org, gcc-bugs@....gnu.org,
gcc-help@....gnu.org
Subject: Investigating a stack state mismatch in Linux kernel
Hello everyone!
I'm investigating an interesting issue with gcc-5 compilation of Linux kernel,
which is triggered by stackleak gcc plugin.
I would really appreciate any help (especially from gcc experts)!
The kbuild test robot reported a build warning for some special kernel config:
https://lists.01.org/pipermail/kbuild-all/2018-November/054567.html
drivers/acpi/acpi_processor.o: warning: objtool:
acpi_duplicate_processor_id()+0x49: stack state mismatch: cfa1=7+8 cfa2=6+16
The compiler misses pop instructions at the end of the function for some
execution path. That makes the frame pointer state inconsistent.
I can reproduce this issue for gcc 5.4.0 (Ubuntu 5.4.0-6ubuntu1~16.04.10). The
newer gcc 7.3.0 (Ubuntu 7.3.0-16ubuntu3) doesn't make this mistake.
This bug happens after stackleak gcc plugin worked with RTL representation of
the code. This plugin registers the 'stackleak_cleanup' pass after the 'reload'
pass, when the stack frame size is final. The plugin does a simple thing: it
removes the stackleak_track_stack() call from the functions with a small stack
frame, it uses delete_insn_and_edges() to do that.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2d6bb6adb714b133db92ccd4bfc9c20f75f71f3f
But after this CALL insn is deleted, gcc-5 misses the pop instruction during RTL
optimization :/
The compiled C function is not complicated:
bool acpi_duplicate_processor_id(int proc_id)
{
int i;
/*
* compare the proc_id with duplicate IDs, if the proc_id is already
* in the duplicate IDs, return true, otherwise, return false.
*/
for (i = 0; i < nr_duplicate_ids; i++) {
if (duplicate_processor_ids[i] == proc_id)
return true;
}
return false;
}
Let me show you asm listings.
1. GCC-5: without removing stackleak_track_stack() call, stack frame pointer is
consistent.
ffffffff8144a170 <acpi_duplicate_processor_id>:
ffffffff8144a170: 53 push %rbx
ffffffff8144a171: 89 fb mov %edi,%ebx
ffffffff8144a173: e8 88 33 d2 ff callq ffffffff8116d500
ffffffff8144a178: 8b 15 e6 68 46 02 mov 0x24668e6(%rip),%edx
ffffffff8144a17e: 48 83 05 9a 68 46 02 addq $0x1,0x246689a(%rip)
ffffffff8144a185: 01
ffffffff8144a186: 85 d2 test %edx,%edx
ffffffff8144a188: 7e 32 jle ffffffff8144a1bc
ffffffff8144a18a: 48 8b 05 9f 68 46 02 mov 0x246689f(%rip),%rax
ffffffff8144a191: 3b 1d d1 99 19 01 cmp 0x11999d1(%rip),%ebx
ffffffff8144a197: 48 8d 48 01 lea 0x1(%rax),%rcx
ffffffff8144a19b: 48 89 0d 8e 68 46 02 mov %rcx,0x246688e(%rip)
ffffffff8144a1a2: 74 1c je ffffffff8144a1c0
ffffffff8144a1a4: 48 83 05 7c 68 46 02 addq $0x1,0x246687c(%rip)
ffffffff8144a1ab: 01
ffffffff8144a1ac: 83 fa 01 cmp $0x1,%edx
ffffffff8144a1af: 74 0b je ffffffff8144a1bc
ffffffff8144a1b1: 48 83 c0 02 add $0x2,%rax
ffffffff8144a1b5: 48 89 05 74 68 46 02 mov %rax,0x2466874(%rip)
ffffffff8144a1bc: 31 c0 xor %eax,%eax
ffffffff8144a1be: 5b pop %rbx
ffffffff8144a1bf: c3 retq
ffffffff8144a1c0: b8 01 00 00 00 mov $0x1,%eax
ffffffff8144a1c5: 5b pop %rbx
ffffffff8144a1c6: c3 retq
2. GCC-5: with removed stackleak_track_stack() call, stack frame pointer IS NOT
consistent.
ffffffff8144a170 <acpi_duplicate_processor_id>:
ffffffff8144a170: 8b 15 ee 68 46 02 mov 0x24668ee(%rip),%edx
ffffffff8144a176: 48 83 05 a2 68 46 02 addq $0x1,0x24668a2(%rip)
ffffffff8144a17d: 01
ffffffff8144a17e: 85 d2 test %edx,%edx
ffffffff8144a180: 7e 33 jle ffffffff8144a1b5
ffffffff8144a182: 48 8b 05 a7 68 46 02 mov 0x24668a7(%rip),%rax
ffffffff8144a189: 3b 3d d9 99 19 01 cmp 0x11999d9(%rip),%edi
ffffffff8144a18f: 53 push %rbx
ffffffff8144a190: 48 8d 48 01 lea 0x1(%rax),%rcx
ffffffff8144a194: 48 89 0d 95 68 46 02 mov %rcx,0x2466895(%rip)
ffffffff8144a19b: 74 1f je ffffffff8144a1bc
ffffffff8144a19d: 48 83 05 83 68 46 02 addq $0x1,0x2466883(%rip)
ffffffff8144a1a4: 01
ffffffff8144a1a5: 83 fa 01 cmp $0x1,%edx
ffffffff8144a1a8: 74 0e je ffffffff8144a1b8
ffffffff8144a1aa: 48 83 c0 02 add $0x2,%rax
ffffffff8144a1ae: 48 89 05 7b 68 46 02 mov %rax,0x246687b(%rip)
ffffffff8144a1b5: 31 c0 xor %eax,%eax
ffffffff8144a1b7: c3 retq # BUG HERE! POP IS ABSENT!
ffffffff8144a1b8: 31 c0 xor %eax,%eax
ffffffff8144a1ba: 5b pop %rbx
ffffffff8144a1bb: c3 retq
ffffffff8144a1bc: b8 01 00 00 00 mov $0x1,%eax
ffffffff8144a1c1: 5b pop %rbx
ffffffff8144a1c2: c3 retq
3. GCC-7: with removed stackleak_track_stack() call, stack frame pointer is
consistent, NO BUG.
ffffffff8144be60 <acpi_duplicate_processor_id>:
ffffffff8144be60: 8b 15 3e d5 44 02 mov 0x244d53e(%rip),%edx
ffffffff8144be66: 48 83 05 f2 d4 44 02 addq $0x1,0x244d4f2(%rip)
ffffffff8144be6d: 01
ffffffff8144be6e: 85 d2 test %edx,%edx
ffffffff8144be70: 7e 3e jle ffffffff8144beb0
ffffffff8144be72: 48 8b 05 f7 d4 44 02 mov 0x244d4f7(%rip),%rax
ffffffff8144be79: 3b 3d e9 79 19 01 cmp 0x11979e9(%rip),%edi
ffffffff8144be7f: 53 push %rbx
ffffffff8144be80: 48 8d 48 01 lea 0x1(%rax),%rcx
ffffffff8144be84: 48 89 0d e5 d4 44 02 mov %rcx,0x244d4e5(%rip)
ffffffff8144be8b: 74 1c je ffffffff8144bea9
ffffffff8144be8d: 48 83 05 d3 d4 44 02 addq $0x1,0x244d4d3(%rip)
ffffffff8144be94: 01
ffffffff8144be95: 83 fa 01 cmp $0x1,%edx
ffffffff8144be98: 74 0b je ffffffff8144bea5
ffffffff8144be9a: 48 83 c0 02 add $0x2,%rax
ffffffff8144be9e: 48 89 05 cb d4 44 02 mov %rax,0x244d4cb(%rip)
ffffffff8144bea5: 31 c0 xor %eax,%eax
ffffffff8144bea7: 5b pop %rbx
ffffffff8144bea8: c3 retq
ffffffff8144bea9: b8 01 00 00 00 mov $0x1,%eax
ffffffff8144beae: 5b pop %rbx
ffffffff8144beaf: c3 retq
ffffffff8144beb0: 31 c0 xor %eax,%eax
ffffffff8144beb2: c3 retq
Of course, there is a naive solution for this issue -- just skip stackleak
instrumentation for acpi_duplicate_processor_id(). But it would be great to find
out the reasons behind this compiler behavior. It might help to create a better
solution.
Right now I don't have answers for the following questions:
1. Which gcc patch fixed this error in RTL optimization?
2. What is so special about acpi_duplicate_processor_id(), that triggers this
bug? Maybe we can determine that particular feature of C code and skip the
instrumentation for all similar functions?
3. Can stackleak plugin mitigate this bug in gcc-5? Maybe we can register this
pass to work with RTL later, after the buggy optimization?
I would really appreciate your help!
Best regards,
Alexander
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
