Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

[Josh Poimboeuf <jpoimboe@redhat.com>]

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.

Thanks!

-- 
Josh