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

Josh Poimboeuf
2015-05-22T14:54:29Z

On Thu, May 21, 2015 at 02:53:07PM -0700, Andy Lutomirski wrote:
> On Thu, May 21, 2015 at 1:54 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > 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.

Thanks. I think the "entry" warnings may be false positives, since that
code isn't called by any C kernel code. (Now that I'm ignoring leaf
functions, the ratio of false positives to true positives has gone up.)

The false positives for "return instruction outside of a function" can
be marked with the RET_NOVALIDATE macro to tell stackvalidate to ignore
the return instruction, or with FILE_NOVALIDATE to tell it to ignore the
entire file.

I can add some patches to fix the warnings. I'll put you on CC for the
"entry" changes.

--
Josh