On Wed, Jul 22, 2020 at 07:54:15PM +0200, Thomas Gleixner wrote: > Subject: x86/dumpstack: Dump user space code correctly again > From: Thomas Gleixner <t...@linutronix.de> > Date: Wed, 22 Jul 2020 10:39:54 +0200 > > H.J. reported that post 5.7 a segfault of a user space task does not longer > dump the Code bytes when /proc/sys/debug/exception-trace is enabled. It > prints 'Code: Bad RIP value.' instead. > > This was broken by a recent change which made probe_kernel_read() reject > non-kernel addresses. > > Update show_opcodes() so it retrieves user space opcodes via > copy_from_user_nmi(). > > Fixes: 98a23609b103 ("maccess: always use strict semantics for > probe_kernel_read") > Reported-by: H.J. Lu <hjl.to...@gmail.com> > Signed-off-by: Thomas Gleixner <t...@linutronix.de>
Looks good, and also cleans up the code nicely: Reviewed-by: Christoph Hellwig <h...@lst.de> But one question below: > + /* > + * 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; > + > + return copy_from_user_nmi(buf, (void __user *)src, nbytes); copy_from_user_nmi already contains a: if (__range_not_ok(from, n, TASK_SIZE)) return n; what is the reason it checks for TASK_SIZE vs TASK_SIZE_MAX, and why do we need both checks?