From 5ed8d8bb38c5dcd78de540182cedb0fb19399aab Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 12 Apr 2017 13:47:10 -0500 Subject: [PATCH] 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 Acked-by: Thomas Gleixner Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Daniel Borkmann Cc: Dave Jones Cc: Denys Vlasenko Cc: H. Peter Anvin Cc: Linus Torvalds Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/a2ee4801113f6d2300d58f08f6b69f85edf4eb43.1492020577.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar --- arch/x86/kernel/unwind_frame.c | 119 +++++++++++++++------------------ 1 file changed, 55 insertions(+), 64 deletions(-) diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c index 08339262b666..9098ef1da8f8 100644 --- 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 -- 2.25.1