Re: [PATCH] Documentation: livepatch: document reliable stacktrace

[Josh Poimboeuf]

On Thu, Jan 14, 2021 at 11:54:18AM +0000, Mark Rutland wrote:
> On Wed, Jan 13, 2021 at 01:33:13PM -0600, Josh Poimboeuf wrote:
> > On Wed, Jan 13, 2021 at 04:57:43PM +0000, Mark Brown wrote:
> > > From: Mark Rutland <mark.rutland@xxxxxxx>
> > > +There are several ways an architecture may identify kernel code which is deemed
> > > +unreliable to unwind from, e.g.
> > > +
> > > +* Using metadata created by objtool, with such code annotated with
> > > +  SYM_CODE_{START,END} or STACKFRAME_NON_STANDARD().
> > 
> > I'm not sure why SYM_CODE_{START,END} is mentioned here, but it doesn't
> > necessarily mean the code is unreliable, and objtool doesn't treat it as
> > such.  Its mention can probably be removed unless there was some other
> > point I'm missing.
> > 
> > Also, s/STACKFRAME/STACK_FRAME/
> 
> When I wrote this, I was under the impression that (for x86) code marked
> as SYM_CODE_{START,END} wouldn't be considered as a function by objtool.
> Specifically SYM_FUNC_END() marks the function with SYM_T_FUNC whereas
> SYM_CODE_END() marks it with SYM_T_NONE, and IIRC I thought that objtool
> only generated ORC for SYM_T_FUNC functions, and hence anything else
> would be considered not unwindable due to the absence of ORC.
> 
> Just to check, is that understanding for x86 correct, or did I get that
> wrong?

Doh, I suppose you read the documentation ;-)

I realize your understanding is pretty much consistent with
tools/objtool/Documentation/stack-validation.txt:

2. Conversely, each section of code which is *not* callable should *not*
   be annotated as an ELF function.  The ENDPROC macro shouldn't be used
   in this case.

   This rule is needed so that objtool can ignore non-callable code.
   Such code doesn't have to follow any of the other rules.

But this statement is no longer true:

  **This rule is needed so that objtool can ignore non-callable code.**

[ and it looks like the ENDPROC reference is also out of date ]

Since that document was written, around the time ORC was written we
realized objtool shouldn't ignore SYM_CODE after all.  That way we can
get full coverage for ORC (including interrupts/exceptions), as well as
some of the other validations like retpoline, uaccess, noinstr, etc.

Though it's still true that SYM_CODE doesn't have to follow the
function-specific rules, e.g. frame pointers.

So now objtool requires that it be able to traverse and understand *all*
code, otherwise it will spit out "unreachable instruction" warnings.
But since SYM_CODE isn't a normal callable function, objtool doesn't
know to interpret it directly.  Therefore all SYM_CODE must be reachable
by objtool in some other way:

- either indirectly, via a jump from a SYM_FUNC; or

- via an UNWIND_HINT

(And that's true for both ORC and frame pointers.)

If you look closely at arch/x86/entry/entry_64.S you should be able to
see that's the case.

> If that's right, it might be worth splitting this into two points, e.g.
> 
> | * Using metadata created by objtool, with such code annotated with
> |   STACKFRAME_NON_STANDARD().
> |
> |
> | * Using ELF symbol attributes, with such code annotated with
> |   SYM_CODE_{START,END}, and not having a function type.
> 
> If that's wrong, I suspect there are latent issues here?

For ORC, UNWIND_HINT_EMPTY is used to annotate that some code is
non-unwindable.  (Note I have plans to split that into UNWIND_HINT_ENTRY
and UNWIND_HINT_UNDEFINED.)

For frame pointers, the hints aren't used, other than by objtool to
follow the code flow as described above.  But objtool doesn't produce
any metadata for the FP unwinder.  Instead the FP unwinder makes such
determinations about unwindability at runtime.

-- 
Josh