Date:	Thu, 21 May 2015 14:53:07 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Josh Poimboeuf <jpoimboe@...hat.com>
Cc:	Ingo Molnar <mingo@...nel.org>,
	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 ML <x86@...nel.org>,
	live-patching@...r.kernel.org,
	"linux-kernel@...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 Thu, May 21, 2015 at 1:54 PM, Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> On Wed, May 20, 2015 at 04:48:10PM +0200, Ingo Molnar wrote:
>> Yeah, so many of these seem to be 'leaf only' functions: functions
>> that don't ever call functions themselves.
>>
>> 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 - 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.)
>>
>> 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.
>
> After removing the frame pointer checks for leaf functions, and adding a
> check for all functions which jump outside of their scope, the number of
> defconfig warnings dropped from 89 -> 47.  The Fedora config warning
> count dropped from 207 -> 83.
>
> Here are the remaining 47 warnings for defconfig:
>
> stackvalidate: arch/x86/ia32/ia32entry.o: ia32_sysenter_target() is missing frame pointer logic
> stackvalidate: arch/x86/ia32/ia32entry.o: return instruction outside of a function at .entry.text+0x52e
> stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x359
> stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19be
> stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19e5
> stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1c21
> stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1ceb
> stackvalidate: arch/x86/kernel/acpi/wakeup_64.o: unsupported jump to outside of the function at wakeup_long64+0x15
> stackvalidate: arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel() is missing frame pointer logic
> stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x6b
> stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0xc7
> stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x110
> stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x145
> stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x1c4
> stackvalidate: arch/x86/kernel/head_64.o: return instruction outside of a function at .head.text+0x1a2
> stackvalidate: arch/x86/kernel/head_64.o: early_idt_handler() is missing frame pointer logic
> stackvalidate: arch/x86/platform/efi/efi_stub_64.o: efi_call() is missing frame pointer logic
> stackvalidate: arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x170
> stackvalidate: arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x176
> stackvalidate: arch/x86/realmode/rm/reboot.o: return instruction outside of a function at .text+0x2a
> stackvalidate: arch/x86/realmode/rm/copy.o: copy_from_fs() is missing frame pointer logic
> stackvalidate: arch/x86/realmode/rm/copy.o: copy_to_fs() is missing frame pointer logic
> stackvalidate: arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x69
> stackvalidate: arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x16d
> stackvalidate: arch/x86/lib/copy_user_64.o: unsupported jump to outside of the function at _copy_to_user+0x25
> stackvalidate: arch/x86/lib/copy_user_64.o: unsupported jump to outside of the function at _copy_from_user+0x25
> stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_1+0x14
> stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_2+0x4
> stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_4+0x4
> stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_8+0x4
> stackvalidate: arch/x86/lib/getuser.o: return instruction outside of a function at .text+0xc5
> stackvalidate: arch/x86/lib/memmove_64.o: return instruction outside of a function at .altinstr_replacement+0x5
> stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_1+0x14
> stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_2+0x1b
> stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_4+0x1b
> stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_8+0x1b
> stackvalidate: arch/x86/lib/putuser.o: return instruction outside of a function at .text+0xc1
> stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_down_read_failed() is missing frame pointer logic
> stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_down_write_failed() is missing frame pointer logic
> stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_wake() is missing frame pointer logic
> stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_downgrade_wake() is missing frame pointer logic
> stackvalidate: arch/x86/boot/copy.o: copy_from_fs() is missing frame pointer logic
> stackvalidate: arch/x86/boot/copy.o: copy_to_fs() is missing frame pointer logic
> stackvalidate: arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x16e
> stackvalidate: arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x172
> stackvalidate: arch/x86/boot/compressed/head_64.o: startup_32() is missing frame pointer logic
> stackvalidate: arch/x86/boot/pmjump.o: unsupported jump to outside of the function at in_pm32+0x1c
>
> Note that only 13 of the 47 warnings are actually due to missing frame
> pointer logic.  The rest are ambiguous conditions which prevent
> stackvalidate from being able to make sense of things: returning from
> outside of a proper ELF function, or jumping from inside of a function
> to outside of its scope.
>
> Similarly, in the Fedora config case, only 27 of the 83 warnings are for
> missing frame pointer logic.
>
> If there are no objections, I'll go with this approach in the next
> version of the patch set.

I'm willing to review anything with "entry" in its filename.

--Andy
--
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