linux-kernel - Re: Re: [PATCH 20/40] arm,kgdb: Add hook to catch an oops with debugger

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]

Message-ID: <4B4F7EC2.6030000@windriver.com>
Date:	Thu, 14 Jan 2010 14:29:54 -0600
From:	Jason Wessel <jason.wessel@...driver.com>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
CC:	linux-kernel@...r.kernel.org, kgdb-bugreport@...ts.sourceforge.net,
	mingo@...e.hu, linux-arm-kernel@...ts.infradead.org
Subject: Re: Re: [PATCH 20/40] arm,kgdb: Add hook to catch an oops with debugger

Russell King - ARM Linux wrote:
> 
> I have a similar patch which implements the hook properly - but
> with one caveat.  It needs a review to ensure that its safe to return
> from die().  Until that's established, this patch can not be merged.
> 

I completed the analysis on your patch and yes, it is safe to return
from __die() and die() the way you currently structured it, but it
doesn't work quite the same as on some other architectures.

After changing kgdb.c to register with the die notifier, I stepped
through your code with an ICE, as well as running my regression tests
which panic, oops, bad access etc...

While kernel execution does happen to continue to work, I don't know
that you really want to continue execution.

1) The kernel is marked tainted
2) bust_spinlocks() was toggled for a while

On x86 for example, the notifier is invoked prior to the
bust_spinlocks() etc... and then it can pass the exception along to
the rest of the system (which can result in something bad, but
remember the human behind the kernel debugger controls did it for some
reason or another).

I made the following addition to your patch, and then it behaved as
the other archs do with respect to passing along the result of the
exception.  Given this information, would you be willing to merge your
patch and possibly fold in the change below, or further comment?

Thanks,
Jason.

--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -273,6 +273,9 @@ void die(const char *str, struct pt_regs
 	bust_spinlocks(1);
 	ret = __die(str, err, thread, regs);

+	if (ret == NOTIFY_STOP)
+		return;
+
 	if (regs && kexec_should_crash(thread->task))
 		crash_kexec(regs);

@@ -285,8 +288,7 @@ void die(const char *str, struct pt_regs
 		panic("Fatal exception in interrupt");
 	if (panic_on_oops)
 		panic("Fatal exception");
-	if (ret != NOTIFY_STOP)
-		do_exit(SIGSEGV);
+	do_exit(SIGSEGV);
 }

 void arm_notify_die(const char *str, struct pt_regs *regs,
--
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/