Commit 860aaaba authored by Thomas Gleixner's avatar Thomas Gleixner Committed by Borislav Petkov
Browse files

x86/dumpstack: Do not try to access user space code of other tasks 



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: default avatarOleg Nesterov <oleg@redhat.com>
Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
Signed-off-by: default avatarBorislav Petkov <bp@suse.de>
Reviewed-by: default avatarBorislav Petkov <bp@suse.de>
Acked-by: default avatarOleg Nesterov <oleg@redhat.com>
Tested-by: default avatarBorislav Petkov <bp@suse.de>
Link: https://lkml.kernel.org/r/20201117202753.667274723@linutronix.de
parent 1a371e67

arch/x86/kernel/dumpstack.c

+19 −4
Original line number Diff line number Diff line
@@ -78,6 +78,9 @@ 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.
@@ -85,6 +88,12 @@ static int copy_code(struct pt_regs *regs, u8 *buf, unsigned long src, 
	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);

}
@@ -115,13 +124,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;

	}

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE