Re: [PATCH v6 2/4] x86/stackvalidate: Compile-time stack validation

[Andy Lutomirski]

On Tue, Jul 7, 2015 at 7:54 AM, Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
> 5. A callable function may not jump to a dynamically determined address.
>    Such jumps can't be validated since the jump destination is unknown
>    at compile time.

Hmm.  Are there no switch statements that get optimized into jump
tables for which this breaks?

>
> 6. A callable function may not execute a context switching instruction.
>    The only code which needs to do context switches is kernel entry
>    code, which shouldn't be annotated to be in callable functions
>    anyway.
>
> It currently only supports x86_64.  I tried to make the code generic so
> that support for other architectures can hopefully be plugged in
> relatively easily.
>
> Currently with my Fedora config it's reporting over 1400 warnings, but
> most of them are duplicates.  The warnings affect 37 .c files and 18 .S
> files.  The C file warnings are generally due to inline assembly, which
> doesn't seem to play nice with frame pointers.

This issue might be worth bringing up on the gcc and binutils lists.
If we need better toolchain support, let's ask for it.

> +2. stackvalidate: asm_file.o: .text+0x53: return instruction outside of a callable function
> +
> +   A return instruction was detected, but stackvalidate couldn't find a
> +   way for a callable function to reach the instruction.
> +
> +   If the return instruction is inside (or reachable from) a callable
> +   function, the function needs to be annotated with the PROC/ENDPROC
> +   macros.
> +
> +   If you _really_ need a return instruction outside of a function, and
> +   are 100% sure that it won't affect stack traces, you can tell
> +   stackvalidate to ignore it.  See the "Adding exceptions" section
> +   below.

This will happen as soon as someone implements
iretless-return-to-kernel for real.  We can add an exception :)

> +
> +5. stackvalidate: asm_file.o: func()+0x6: context switch from callable function

This description threw me off for a bit.  When I read "context
switch", I thought of __switch_to.  Would something like "kernel
entry/exit instruction" instead of "context switch" be clearer?

> +
> +   This is a context switch instruction like sysenter or sysret.  Such
> +   instructions aren't allowed in a callable function, and are most
> +   likely part of kernel entry code.
> +
> +   If the instruction isn't actually in a callable function, change
> +   ENDPROC to END.
> +
> +
> +6. stackvalidate: asm_file.o: func()+0x26: jump to outside file from callable function
> +   or
> +   stackvalidate: asm_file.o: func()+0xd9: jump to dynamic address from callable function
> +
> +   These are constraints imposed by stackvalidate so that it can
> +   properly analyze all jump targets.  Dynamic jump targets and jumps to
> +   code in other object files aren't allowed.

Does this not trigger due to optimized sibling calls to different files?


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