Message-ID: <20150520155156.GB16995@treble.redhat.com>
Date:	Wed, 20 May 2015 10:51:56 -0500
From:	Josh Poimboeuf <jpoimboe@...hat.com>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, Michal Marek <mmarek@...e.cz>,
	Peter Zijlstra <peterz@...radead.org>, x86@...nel.org,
	live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andy Lutomirski <luto@...nel.org>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Brian Gerst <brgerst@...il.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Borislav Petkov <bp@...en8.de>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Wed, May 20, 2015 at 04:48:10PM +0200, Ingo Molnar wrote:
> 
> * Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> 
> > On Wed, May 20, 2015 at 12:33:39PM +0200, Ingo Molnar wrote:
> > > 
> > > * Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> > > 
> > > > In discussions around the live kernel patching consistency model RFC
> > > > [1], Peter and Ingo correctly pointed out that stack traces aren't
> > > > reliable.  And as Ingo said, there's no "strong force" which ensures we
> > > > can rely on them.
> > > > 
> > > > So I've been thinking about how to fix that.  My goal is to eventually
> > > > make stack traces reliable.  Or at the very least, to be able to detect
> > > > at runtime when a given stack trace *might* be unreliable.  But improved
> > > > stack traces would broadly benefit the entire kernel, regardless of the
> > > > outcome of the live kernel patching consistency model discussions.
> > > > 
> > > > This patch set is just the first in a series of proposed stack trace
> > > > reliability improvements.  Future proposals will include runtime stack
> > > > reliability checking, as well as compile-time and runtime DWARF
> > > > validations.
> > > > 
> > > > As far as I can tell, there are two main obstacles which prevent frame
> > > > pointer based stack traces from being reliable:
> > > > 
> > > > 1) Missing frame pointer logic: currently, most assembly functions don't
> > > >    set up the frame pointer.
> > > 
> > > Could you please paste here the output of what the new checks print 
> > > for x86/64 defconfig?
> > 
> > Here are all 89 warnings from defconfig:
> > 
> > arch/x86/ia32/ia32entry.o: ia32_sysenter_target() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/ia32/ia32entry.o: return instruction outside of a function at .entry.text+0x52e.  Please use FUNC_ENTER.
> > arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x359.  Please use FUNC_ENTER.
> > arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19be.  Please use FUNC_ENTER.
> > arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19e5.  Please use FUNC_ENTER.
> > arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1c21.  Please use FUNC_ENTER.
> > arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1ceb.  Please use FUNC_ENTER.
> > arch/x86/kernel/acpi/wakeup_64.o: wakeup_long64() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/platform/efi/efi_stub_64.o: efi_call() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x6b.  Please use FUNC_ENTER.
> > arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0xc7.  Please use FUNC_ENTER.
> > arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x110.  Please use FUNC_ENTER.
> > arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x145.  Please use FUNC_ENTER.
> > arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x1c4.  Please use FUNC_ENTER.
> > arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x170.  Please use FUNC_ENTER.
> > arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x176.  Please use FUNC_ENTER.
> > arch/x86/kernel/head_64.o: return instruction outside of a function at .head.text+0x1a2.  Please use FUNC_ENTER.
> > arch/x86/kernel/head_64.o: start_cpu0() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/kernel/head_64.o: early_idt_handler() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/realmode/rm/reboot.o: return instruction outside of a function at .text+0x2a.  Please use FUNC_ENTER.
> > arch/x86/realmode/rm/copy.o: memcpy() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/realmode/rm/copy.o: memset() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/realmode/rm/copy.o: copy_from_fs() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/realmode/rm/copy.o: copy_to_fs() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/realmode/rm/bioscall.o: intcall() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/int80.o: __kernel_sigreturn() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/int80.o: __kernel_rt_sigreturn() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/int80.o: __kernel_vsyscall() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/syscall.o: __kernel_sigreturn() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/syscall.o: __kernel_rt_sigreturn() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/syscall.o: __kernel_vsyscall() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/sysenter.o: __kernel_sigreturn() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/sysenter.o: __kernel_rt_sigreturn() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x69.  Please use FUNC_ENTER.
> > arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x16d.  Please use FUNC_ENTER.
> > arch/x86/lib/msr-reg.o: rdmsr_safe_regs() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/msr-reg.o: wrmsr_safe_regs() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/iomap_copy_64.o: __iowrite32_copy() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/clear_page_64.o: clear_page() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/clear_page_64.o: clear_page_orig() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/clear_page_64.o: clear_page_c_e() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/cmpxchg16b_emu.o: this_cpu_cmpxchg16b_emu() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/copy_page_64.o: copy_page() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/copy_page_64.o: copy_page_regs() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/copy_user_64.o: _copy_to_user() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/copy_user_64.o: _copy_from_user() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/copy_user_64.o: copy_user_generic_unrolled() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/copy_user_64.o: copy_user_generic_string() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/copy_user_64.o: copy_user_enhanced_fast_string() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/copy_user_64.o: __copy_user_nocache() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/copy_user_64.o: bad_from_user() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/csum-copy_64.o: csum_partial_copy_generic() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/getuser.o: __get_user_1() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/getuser.o: __get_user_2() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/getuser.o: __get_user_4() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/getuser.o: __get_user_8() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/getuser.o: return instruction outside of a function at .text+0xc5.  Please use FUNC_ENTER.
> > arch/x86/lib/memcpy_64.o: memcpy() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/memcpy_64.o: __memcpy() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/memcpy_64.o: memcpy_erms() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/memcpy_64.o: memcpy_orig() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/memmove_64.o: memmove() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/memmove_64.o: __memmove() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/memmove_64.o: return instruction outside of a function at .altinstr_replacement+0x5.  Please use FUNC_ENTER.
> > arch/x86/lib/memset_64.o: memset() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/memset_64.o: __memset() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/memset_64.o: memset_erms() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/memset_64.o: memset_orig() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/putuser.o: __put_user_1() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/putuser.o: __put_user_2() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/putuser.o: __put_user_4() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/putuser.o: __put_user_8() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/putuser.o: return instruction outside of a function at .text+0xc1.  Please use FUNC_ENTER.
> > arch/x86/lib/rwsem.o: call_rwsem_down_read_failed() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/rwsem.o: call_rwsem_down_write_failed() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/rwsem.o: call_rwsem_wake() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/lib/rwsem.o: call_rwsem_downgrade_wake() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/boot/bioscall.o: intcall() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/boot/copy.o: memcpy() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/boot/copy.o: memset() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/boot/copy.o: copy_from_fs() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/boot/copy.o: copy_to_fs() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/boot/pmjump.o: protected_mode_jump() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/boot/pmjump.o: in_pm32() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x16e.  Please use FUNC_ENTER.
> > arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x172.  Please use FUNC_ENTER.
> > arch/x86/boot/compressed/head_64.o: startup_32() is missing frame pointer logic.  Please use FUNC_ENTER.
> > arch/x86/boot/header.o: die() is missing frame pointer logic.  Please use FUNC_ENTER.
> 
> Yeah, so many of these seem to be 'leaf only' functions: functions 
> that don't ever call functions themselves.

Yeah, good observation.

> So lets assume we always have CONFIG_FRAME_POINTERS=y.
> 
> If they don't set up a frame pointer then they in essence won't show 
> up in the call chain

It's actually the _caller_ of the asm function which gets skipped in the
trace.

(Though it doesn't really matter -- either way it's unreliable.)

> but normally they wouldn't because they call nothing.
> 
> If they trigger an exception/fault or if they get hit by an interrupt 
> then I think we'll still correctly walk the stack - just those 
> functions might be missing from the deterministic call chain, right? 
> (it will still show up as a '?' entry.)

Right.  This patch set takes the more conservative approach of requiring
_all_ callable asm functions to have frame pointer logic.  Which has the
benefit of getting rid of some of the cases where we need the '?' stack
entries.

> If they crash then we'll see them because the crashing RIP will be 
> printed.
> 
> So I'm wondering what the x86 policy here should be: to create frame 
> pointers in them or not. Cc:-ed a few more gents for thoughts.

I agree that frame pointers aren't strictly necessary for leaf
functions.

I could easily relax the stackvalidate restrictions to exclude the
checking of leaf functions.  In fact I think that would be more
consistent with how gcc does it, so maybe that's a more reasonable
approach.

-- 
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists