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

Josh Poimboeuf
2015-06-10T14:20:30Z

On Wed, Jun 10, 2015 at 03:42:41PM +0200, Pavel Machek wrote:
> On Wed 2015-06-10 07:06:08, Josh Poimboeuf wrote:
> > The previous version of this patch set was named "Compile-time stack
> > frame pointer validation". I changed the subject from "frame pointer
> > validation" to "asm code validation" because the focus of the patch set
> > has changed to be less frame pointer-focused and more asm-focused. I
> > also renamed the tool to asmvalidate (it was previously called
> > stackvalidate) and basically rewrote most of the code.
> >
> > The goal of asm validation is to enforce sane rules on asm code: all
> > callable asm functions must be self-contained and properly annotated.
> >
> > Some of the benefits are:
> >
> > - Frame pointers are more reliable.
> >
> > - DWARF CFI metadata can be autogenerated (coming soon).
> >
> > - The asm code becomes less like spaghetti, more like C, and easier to
> > comprehend.
> >
> >
> > The asmvalidate tool runs on every compiled .S file, and enforces the
> > following rules:
> >
> > 1. Each callable function must be annotated with the ELF STT_FUNC type.
> > This is typically done using the existing 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.
> >
> > 3. Each callable non-leaf function must have frame pointer logic (if
> > required by CONFIG_FRAME_POINTER or the architecture's back chain
> > rules). This should by done by the FP_SAVE/FP_RESTORE macros.
> >
> >
> > It currently only supports x86_64, but the code is generic and designed
> > for it to be easy to plug in support for other architectures.
> >
> > There are still a lot of outstanding warnings (which I'll paste as a
> > reply to this email). Once those are all cleaned up, we can change the
> > warnings to build errors and change the default to
> > CONFIG_ASM_VALIDATION=y so the asm code stays clean.
>
> You have interesting definition of "clean".

"clean":

- reliably honors CONFIG_FRAME_POINTER
- reliably creates/generates DWARF CFI metadata
- doesn't break stack walking
- code is more readable

> The reason we sometimes have to use assembly is that it is impossible
> to write corresponding code in C, or that performance would be bad.

Agreed, but I don't see how this patch set prevents those things.

> So... fixing these may have some sense, but I doubt enforcing "you
> can't write real assembly" is a good idea.

You can certainly still write real assembly. This just creates a few
constraints. I really don't think they are very limiting.

--
Josh