Re: [PATCH 17/19] x86/entry/dumpstack: encode pt_regs pointer in frame pointer

From: Andy Lutomirski
Date: Thu Jul 21 2016 - 18:28:13 EST


On Thu, Jul 21, 2016 at 2:21 PM, Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
> With frame pointers, when a task is interrupted, its stack is no longer
> completely reliable because the function could have been interrupted
> before it had a chance to save the previous frame pointer on the stack.
> So the caller of the interrupted function could get skipped by a stack
> trace.
>
> This is problematic for live patching, which needs to know whether a
> stack trace of a sleeping task can be relied upon. There's currently no
> way to detect if a sleeping task was interrupted by a page fault
> exception or preemption before it went to sleep.
>
> Another issue is that when dumping the stack of an interrupted task, the
> unwinder has no way of knowing where the saved pt_regs registers are, so
> it can't print them.
>
> This solves those issues by encoding the pt_regs pointer in the frame
> pointer on entry from an interrupt or an exception. The frame pointer
> unwinder is also updated to decode it.
>
> Suggested-by: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
> Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>

>
> +/*
> + * This determines if the frame pointer actually contains an encoded pointer to
> + * pt_regs on the stack. See ENCODE_FRAME_POINTER.
> + */
> +static struct pt_regs *decode_frame_pointer(struct unwind_state *state,
> + unsigned long *bp)
> +{
> + struct pt_regs *regs = (struct pt_regs *)bp;
> + unsigned long *task_begin = task_stack_page(state->task);
> + unsigned long *task_end = task_stack_page(state->task) + THREAD_SIZE;
> +
> + if (test_and_set_bit(BITS_PER_LONG - 1, (unsigned long *)&regs))
> + return NULL;

test_and_set_bit is a fairly heavyweight atomic operation. It's
probably better to use plain C bitwise ops.