linux-kernel - Re: [PATCH] Documentation: livepatch: document reliable stacktrace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 Jan 2021 08:36:50 -0600
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Mark Rutland <mark.rutland@....com>
Cc:     Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org,
        Jiri Kosina <jikos@...nel.org>,
        Joe Lawrence <joe.lawrence@...hat.com>,
        Jonathan Corbet <corbet@....net>,
        Miroslav Benes <mbenes@...e.cz>,
        Petr Mladek <pmladek@...e.com>, linux-doc@...r.kernel.org,
        live-patching@...r.kernel.org
Subject: Re: [PATCH] Documentation: livepatch: document reliable stacktrace

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@....com>
> > > +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