Re: Top kernel oopses/warnings this week

Arjan van de Ven
2007-12-18T23:42:31Z

[-- Attachment #1: Type: text/plain, Size: 5513 bytes --]

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@linux.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@linux.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

[-- Attachment #2: oopsend2.patch --]
[-- Type: text/x-patch, Size: 5063 bytes --]

From: Arjan van de Ven <arjan@linux.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@linux.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