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

[Josh Poimboeuf <jpoimboe@redhat.com>]

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