linux-kernel - Re: [PATCH] x86, dumpstack: make die and die

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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 21 Oct 2008 10:59:54 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	Alexander van Heukelum <heukelum@...lshack.com>
Cc:	Ingo Molnar <mingo@...e.hu>, kexec@...ts.infradead.org,
	linux-kernel@...r.kernel.org, vgoyal@...hat.com, hbabu@...ibm.com,
	ebiederm@...ssion.com, tglx@...utronix.de, mingo@...hat.com,
	hpa@...or.com, akpm@...ux-foundation.org,
	Alexander van Heukelum <heukelum@...tmail.fm>
Subject: Re: [PATCH] x86, dumpstack: make die and die_nmi equal

On Mon, Oct 20, 2008 at 05:11:06PM +0200, Alexander van Heukelum wrote:
> Use the x86_64 version of die() and die_nmi() on i386 too.
> Changes to the original x86_64-version have no influence
> on the generated code:
> 
>  - whitespace, comments
>  - use user_mode_vm() instead of user_mode()
> 
Hey, smore more comments inline

> Signed-off-by: Alexander van Heukelum <heukelum@...tmail.fm>
> ---
>  arch/x86/kernel/dumpstack_32.c |   40 ++++++++++++++--------------------------
>  arch/x86/kernel/dumpstack_64.c |    9 +++++++--
>  2 files changed, 21 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
> index e45952b..6f00938 100644
> --- a/arch/x86/kernel/dumpstack_32.c
> +++ b/arch/x86/kernel/dumpstack_32.c
> @@ -377,52 +377,40 @@ void die(const char *str, struct pt_regs *regs, long err)
>  {
>  	unsigned long flags = oops_begin();
>  
> -	if (die_nest_count < 3) {
> +	if (!user_mode_vm(regs))
>  		report_bug(regs->ip, regs);
>  
> -		if (__die(str, regs, err))
> -			regs = NULL;
> -	} else {
> -		printk(KERN_EMERG "Recursive die() failure, output suppressed\n");
> -	}
> +	if (__die(str, regs, err))
> +		regs = NULL;
>  
>  	oops_end(flags, regs, SIGSEGV);
>  }
>  
> -static DEFINE_SPINLOCK(nmi_print_lock);
> -
>  void notrace __kprobes
>  die_nmi(char *str, struct pt_regs *regs, int do_panic)
>  {
> +	unsigned long flags;
> +
>  	if (notify_die(DIE_NMIWATCHDOG, str, regs, 0, 2, SIGINT) == NOTIFY_STOP)
>  		return;
>  
> -	spin_lock(&nmi_print_lock);
> +	flags = oops_begin();
>  	/*
>  	* We are in trouble anyway, lets at least try
> -	* to get a message out:
> +	* to get a message out.
>  	*/
> -	bust_spinlocks(1);
>  	printk(KERN_EMERG "%s", str);
>  	printk(" on CPU%d, ip %08lx, registers:\n",
>  		smp_processor_id(), regs->ip);
>  	show_registers(regs);
> -	if (do_panic)
> -		panic("Non maskable interrupt");
> -	console_silent();
> -	spin_unlock(&nmi_print_lock);
> -	bust_spinlocks(0);
> -
> -	/*
> -	 * If we are in kernel we are probably nested up pretty bad
> -	 * and might aswell get out now while we still can:
> -	 */
> -	if (!user_mode_vm(regs)) {
> -		current->thread.trap_no = 2;
> +	if (kexec_should_crash(current))
>  		crash_kexec(regs);
As I mentioned in your other patch, this crash_kexec can go I think if you're
going to do it in oops_end (as long as you correct the deadlock condition there

And as before, if you could rediff so that we didn't reintroduce the deadlock
that we just fixed, that would be much appreciated.

Beyond that, I think this looks good.  Fix that up and It'll have my ack.

Best
Neil

--
/****************************************************
 * Neil Horman <nhorman@...driver.com>
 * Software Engineer, Red Hat
 ****************************************************/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/