Re: [PATCH] x86/dumpstack: Fix misleading instruction pointer error message

[Borislav Petkov <bp@alien8.de>]

On Fri, Sep 25, 2020 at 07:31:51PM +0000, Mark Mossberg wrote:
> Unconditionally printing "Bad RIP value" if copy_code() fails can be
> misleading for userspace pointers, since copy_code() can fail if the
> instruction pointer is valid, but the code is paged out.  This is
> because copy_code() calls copy_from_user_nmi() for userspace pointers,
> which disables page fault handling.
> 
> This is reproducible in OOM situations, where it's plausible that the
> code may be reclaimed in the time between entry into the kernel and when
> this message is printed. This leaves a misleading log in dmesg that
> suggests instruction pointer corruption has occurred, which may alarm
> users.
> 
> This patch changes the message printed for userspace pointers to more
> accurately reflect the possible reasons why the code cannot be dumped.
> 
> Signed-off-by: Mark Mossberg <mark.mossberg@gmail.com>
> ---
>  arch/x86/kernel/dumpstack.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> index 48ce44576947..37dbf16c7456 100644
> --- a/arch/x86/kernel/dumpstack.c
> +++ b/arch/x86/kernel/dumpstack.c
> @@ -115,7 +115,10 @@ void show_opcodes(struct pt_regs *regs, const char *loglvl)
>  	unsigned long prologue = regs->ip - PROLOGUE_SIZE;
>  
>  	if (copy_code(regs, opcodes, prologue, sizeof(opcodes))) {
> -		printk("%sCode: Bad RIP value.\n", loglvl);

I'd prefer if this thing said exactly what the problem is:

		printk("%sCode: Unable to access opcode bytes at rIP 0x%lx... "

or so.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette