lkml.org 
[lkml]   [2017]   [Oct]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [lockdep] b09be676e0 BUG: unable to handle kernel NULL pointer dereference at 000001f2
On Tue, Oct 03, 2017 at 11:28:15AM -0500, Josh Poimboeuf wrote:
> There are two bugs:
>
> 1) Somebody -- presumably lockdep -- is corrupting the stack. Need the
> lockdep people to look at that.
>
> 2) The 32-bit FP unwinder isn't handling the corrupt stack very well,
> It's blindly dereferencing untrusted data:
>
> /* 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);
> state->got_irq = true;
>
> On 32-bit, regs_size() dereferences the regs pointer before we know it
> points to a valid stack. I'll fix that, along with the other unwinder
> improvements I discussed with Linus.

Tetsuo and/or Fengguang,

Would you mind testing with this patch? It should at least prevent the
unwinder panic and should hopefully print a useful unwinder dump
instead.


diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index d145a0b1f529..a0dd4df2234d 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -44,7 +44,8 @@ static void unwind_dump(struct unwind_state *state)
state->stack_info.type, state->stack_info.next_sp,
state->stack_mask, state->graph_idx);

- for (sp = state->orig_sp; sp; sp = PTR_ALIGN(stack_info.next_sp, sizeof(long))) {
+ for (sp = PTR_ALIGN(state->orig_sp, sizeof(long)); sp;
+ sp = PTR_ALIGN(stack_info.next_sp, sizeof(long))) {
if (get_stack_info(sp, state->task, &stack_info, &visit_mask))
break;

@@ -77,6 +78,12 @@ static size_t regs_size(struct pt_regs *regs)
return sizeof(*regs);
}

+#ifdef CONFIG_X86_32
+#define KERNEL_REGS_SIZE (sizeof(struct pt_regs) - 2*sizeof(long))
+#else
+#define KERNEL_REGS_SIZE (sizeof(struct pt_regs))
+#endif
+
static bool in_entry_code(unsigned long ip)
{
char *addr = (char *)ip;
@@ -178,7 +185,7 @@ static struct pt_regs *decode_frame_pointer(unsigned long *bp)
{
unsigned long regs = (unsigned long)bp;

- if (!(regs & 0x1))
+ if ((regs & (sizeof(long)-1)) != 1)
return NULL;

return (struct pt_regs *)(regs & ~0x1);
@@ -202,7 +209,7 @@ static bool update_stack_state(struct unwind_state *state,
regs = decode_frame_pointer(next_bp);
if (regs) {
frame = (unsigned long *)regs;
- len = regs_size(regs);
+ len = KERNEL_REGS_SIZE;
state->got_irq = true;
} else {
frame = next_bp;
@@ -221,11 +228,23 @@ static bool update_stack_state(struct unwind_state *state,
&state->stack_mask))
return false;

+ /* Make sure the frame pointer is properly aligned: */
+ if ((unsigned long)frame & (sizeof(long)-1))
+ return false;
+
/* 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;

+ /*
+ * On 32-bit with user mode regs, make sure the last two regs are safe
+ * to access:
+ */
+ if (IS_ENABLED(CONFIG_X86_32) && regs && user_mode(regs) &&
+ !on_stack(info, frame, len + 2*sizeof(long)))
+ return false;
+
/* Move state to the next frame: */
if (regs) {
state->regs = regs;
\
 
 \ /
  Last update: 2017-10-03 19:34    [W:0.102 / U:0.856 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site