From e662e1cfd434aa234b72fbc781f1d70211cb785b Mon Sep 17 00:00:00 2001 From: Cyrill Gorcunov Date: Mon, 12 May 2008 14:02:22 -0700 Subject: init: don't lose initcall return values There is an ability to lose an initcall return value if it happened with irq disabled or imbalanced preemption (and if we debug initcall). Signed-off-by: Cyrill Gorcunov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- init/main.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'init/main.c') diff --git a/init/main.c b/init/main.c index ddada7acf363..f406fefa626c 100644 --- a/init/main.c +++ b/init/main.c @@ -702,7 +702,6 @@ static void __init do_initcalls(void) for (call = __initcall_start; call < __initcall_end; call++) { ktime_t t0, t1, delta; - char *msg = NULL; char msgbuf[40]; int result; @@ -724,22 +723,23 @@ static void __init do_initcalls(void) (unsigned long long) delta.tv64 >> 20); } - if (result && result != -ENODEV && initcall_debug) { - sprintf(msgbuf, "error code %d", result); - msg = msgbuf; - } + msgbuf[0] = 0; + + if (result && result != -ENODEV && initcall_debug) + sprintf(msgbuf, "error code %d ", result); + if (preempt_count() != count) { - msg = "preemption imbalance"; + strncat(msgbuf, "preemption imbalance ", sizeof(msgbuf)); preempt_count() = count; } if (irqs_disabled()) { - msg = "disabled interrupts"; + strncat(msgbuf, "disabled interrupts ", sizeof(msgbuf)); local_irq_enable(); } - if (msg) { + if (msgbuf[0]) { print_fn_descriptor_symbol(KERN_WARNING "initcall %s()", (unsigned long) *call); - printk(" returned with %s\n", msg); + printk(" returned with %s\n", msgbuf); } } -- cgit v1.2.3 From a442ac512f36981182e66a427ad05f449ff6593b Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Thu, 15 May 2008 17:50:37 -0700 Subject: Clean up 'print_fn_descriptor_symbol()' types Everybody wants to pass it a function pointer, and in fact, that is what you _must_ pass it for it to make sense (since it knows that ia64 and ppc64 use descriptors for function pointers and fetches the actual address from there). So don't make the argument be a 'unsigned long' and force everybody to add a cast. Signed-off-by: Linus Torvalds --- init/main.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'init/main.c') diff --git a/init/main.c b/init/main.c index f406fefa626c..c62215146a80 100644 --- a/init/main.c +++ b/init/main.c @@ -706,8 +706,7 @@ static void __init do_initcalls(void) int result; if (initcall_debug) { - print_fn_descriptor_symbol("calling %s()\n", - (unsigned long) *call); + print_fn_descriptor_symbol("calling %s\n", *call); t0 = ktime_get(); } @@ -717,8 +716,7 @@ static void __init do_initcalls(void) t1 = ktime_get(); delta = ktime_sub(t1, t0); - print_fn_descriptor_symbol("initcall %s()", - (unsigned long) *call); + print_fn_descriptor_symbol("initcall %s", *call); printk(" returned %d after %Ld msecs\n", result, (unsigned long long) delta.tv64 >> 20); } @@ -737,8 +735,7 @@ static void __init do_initcalls(void) local_irq_enable(); } if (msgbuf[0]) { - print_fn_descriptor_symbol(KERN_WARNING "initcall %s()", - (unsigned long) *call); + print_fn_descriptor_symbol(KERN_WARNING "initcall %s", *call); printk(" returned with %s\n", msgbuf); } } -- cgit v1.2.3 From e0df154f45e40677781e971daec6c430cb34716b Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Thu, 15 May 2008 18:14:01 -0700 Subject: Split up 'do_initcalls()' into two simpler functions One function to just loop over the entries, one function to actually do the call and the associated debugging code. Signed-off-by: Linus Torvalds --- init/main.c | 77 ++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 41 insertions(+), 36 deletions(-) (limited to 'init/main.c') diff --git a/init/main.c b/init/main.c index c62215146a80..b8bcf6da8a77 100644 --- a/init/main.c +++ b/init/main.c @@ -693,52 +693,57 @@ static int __init initcall_debug_setup(char *str) } __setup("initcall_debug", initcall_debug_setup); -extern initcall_t __initcall_start[], __initcall_end[]; - -static void __init do_initcalls(void) +static void __init do_one_initcall(initcall_t fn) { - initcall_t *call; int count = preempt_count(); + ktime_t t0, t1, delta; + char msgbuf[40]; + int result; - for (call = __initcall_start; call < __initcall_end; call++) { - ktime_t t0, t1, delta; - char msgbuf[40]; - int result; - - if (initcall_debug) { - print_fn_descriptor_symbol("calling %s\n", *call); - t0 = ktime_get(); - } + if (initcall_debug) { + print_fn_descriptor_symbol("calling %s\n", fn); + t0 = ktime_get(); + } - result = (*call)(); + result = fn(); - if (initcall_debug) { - t1 = ktime_get(); - delta = ktime_sub(t1, t0); + if (initcall_debug) { + t1 = ktime_get(); + delta = ktime_sub(t1, t0); - print_fn_descriptor_symbol("initcall %s", *call); - printk(" returned %d after %Ld msecs\n", result, - (unsigned long long) delta.tv64 >> 20); - } + print_fn_descriptor_symbol("initcall %s", fn); + printk(" returned %d after %Ld msecs\n", result, + (unsigned long long) delta.tv64 >> 20); + } - msgbuf[0] = 0; + msgbuf[0] = 0; - if (result && result != -ENODEV && initcall_debug) - sprintf(msgbuf, "error code %d ", result); + if (result && result != -ENODEV && initcall_debug) + sprintf(msgbuf, "error code %d ", result); - if (preempt_count() != count) { - strncat(msgbuf, "preemption imbalance ", sizeof(msgbuf)); - preempt_count() = count; - } - if (irqs_disabled()) { - strncat(msgbuf, "disabled interrupts ", sizeof(msgbuf)); - local_irq_enable(); - } - if (msgbuf[0]) { - print_fn_descriptor_symbol(KERN_WARNING "initcall %s", *call); - printk(" returned with %s\n", msgbuf); - } + if (preempt_count() != count) { + strncat(msgbuf, "preemption imbalance ", sizeof(msgbuf)); + preempt_count() = count; } + if (irqs_disabled()) { + strncat(msgbuf, "disabled interrupts ", sizeof(msgbuf)); + local_irq_enable(); + } + if (msgbuf[0]) { + print_fn_descriptor_symbol(KERN_WARNING "initcall %s", fn); + printk(" returned with %s\n", msgbuf); + } +} + + +extern initcall_t __initcall_start[], __initcall_end[]; + +static void __init do_initcalls(void) +{ + initcall_t *call; + + for (call = __initcall_start; call < __initcall_end; call++) + do_one_initcall(*call); /* Make sure there is no pending stuff from the initcall sequence */ flush_scheduled_work(); -- cgit v1.2.3 From a76bfd0da2321ed0a978ccbef192856ce7ed687a Mon Sep 17 00:00:00 2001 From: Cyrill Gorcunov Date: Thu, 15 May 2008 13:52:41 -0700 Subject: initcalls: Fix m68k build and possible buffer overflow This patch fixes a build bug on m68k - gcc decides to emit a call to the strlen library function, which we don't implement. More importantly - my previous patch "init: don't lose initcall return values" (commit e662e1cfd434aa234b72fbc781f1d70211cb785b) had introduced potential buffer overflow by wrong calculation of string accumulator size. Use strlcat() instead, fixing both bugs. Many thanks Andreas Schwab and Geert Uytterhoeven for helping to catch and fix the bug. Signed-off-by: Cyrill Gorcunov Cc: Geert Uytterhoeven Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- init/main.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'init/main.c') diff --git a/init/main.c b/init/main.c index b8bcf6da8a77..f7fb20021d48 100644 --- a/init/main.c +++ b/init/main.c @@ -697,7 +697,7 @@ static void __init do_one_initcall(initcall_t fn) { int count = preempt_count(); ktime_t t0, t1, delta; - char msgbuf[40]; + char msgbuf[64]; int result; if (initcall_debug) { @@ -722,11 +722,11 @@ static void __init do_one_initcall(initcall_t fn) sprintf(msgbuf, "error code %d ", result); if (preempt_count() != count) { - strncat(msgbuf, "preemption imbalance ", sizeof(msgbuf)); + strlcat(msgbuf, "preemption imbalance ", sizeof(msgbuf)); preempt_count() = count; } if (irqs_disabled()) { - strncat(msgbuf, "disabled interrupts ", sizeof(msgbuf)); + strlcat(msgbuf, "disabled interrupts ", sizeof(msgbuf)); local_irq_enable(); } if (msgbuf[0]) { -- cgit v1.2.3