x86/unwind: Move common code into update_stack_state()

The __unwind_start() and unwind_next_frame() functions have some
duplicated functionality.  They both call decode_frame_pointer() and set
state->regs and state->bp accordingly.  Move that functionality to a
common place in update_stack_state().

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Dave Jones <davej@codemonkey.org.uk>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/a2ee4801113f6d2300d58f08f6b69f85edf4eb43.1492020577.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>

diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index 08339262b666e56f2623406a10c42f3184c83e29..9098ef1da8f8eebeb2a59f47afc3ebeb4cd5778e 100644 (file)
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -135,26 +135,59 @@ static struct pt_regs *decode_frame_pointer(unsigned long *bp)
        return (struct pt_regs *)(regs & ~0x1);
 }
 
-static bool update_stack_state(struct unwind_state *state, void *addr,
-                              size_t len)
+static bool update_stack_state(struct unwind_state *state,
+                              unsigned long *next_bp)
 {
        struct stack_info *info = &state->stack_info;
-       enum stack_type orig_type = info->type;
+       enum stack_type prev_type = info->type;
+       struct pt_regs *regs;
+       unsigned long *frame, *prev_frame_end;
+       size_t len;
+
+       if (state->regs)
+               prev_frame_end = (void *)state->regs + regs_size(state->regs);
+       else
+               prev_frame_end = (void *)state->bp + FRAME_HEADER_SIZE;
+
+       /* Is the next frame pointer an encoded pointer to pt_regs? */
+       regs = decode_frame_pointer(next_bp);
+       if (regs) {
+               frame = (unsigned long *)regs;
+               len = regs_size(regs);
+       } else {
+               frame = next_bp;
+               len = FRAME_HEADER_SIZE;
+       }
 
        /*
-        * If addr isn't on the current stack, switch to the next one.
+        * If the next bp isn't on the current stack, switch to the next one.
         *
         * We may have to traverse multiple stacks to deal with the possibility
-        * that 'info->next_sp' could point to an empty stack and 'addr' could
-        * be on a subsequent stack.
+        * that info->next_sp could point to an empty stack and the next bp
+        * could be on a subsequent stack.
         */
-       while (!on_stack(info, addr, len))
+       while (!on_stack(info, frame, len))
                if (get_stack_info(info->next_sp, state->task, info,
                                   &state->stack_mask))
                        return false;
 
-       if (!state->orig_sp || info->type != orig_type)
-               state->orig_sp = addr;
+       /* Make sure it only unwinds up and doesn't overlap the prev frame: */
+       if (state->orig_sp && state->stack_info.type == prev_type &&
+           frame < prev_frame_end)
+               return false;
+
+       /* Move state to the next frame: */
+       if (regs) {
+               state->regs = regs;
+               state->bp = NULL;
+       } else {
+               state->bp = next_bp;
+               state->regs = NULL;
+       }
+
+       /* Save the original stack pointer for unwind_dump(): */
+       if (!state->orig_sp || info->type != prev_type)
+               state->orig_sp = frame;
 
        return true;
 }
