Re: [RFC PATCH 0/3] arm64: Implement reliable stack trace

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2355 bytes --]

On Thu, Oct 15, 2020 at 04:29:31PM -0500, Josh Poimboeuf wrote:
> I can't see the original patch.  Can the original poster (Mark B?) add
> me to Cc on the next version?

https://lore.kernel.org/linux-arm-kernel/20201012172605.10715-1-broonie@kernel.org/

> It's also good practice to add lkml as well.  That way, those of us not
> copied can at least find the patch in the archives.

> live-patching@vger.kernel.org would also be a good idea for this one.

Sorry about that.  I don't know if it's worth including a K: pattern for
arch_stack_walk_reliable() in the livepatch entry in MAINTAINERS?

> If by cargo culting, you mean reverse engineering the requirements due
> to lack of documentation, that's fair.

Yes, exactly - just copying the existing implementations and hoping that
it's sensible/relevant and covers everything that's needed.  It's not
entirely clear what a reliable stacktrace is expected to do that a
normal stacktrace doesn't do beyond returning an error code.

> > The searching for a defined thread entry point for example isn't
> > entirely visible in the implementations.

> For now I'll speak only of x86, because I don't quite remember how
> powerpc does it.

> For thread entry points, aka the "end" of the stack:

> - For ORC, the end of the stack is either pt_regs, or -- when unwinding
>   from kthreads, idle tasks, or irqs/exceptions in entry code --
>   UNWIND_HINT_EMPTY (found by the unwinder's check for orc->end.

>   [ Admittedly the implementation needs to be cleaned up a bit.  EMPTY
>     is too broad and needs to be split into UNDEFINED and ENTRY. ]

> - For frame pointers, by convention, the end of the stack for all tasks
>   is a defined stack offset: end of stack page - sizeof(pt_regs).

> And yes, all that needs to be documented.

Ah, I'd have interpreted "defined thread entry point" as meaning
expecting to find specific functions appering at the end of the stack
rather than meaning positively identifying the end of the stack - for
arm64 we use a NULL frame pointer to indicate this in all situations.
In that case that's one bit that is already clear.

From the list Miroslav posted the bits I wouldn't have inferred were:

 - Detecting preemption/page faults
 - Preventing recursive loops
 - Verifying that return addresses are text addresses

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]