On Thu, Jan 14, 2021 at 08:36:50AM -0600, Josh Poimboeuf wrote:
> 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/
Given that (per the discussion below) STACK_FRAME_NON_STANDARD() also
doesn't result in objtool producing anything special metadata (and such
code is still expected to be unwindable), I believe we can delete this
bullet point outright?
> > 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 think I skimmed the objtool source, too, but it was a while back. ;)
> 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 ]
Ok -- looks like that needs an update!
> 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.
Ok; I suspect on the arm64 side we'll need to think a bit harder about
what that means for us. I guess that'll influence or interact with
whatever support we need specifically in objtool.
> 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.
Assuming you mean the UNWIND_HINT_EMPTY at the start of each exception
entry point, I think I follow.
> > 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.)
Interesting; where would the UNDEFINED case be used?
> 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.
I suspect for arm64 with frame pointers we'll need a fair amount of
special casing for the entry code; I'll have a think offline.
Thanks,
Mark.
