linux-kernel - Re: [PATCH v5 2/4] x86/traps: Print address on #GP

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]

Date:   Tue, 3 Dec 2019 12:58:51 -0800
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Jann Horn <jannh@...gle.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Alexander Potapenko <glider@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>, kasan-dev@...glegroups.com,
        linux-kernel@...r.kernel.org,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Andy Lutomirski <luto@...nel.org>
Subject: Re: [PATCH v5 2/4] x86/traps: Print address on #GP

On Thu, Nov 28, 2019 at 12:49:14AM +0100, Jann Horn wrote:

With a few nits below,

Reviewed-and-tested-by: Sean Christopherson <sean.j.christopherson@...el.com>

> +#define GPFSTR "general protection fault"
> +
>  dotraplinkage void
>  do_general_protection(struct pt_regs *regs, long error_code)
>  {
> -	const char *desc = "general protection fault";
>  	struct task_struct *tsk;
> +	char desc[sizeof(GPFSTR) + 50 + 2*sizeof(unsigned long) + 1] = GPFSTR;

Nit, x86 maintainers prefer inverse fir tree for variable declarations.

>  
>  	RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
>  	cond_local_irq_enable(regs);
> @@ -540,6 +587,9 @@ do_general_protection(struct pt_regs *regs, long error_code)
>  
>  	tsk = current;
>  	if (!user_mode(regs)) {
> +		enum kernel_gp_hint hint = GP_NO_HINT;
> +		unsigned long gp_addr;
> +
>  		if (fixup_exception(regs, X86_TRAP_GP, error_code, 0))
>  			return;
>  
> @@ -556,8 +606,22 @@ do_general_protection(struct pt_regs *regs, long error_code)
>  			return;
>  
>  		if (notify_die(DIE_GPF, desc, regs, error_code,
> -			       X86_TRAP_GP, SIGSEGV) != NOTIFY_STOP)
> -			die(desc, regs, error_code);
> +			       X86_TRAP_GP, SIGSEGV) == NOTIFY_STOP)
> +			return;
> +
> +		if (error_code)
> +			snprintf(desc, sizeof(desc), "segment-related " GPFSTR);
> +		else
> +			hint = get_kernel_gp_address(regs, &gp_addr);
> +
> +		if (hint != GP_NO_HINT)
> +			snprintf(desc, sizeof(desc), GPFSTR " %s 0x%lx",

Nit, probably should have a comma before the hint, i.e. GPFSTR ", %s...".

    general protection fault maybe for address 0xffffc9000017cf58: 0000 [#1] SMP
    general protection fault probably for non-canonical address 0xdead000000000000: 0000 [#1] SMP

  vs. 

    general protection fault, maybe for address 0xffffc9000017cf58: 0000 [#1] SMP
    general protection fault, probably for non-canonical address 0xdead000000000000: 0000 [#1] SMP

> +				 (hint == GP_NON_CANONICAL) ?
> +				 "probably for non-canonical address" :
> +				 "maybe for address",
> +				 gp_addr);
> +
> +		die(desc, regs, error_code);
>  		return;
>  	}
>  
> -- 
> 2.24.0.432.g9d3f5f5b63-goog
>