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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20200925200115.GP16872@zn.tnic>
Date:   Fri, 25 Sep 2020 22:01:15 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Mark Mossberg <mark.mossberg@...il.com>
Cc:     tglx@...utronix.de, mingo@...hat.com, x86@...nel.org,
        linux-kernel@...r.kernel.org, hpa@...or.com, jannh@...gle.com
Subject: Re: [PATCH] x86/dumpstack: Fix misleading instruction pointer error
 message

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@...il.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