Skip to content

Commit

Permalink
x86/dumpstack: Do not try to access user space code of other tasks
Browse files Browse the repository at this point in the history
[ Upstream commit 860aaab ]

sysrq-t ends up invoking show_opcodes() for each task which tries to access
the user space code of other processes, which is obviously bogus.

It either manages to dump where the foreign task's regs->ip points to in a
valid mapping of the current task or triggers a pagefault and prints "Code:
Bad RIP value.". Both is just wrong.

Add a safeguard in copy_code() and check whether the @regs pointer matches
currents pt_regs. If not, do not even try to access it.

While at it, add commentary why using copy_from_user_nmi() is safe in
copy_code() even if the function name suggests otherwise.

Reported-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Borislav Petkov <bp@suse.de>
Acked-by: Oleg Nesterov <oleg@redhat.com>
Tested-by: Borislav Petkov <bp@suse.de>
Link: https://lkml.kernel.org/r/20201117202753.667274723@linutronix.de
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
Thomas Gleixner authored and gregkh committed Dec 2, 2020
1 parent 4381c76 commit f6cbd74
Showing 1 changed file with 19 additions and 4 deletions.
23 changes: 19 additions & 4 deletions arch/x86/kernel/dumpstack.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,22 @@ static int copy_code(struct pt_regs *regs, u8 *buf, unsigned long src,
if (!user_mode(regs))
return copy_from_kernel_nofault(buf, (u8 *)src, nbytes);

/* The user space code from other tasks cannot be accessed. */
if (regs != task_pt_regs(current))
return -EPERM;
/*
* Make sure userspace isn't trying to trick us into dumping kernel
* memory by pointing the userspace instruction pointer at it.
*/
if (__chk_range_not_ok(src, nbytes, TASK_SIZE_MAX))
return -EINVAL;

/*
* Even if named copy_from_user_nmi() this can be invoked from
* other contexts and will not try to resolve a pagefault, which is
* the correct thing to do here as this code can be called from any
* context.
*/
return copy_from_user_nmi(buf, (void __user *)src, nbytes);
}

Expand Down Expand Up @@ -114,13 +123,19 @@ void show_opcodes(struct pt_regs *regs, const char *loglvl)
u8 opcodes[OPCODE_BUFSIZE];
unsigned long prologue = regs->ip - PROLOGUE_SIZE;

if (copy_code(regs, opcodes, prologue, sizeof(opcodes))) {
printk("%sCode: Unable to access opcode bytes at RIP 0x%lx.\n",
loglvl, prologue);
} else {
switch (copy_code(regs, opcodes, prologue, sizeof(opcodes))) {
case 0:
printk("%sCode: %" __stringify(PROLOGUE_SIZE) "ph <%02x> %"
__stringify(EPILOGUE_SIZE) "ph\n", loglvl, opcodes,
opcodes[PROLOGUE_SIZE], opcodes + PROLOGUE_SIZE + 1);
break;
case -EPERM:
/* No access to the user space stack of other tasks. Ignore. */
break;
default:
printk("%sCode: Unable to access opcode bytes at RIP 0x%lx.\n",
loglvl, prologue);
break;
}
}

Expand Down

0 comments on commit f6cbd74

Please sign in to comment.