Re: [PATCH v5 02/10] x86: Compile-time asm code validation

[Andy Lutomirski]

On Wed, Jun 10, 2015 at 10:53 AM, Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
> On Wed, Jun 10, 2015 at 10:21:36AM -0700, Andy Lutomirski wrote:
>> On Jun 10, 2015 5:07 AM, "Josh Poimboeuf" <jpoimboe@xxxxxxxxxx> wrote:
>> >
>> > Add a new CONFIG_ASM_VALIDATION option which adds an asmvalidate host
>> > tool which runs on every compiled .S file.  Its goal is to enforce sane
>> > rules on all asm code, so that stack debug metadata (frame/back chain
>> > pointers and/or DWARF CFI metadata) can be made reliable.
>> >
>> > It enforces the following rules:
>> >
>> > 1. Each callable function must be annotated with the ELF STT_FUNC type.
>> >    This is typically done using the ENTRY/ENDPROC macros.  If
>> >    asmvalidate finds a return instruction outside of a function, it
>> >    flags an error, since that usually indicates callable code which
>> >    should be annotated accordingly.
>> >
>> > 2. Each callable function must never leave its own bounds (i.e. with a
>> >    jump to outside the function) except when returning.
>>
>> Won't that break with sibling/tail calls?
>
> Yes, asmvalidate will flag a warning for tail calls.
>
>> GCC can generate those, and the ia32_ptregs_common label is an example
>> of such a thing.
>>
>> I'd rather have the script understand tail calls and possibly require
>> that ia32_ptregs_common have a dummy frame pointer save in front
>> before the label if needed.
>
> Why do you prefer tail calls there?  See patch 3 for how I handled that
> for ia32_ptregs_common (I duplicated the code with macros).
>
> I think adding support for tail calls in the tooling would be tricky.
> So I'm just trying to figure out if there's a good reason to keep them.

To save code size by deduplicating common tails.  The code currently
does that, and it would be nice to avoid bloating the code to keep the
validator happy.

I imagine that an automatic CFI annotation adder would walk through
functions one instruction at a time and keep track of the frame state.
If so, then it could verify that common jump targets had identical
state and continue walking through them and annotating.  I think this
would get this case right, and it might be necessary anyway to handle
jumps within functions.

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