Hello all, In a small exchange in fedora-kernel-list [1] Eric Sandeen has pointed out a possible stack overflow... when CONFIG_DEBUG_STACKOVERFLOW is enabled. (Though not limited to it)
Code path is simple: do_IRQ detects a a near stack overflow condition and calls show_trace_log_lvl which, down the line uses __print_symbol and sprint_symbol to print the call stack. However, both __print_symbol + sprint_symbol are eating no-less then 128+223 bytes on static char arrays, which, given the fact that this code path is actually generated by low stack warning (< 512 bytes), might turn a minor (?) problem (low stack) into a full blown crash. The patch itself is fairly simple and non-intrusive. [2] Both functions allocate memory for their buffers - falling back to minimal address display if memory allocation fails. P.S. Can anyone please point me to the maintainer of kernel/syms? (I rather not spam world + dog for such a minor patch) -- Gilboa Davara <[EMAIL PROTECTED]> [1] http://www.mail-archive.com/[EMAIL PROTECTED]/msg00640.html [2]. In theory, there's a second option: pre-allocating memory on a per_cpu basis, however: A. dump_trace/stack are usually called when something bad has happened - reducing the need for performance optimizations. B. per_cpu allocation will also require local_irq_disable/enable as both functions are being called from multiple contexts. Too much hassle. --- linux-2.6/kernel/kallsyms.orig 2007-09-15 11:46:54.000000000 +0300 +++ linux-2.6/kernel/kallsyms.c 2007-09-15 14:25:21.000000000 +0300 @@ -309,30 +309,62 @@ int lookup_symbol_attrs(unsigned long ad /* Look up a kernel symbol and return it in a text buffer. */ int sprint_symbol(char *buffer, unsigned long address) { - char *modname; - const char *name; unsigned long offset, size; - char namebuf[KSYM_NAME_LEN]; + const char *name = NULL; + char *namebuf = NULL; + char *modname; + int ret; + + + /* Static buffer allocation. + Required in-order to reduce stack footprint on + do_IRQ/4KSTACK/i386 */ + namebuf = kmalloc(KSYM_NAME_LEN, GFP_ATOMIC); + if (namebuf) + name = kallsyms_lookup(address, &size, &offset, + &modname, namebuf); - name = kallsyms_lookup(address, &size, &offset, &modname, namebuf); if (!name) - return sprintf(buffer, "0x%lx", address); + ret = sprintf(buffer, "0x%lx", address); + else { + if (modname) + ret = sprintf(buffer, "%s+%#lx/%#lx [%s]", + name, offset, size, modname); + else + ret = sprintf(buffer, "%s+%#lx/%#lx", + name, offset, size); + } - if (modname) - return sprintf(buffer, "%s+%#lx/%#lx [%s]", name, offset, - size, modname); - else - return sprintf(buffer, "%s+%#lx/%#lx", name, offset, size); + if (namebuf) + kfree(namebuf); + + return ret; } /* Look up a kernel symbol and print it to the kernel messages. */ void __print_symbol(const char *fmt, unsigned long address) { - char buffer[KSYM_SYMBOL_LEN]; + char *buffer = NULL; - sprint_symbol(buffer, address); - printk(fmt, buffer); + /* Static buffer allocation. + Required in-order to reduce stack footprint on + do_IRQ/4KSTACK/i386 */ + buffer = kmalloc(KSYM_SYMBOL_LEN, GFP_ATOMIC); + if (buffer) { + sprint_symbol(buffer, address); + printk(fmt, buffer); + kfree(buffer); + } else { + /* Address + '0x' + NULL. */ + char sbuffer[(BITS_PER_LONG / 4) + 3]; + + /* Fall-back mode. + Memory allocation failed. + Convert the address to string and display it. */ + sprintf(sbuffer, "0x%lx", address); + printk(fmt, sbuffer); + } } /* To avoid using get_symbol_offset for every symbol, we carry prefix along. */ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
