Date:	Tue, 18 Dec 2007 15:37:09 -0800
From:	Arjan van de Ven <arjan@...ux.intel.com>
To:	Matt Mackall <mpm@...enic.com>
CC:	Ingo Molnar <mingo@...e.hu>, linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	protasnb@...il.com, tytso@...nk.org
Subject: Re: Top kernel oopses/warnings this week

Matt Mackall wrote:
> 
> Blech. Invoking the random pool machinery at oops time is moderately
> safe, but not very shiny. Going through all the sprintf ugliness to
> format it to an irrelevant UUID standard is not very shiny either. At
> least refactor it so it's not duplicating code.
> 
> And I'd much rather the static variable lived with its user, as
> random.c is already too miscellaneous:

ok so something like this?


From: Arjan van de Ven <arjan@...ux.intel.com>
Subject: [patch] Print end-of-oops marker with UUID

Right now, it's nearly impossible for parsers to detect the end-of-oops
condition; for example this is a problem for www.kerneloops.org.
In addition, it's not currently possible to detect whether or not
2 oopses that look alike are actually the same oops reported twice,
or truely 2 unique oopses.

This patch factors out the "sprintf a UUID into a string" code from
random.c into a separate function (using snprintf as suggested by
Randy). So far I left the %02x in place instead of using Linus'
"improvement"; if someone really hates the %02x's he/she can do that
later.

It also reduces the stack footprint of proc_do_uuid(); it
was using 64 bytes for the string where 37 is sufficient.
With these random.c changes, the oops_exit() function can print an
end-of-oops marker from the oops_exit() function.

Normally, the UUID used for oopses is calculated as late_initcall
(in the hope that at that time there is enough entropy to get a
unique enough UUID); however for early oopses the oops_exit() function
needs to generate the UUID on the fly.

Signed-off-by: Arjan van de Ven <arjan@...ux.intel.com>
CC: Matt
CC: Ted
CC: Randy

--- linux-2.6.24-rc5/drivers/char/random.c.org	2007-12-18 11:37:22.000000000 -0800
+++ linux-2.6.24-rc5/drivers/char/random.c	2007-12-18 12:20:48.000000000 -0800
@@ -1176,8 +1175,34 @@ static int max_read_thresh = INPUT_POOL_
  static int max_write_thresh = INPUT_POOL_WORDS * 32;
  static char sysctl_bootid[16];

+
+/**
+ * snprintf_uuid - Convert a 16 byte UUID into string format
+ * @string: buffer to store the UUID into
+ * @len:    size of @string
+ * @uuid:   the UUID to convert
+ *
+ * This function converts a 16 byte binary UUID into canonical
+ * ASCII form. This ASCII form needs 37 bytes of storage space,
+ * allocated and provided by the caller.
+ *
+ * Returns: pointer to @string
+ *
+ * Locking: none
+ */
+const char *snprintf_uuid(char *string, int len, unsigned char *uuid)
+{
+	snprintf(string, len,  "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
+		"%02x%02x-%02x%02x%02x%02x%02x%02x",
+		uuid[0],  uuid[1],  uuid[2],  uuid[3],
+		uuid[4],  uuid[5],  uuid[6],  uuid[7],
+		uuid[8],  uuid[9],  uuid[10], uuid[11],
+		uuid[12], uuid[13], uuid[14], uuid[15]);
+	return string;
+}
+
  /*
- * These functions is used to return both the bootid UUID, and random
+ * These functions are used to return both the bootid UUID, and random
   * UUID.  The difference is in whether table->data is NULL; if it is,
   * then a new UUID is generated and returned to the user.
   *
@@ -1189,7 +1214,7 @@ static int proc_do_uuid(ctl_table *table
  			void __user *buffer, size_t *lenp, loff_t *ppos)
  {
  	ctl_table fake_table;
-	unsigned char buf[64], tmp_uuid[16], *uuid;
+	unsigned char buf[37], tmp_uuid[16], *uuid;

  	uuid = table->data;
  	if (!uuid) {
@@ -1199,12 +1224,7 @@ static int proc_do_uuid(ctl_table *table
  	if (uuid[8] == 0)
  		generate_random_uuid(uuid);

-	sprintf(buf, "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-"
-		"%02x%02x%02x%02x%02x%02x",
-		uuid[0],  uuid[1],  uuid[2],  uuid[3],
-		uuid[4],  uuid[5],  uuid[6],  uuid[7],
-		uuid[8],  uuid[9],  uuid[10], uuid[11],
-		uuid[12], uuid[13], uuid[14], uuid[15]);
+	snprintf_uuid(buf, sizeof(buf), uuid);
  	fake_table.data = buf;
  	fake_table.maxlen = sizeof(buf);

--- linux-2.6.24-rc5/include/linux/random.h.org	2007-12-18 12:22:49.000000000 -0800
+++ linux-2.6.24-rc5/include/linux/random.h	2007-12-18 12:22:57.000000000 -0800
@@ -71,6 +71,7 @@ unsigned long randomize_range(unsigned l

  u32 random32(void);
  void srandom32(u32 seed);
+const char *snprintf_uuid(char *string, int len, unsigned char *uuid);

  #endif /* __KERNEL___ */

--- linux-2.6.24-rc5/kernel/panic.c.org	2007-12-18 12:23:19.000000000 -0800
+++ linux-2.6.24-rc5/kernel/panic.c	2007-12-18 12:35:46.000000000 -0800
@@ -19,6 +19,7 @@
  #include <linux/nmi.h>
  #include <linux/kexec.h>
  #include <linux/debug_locks.h>
+#include <linux/random.h>

  int panic_on_oops;
  int tainted;
@@ -32,6 +33,8 @@ ATOMIC_NOTIFIER_HEAD(panic_notifier_list

  EXPORT_SYMBOL(panic_notifier_list);

+static unsigned char oops_uuid[16];
+
  static int __init panic_setup(char *str)
  {
  	panic_timeout = simple_strtoul(str, NULL, 0);
@@ -265,15 +268,32 @@ void oops_enter(void)
  	do_oops_enter_exit();
  }

+static int prime_oops_uuid(void)
+{
+	if (oops_uuid[8] == 0)
+		generate_random_uuid(oops_uuid);
+	return 0;
+}
+
  /*
   * Called when the architecture exits its oops handler, after printing
   * everything.
   */
  void oops_exit(void)
  {
+	char uuid_string[37];
  	do_oops_enter_exit();
+
+	/*
+	 * normally the oops_uid is already calculated, but if we oops during
+	 * really early boot, it may not be. In that case, calculate it here.
+	 */
+	prime_oops_uuid();
+	printk("---[ end trace %s ]---\n",
+		snprintf_uuid(uuid_string, sizeof(uuid_string), oops_uuid));
  }

+late_initcall(prime_oops_uuid);
  #ifdef CONFIG_CC_STACKPROTECTOR
  /*
   * Called when gcc's -fstack-protector feature is used, and

View attachment "oopsend2.patch" of type "text/x-patch" (5064 bytes)

Powered by blists - more mailing lists