Re: [PATCH 3/3] arm64: stacktrace: better handle corrupted stacks

[Mark Rutland <mark.rutland@arm.com>]

On Mon, Jul 01, 2019 at 11:48:21AM +0100, Dave Martin wrote:
> On Fri, Jun 28, 2019 at 02:02:55PM +0100, Mark Rutland wrote:
> > The problem is that we unwind one frame, then check the fp of that
> > frame.
> > 
> > Say we have three stack frames, A->B->C, where A and B are on the IRQ
> > stack, and C is on the task stack.
> > 
> > At entry to unwind_frame(), frame describes A, and frame->fp points at
> > B. Thus frame->stack_current == info.type, and changed_stack == false.
> > 
> > Then we sample B:
> > 
> > 	frame->fp = READ_ONCE(fp); // points at C on the task tasck
> > 
> > Then we do:
> > 
> > 	if (!changed_stack && frame->fp <= fp)
> > 
> > ... where changed_stack describes the A->B transition (false), but
> > frame->fp <= fp is the B->C transition, and B and C are on different
> > stacks!
> 
> OK, if I've understood that right, it looks like frame->stack_current
> describes where the contents of frame were fetched from, not the frame
> at frame->fp.
> 
> This is actually pretty confusing: the frame stack_current refers to is
> already history: we have no pointer to it any more anyway.
> 
> I wonder whether this can be refactored so that stack_current doesn't
> lag behind: say, call it fp_stack (the stack frame->fp points at).
> 
> That's just one option though.  I'll take a look at the repost.

For v2 I added prev_fp, and renamed stack_current to prev_type.

We need the prev_fp so that we can do the intra-stack monotonicity
check. We can derive prev_type from prev_fp, but it was simpler to store
prev_type than to recalculate it.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel