kernel/panic: Filter out a potential trailing newline
RFC PATCH 1/1 kernel/panic: Filter out a potential trailing newline
From: Borislav Petkov <[email protected]>
If a call to panic() terminates the string with a \n, the result puts the closing brace ']---' on a newline because panic() itself adds \n too.
Now, if one goes and removes the newline chars from all panic() invocations - and the stats right now look like this:
~300 calls with an \n
~500 calls without a \n
one is destined to a neverending game of whack-a-mole because the usual thing to do is add a newline at the end of a string a function is supposed to print.
Therefore, simply zap any \n at the end of the panic string to avoid touching so many places in the kernel.
--- kernel/panic.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
kernel/panic.c +6 -2
@@ -136,7 +136,7 @@ void panic(const char *fmt, ...) {static char buf[1024 ];va_list args; -long i, i_next =0 ; +long i, i_next =0 , len;int state =0 ;int old_cpu, this_cpu;bool _crash_kexec_post_notifiers = crash_kexec_post_notifiers;
@@ -173,8 +173,12 @@ void panic(const char *fmt, ...)console_verbose ();bust_spinlocks (1 );va_start (args, fmt); -vsnprintf (buf,sizeof (buf), fmt, args); + len =vscnprintf (buf,sizeof (buf), fmt, args);va_end (args); + +if (len && buf[len -1 ] =='\n' ) + buf[len -1 ] ='\0' ; +pr_emerg ("Kernel panic - not syncing: %s\n" , buf);#ifdef CONFIG_DEBUG_BUGVERBOSE /*
Signature · 1 line
2.19.0.271.gfe8321ec057f
Quoted 16 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
If a call to
panic()terminates the string with a \n, the result puts the closing brace ']---' on a newline becausepanic()itself adds \n too.Now, if one goes and removes the newline chars from all
panic()invocations - and the stats right now look like this:~300 calls with an \n
~500 calls without a \none is destined to a neverending game of whack-a-mole because the usual thing to do is add a newline at the end of a string a function is supposed to print.
Quoted 8 lines
Therefore, simply zap any \n at the end of the panic string to avoid touching so many places in the kernel.
Signed-off-by Borislav Petkov <[email protected]>Cc Andrew Morton <[email protected]>Cc Kees Cook <[email protected]>Cc "Steven Rostedt (VMware)" <[email protected]>
Ah yes, I like this. :)
-Kees
Quoted 34 lines
--- kernel/panic.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)kernel/panic.c +6
-3@@ -136,7 +136,7 @@ void panic(const char *fmt, ...) {static char buf[1024 ];va_list args; -long i, i_next =0 ; +long i, i_next =0 , len;int state =0 ;int old_cpu, this_cpu;bool _crash_kexec_post_notifiers = crash_kexec_post_notifiers;@@ -173,8 +173,12 @@ void panic(const char *fmt, ...)console_verbose ();bust_spinlocks (1 );va_start (args, fmt); -vsnprintf (buf,sizeof (buf), fmt, args); + len =vscnprintf (buf,sizeof (buf), fmt, args);va_end (args); + +if (len && buf[len -1 ] =='\n' ) + buf[len -1 ] ='\0' ; +pr_emerg ("Kernel panic - not syncing: %s\n" , buf);#ifdef CONFIG_DEBUG_BUGVERBOSE /* -- 2.19.0.271.gfe8321ec057f
Signature · 2 lines
Kees Cook Pixel Security
Quoted 45 lines · On Tue, 9 Oct 2018 22:50:19 +0200 Borislav Petkov <[email protected]> wrote:
From: Borislav Petkov <[email protected]>
If a call to
panic()terminates the string with a \n, the result puts the closing brace ']---' on a newline becausepanic()itself adds \n too.Now, if one goes and removes the newline chars from all
panic()invocations - and the stats right now look like this:~300 calls with an \n
~500 calls without a \none is destined to a neverending game of whack-a-mole because the usual thing to do is add a newline at the end of a string a function is supposed to print.
Therefore, simply zap any \n at the end of the panic string to avoid touching so many places in the kernel.
Signed-off-by Borislav Petkov <[email protected]>Cc Andrew Morton <[email protected]>Cc Kees Cook <[email protected]>Cc "Steven Rostedt (VMware)" <[email protected]>--- kernel/panic.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)kernel/panic.c +1
-1@@ -136,7 +136,7 @@ void panic(const char *fmt, ...) {static char buf[1024 ];va_list args; -long i, i_next =0 ; +long i, i_next =0 , len;int state =0 ;int old_cpu, this_cpu;bool _crash_kexec_post_notifiers = crash_kexec_post_notifiers;@@ -173,8 +173,12 @@ void panic(const char *fmt, ...)console_verbose ();bust_spinlocks (1 );va_start (args, fmt);
kernel/panic.c void panic(const char *fmt, ...) · line 184
Quoted 10 lines
kernel/panic.c +5
-1-vsnprintf (buf,sizeof (buf), fmt, args); + len =vscnprintf (buf,sizeof (buf), fmt, args);va_end (args); + +if (len && buf[len -1 ] =='\n' ) + buf[len -1 ] ='\0' ; +pr_emerg ("Kernel panic - not syncing: %s\n" , buf);#ifdef CONFIG_DEBUG_BUGVERBOSE /*
-- Steve
Quoted 42 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
If a call to
panic()terminates the string with a \n, the result puts the closing brace ']---' on a newline becausepanic()itself adds \n too.Now, if one goes and removes the newline chars from all
panic()invocations - and the stats right now look like this:~300 calls with an \n
~500 calls without a \none is destined to a neverending game of whack-a-mole because the usual thing to do is add a newline at the end of a string a function is supposed to print.
Therefore, simply zap any \n at the end of the panic string to avoid touching so many places in the kernel.
Signed-off-by Borislav Petkov <[email protected]>Cc Andrew Morton <[email protected]>Cc Kees Cook <[email protected]>Cc "Steven Rostedt (VMware)" <[email protected]>--- kernel/panic.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)kernel/panic.c +1
-1@@ -136,7 +136,7 @@ void panic(const char *fmt, ...) {static char buf[1024 ];va_list args; -long i, i_next =0 ; +long i, i_next =0 , len;int state =0 ;int old_cpu, this_cpu;bool _crash_kexec_post_notifiers = crash_kexec_post_notifiers;
kernel/panic.c void panic(const char *fmt, ...) · line 180
Quoted 9 lines
kernel/panic.c +4
-1@@ -173,8 +173,12 @@ void panic(const char *fmt, ...)console_verbose ();bust_spinlocks (1 );va_start (args, fmt); -vsnprintf (buf,sizeof (buf), fmt, args); + len =vscnprintf (buf,sizeof (buf), fmt, args);va_end (args); + +if (len && buf[len -1 ] =='\n' ) + buf[len -1 ] ='\0' ;
Nice, this patch saves a couple of hundred trivial upstream commits in the next 20 years:
Thanks,
Ingo