@@ -162,14 +195,12 @@ static bool update_stack_state(struct unwind_state *state, void *addr,
 bool unwind_next_frame(struct unwind_state *state)
 {
        struct pt_regs *regs;
-       unsigned long *next_bp, *next_frame;
-       size_t next_len;
-       enum stack_type prev_type = state->stack_info.type;
+       unsigned long *next_bp;
 
        if (unwind_done(state))
                return false;
 
-       /* have we reached the end? */
+       /* Have we reached the end? */
        if (state->regs && user_mode(state->regs))
                goto the_end;
 
@@ -200,24 +231,14 @@ bool unwind_next_frame(struct unwind_state *state)
                return true;
        }
 
-       /* get the next frame pointer */
+       /* Get the next frame pointer: */
        if (state->regs)
                next_bp = (unsigned long *)state->regs->bp;
        else
-               next_bp = (unsigned long *)READ_ONCE_TASK_STACK(state->task,*state->bp);
+               next_bp = (unsigned long *)READ_ONCE_TASK_STACK(state->task, *state->bp);
 
-       /* is the next frame pointer an encoded pointer to pt_regs? */
-       regs = decode_frame_pointer(next_bp);
-       if (regs) {
-               next_frame = (unsigned long *)regs;
-               next_len = sizeof(*regs);
-       } else {
-               next_frame = next_bp;
-               next_len = FRAME_HEADER_SIZE;
-       }
-
-       /* make sure the next frame's data is accessible */
-       if (!update_stack_state(state, next_frame, next_len)) {
+       /* Move to the next frame if it's safe: */
+       if (!update_stack_state(state, next_bp)) {
                /*
                 * Don't warn on bad regs->bp.  An interrupt in entry code
                 * might cause a false positive warning.
@@ -228,24 +249,6 @@ bool unwind_next_frame(struct unwind_state *state)
                goto bad_address;
        }
 
-       /* Make sure it only unwinds up and doesn't overlap the last frame: */
-       if (state->stack_info.type == prev_type) {
-               if (state->regs && (void *)next_frame < (void *)state->regs + regs_size(state->regs))
-                       goto bad_address;
-
-               if (state->bp && (void *)next_frame < (void *)state->bp + FRAME_HEADER_SIZE)
-                       goto bad_address;
-       }
-
-       /* move to the next frame */
-       if (regs) {
-               state->regs = regs;
-               state->bp = NULL;
-       } else {
-               state->bp = next_bp;
-               state->regs = NULL;
-       }
-
        return true;
 
 bad_address:
@@ -263,13 +266,13 @@ bad_address:
                printk_deferred_once(KERN_WARNING
                        "WARNING: kernel stack regs at %p in %s:%d has bad 'bp' value %p\n",
                        state->regs, state->task->comm,
-                       state->task->pid, next_frame);
+                       state->task->pid, next_bp);
                unwind_dump(state, (unsigned long *)state->regs);
        } else {
                printk_deferred_once(KERN_WARNING
                        "WARNING: kernel stack frame pointer at %p in %s:%d has bad value %p\n",
                        state->bp, state->task->comm,
-                       state->task->pid, next_frame);
+                       state->task->pid, next_bp);
                unwind_dump(state, state->bp);
        }
 the_end:
@@ -281,35 +284,23 @@ EXPORT_SYMBOL_GPL(unwind_next_frame);
 void __unwind_start(struct unwind_state *state, struct task_struct *task,
                    struct pt_regs *regs, unsigned long *first_frame)
 {
-       unsigned long *bp, *frame;
-       size_t len;
+       unsigned long *bp;
 
        memset(state, 0, sizeof(*state));
        state->task = task;
 
-       /* don't even attempt to start from user mode regs */
+       /* Don't even attempt to start from user mode regs: */
        if (regs && user_mode(regs)) {
                state->stack_info.type = STACK_TYPE_UNKNOWN;
                return;
        }
 
-       /* set up the starting stack frame */
        bp = get_frame_pointer(task, regs);
-       regs = decode_frame_pointer(bp);
-       if (regs) {
-               state->regs = regs;
-               frame = (unsigned long *)regs;
-               len = sizeof(*regs);
-       } else {
-               state->bp = bp;
-               frame = bp;
-               len = FRAME_HEADER_SIZE;
-       }
 
-       /* initialize stack info and make sure the frame data is accessible */
-       get_stack_info(frame, state->task, &state->stack_info,
+       /* Initialize stack info and make sure the frame data is accessible: */
+       get_stack_info(bp, state->task, &state->stack_info,
                       &state->stack_mask);
-       update_stack_state(state, frame, len);
+       update_stack_state(state, bp);
 
        /*
         * The caller can provide the address of the first frame directly