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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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
From: Arjan van de Ven <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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