linux-kernel - Re: [PATCH] Fix print out of function which called WARN

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: <alpine.LFD.2.01.0905161342150.3301@localhost.localdomain>
Date:	Sat, 16 May 2009 13:47:36 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Andi Kleen <ak@...ux.intel.com>
cc:	Ian Campbell <ian.campbell@...rix.com>,
	Jakub Jelinek <jakub@...hat.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Jesper Nilsson <jesper.nilsson@...s.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	Hugh Dickins <hugh@...itas.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] Fix print out of function which called WARN_ON()



On Fri, 15 May 2009, Andi Kleen wrote:

> Linus Torvalds wrote:
> > [ Jakub - I added you to the participants list because it really looks
> > like this patch (without 'noinline') triggers a gcc bug.
> 
> This whole thing seems to be much more problematic than I originally thought
> :-/

No, sorry, it was my mistake. The code generated was indeed wrong, but it 
was because the source code was wrong (me calling the _fmt() function 
instead of the _common() function).

What's wrong with me that I can spot subtle errors in assembly language, 
but not the obvious ones in the source code?

Anyway, the proper patch is appended, and Jakub, you can ignore these 
things. gcc is fine.

This patch not only avoids the warnings and gets the right caller 
information, it cleans up the code too:

 - it uses '%pS' instead of of sprint_symbol

 - it avoids stupidly wasting stack space for varargs information in the 
   warn_slowpath_null case.

 - the code actually looks more readable too.

I'll commit it.

		Linus

---
 kernel/panic.c |   35 ++++++++++++++++++++---------------
 1 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index 874ecf1..984b3ec 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -340,39 +340,44 @@ void oops_exit(void)
 }
 
 #ifdef WANT_WARN_ON_SLOWPATH
-void warn_slowpath_fmt(const char *file, int line, const char *fmt, ...)
-{
+struct slowpath_args {
+	const char *fmt;
 	va_list args;
-	char function[KSYM_SYMBOL_LEN];
-	unsigned long caller = (unsigned long)__builtin_return_address(0);
-	const char *board;
+};
 
-	sprint_symbol(function, caller);
+static void warn_slowpath_common(const char *file, int line, void *caller, struct slowpath_args *args)
+{
+	const char *board;
 
 	printk(KERN_WARNING "------------[ cut here ]------------\n");
-	printk(KERN_WARNING "WARNING: at %s:%d %s()\n", file,
-		line, function);
+	printk(KERN_WARNING "WARNING: at %s:%d %pS()\n", file, line, caller);
 	board = dmi_get_system_info(DMI_PRODUCT_NAME);
 	if (board)
 		printk(KERN_WARNING "Hardware name: %s\n", board);
 
-	if (*fmt) {
-		va_start(args, fmt);
-		vprintk(fmt, args);
-		va_end(args);
-	}
+	if (args)
+		vprintk(args->fmt, args->args);
 
 	print_modules();
 	dump_stack();
 	print_oops_end_marker();
 	add_taint(TAINT_WARN);
 }
+
+void warn_slowpath_fmt(const char *file, int line, const char *fmt, ...)
+{
+	struct slowpath_args args;
+
+	args.fmt = fmt;
+	va_start(args.args, fmt);
+	warn_slowpath_common(file, line, __builtin_return_address(0), &args);
+	va_end(args.args);
+}
 EXPORT_SYMBOL(warn_slowpath_fmt);
 
 void warn_slowpath_null(const char *file, int line)
 {
-	static const char *empty = "";
-	warn_slowpath_fmt(file, line, empty);
+	warn_slowpath_common(file, line, __builtin_return_address(0), NULL);
 }
 EXPORT_SYMBOL(warn_slowpath_null);
 #endif
--
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/