From caa72c3bc584bc28b557bcf1a47532a7a6f37e6f Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 3 Feb 2020 15:31:25 +0200 Subject: console: Drop double check for console_drivers being non-NULL There is no need to explicitly check for console_drivers to be non-NULL since for_each_console() does this. Link: http://lkml.kernel.org/r/20200203133130.11591-2-andriy.shevchenko@linux.intel.com To: linux-kernel@vger.kernel.org To: Steven Rostedt Signed-off-by: Andy Shevchenko Reviewed-by: Petr Mladek Reviewed-by: Sergey Senozhatsky Signed-off-by: Petr Mladek --- kernel/printk/printk.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index fada22dc4ab6..51337ed426e0 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -1772,9 +1772,6 @@ static void call_console_drivers(const char *ext_text, size_t ext_len, trace_console_rcuidle(text, len); - if (!console_drivers) - return; - for_each_console(con) { if (exclusive_console && con != exclusive_console) continue; @@ -2653,18 +2650,17 @@ void register_console(struct console *newcon) struct console_cmdline *c; static bool has_preferred; - if (console_drivers) - for_each_console(bcon) - if (WARN(bcon == newcon, - "console '%s%d' already registered\n", - bcon->name, bcon->index)) - return; + for_each_console(bcon) { + if (WARN(bcon == newcon, "console '%s%d' already registered\n", + bcon->name, bcon->index)) + return; + } /* * before we register a new CON_BOOT console, make sure we don't * already have a valid console */ - if (console_drivers && newcon->flags & CON_BOOT) { + if (newcon->flags & CON_BOOT) { /* find the last or real console */ for_each_console(bcon) { if (!(bcon->flags & CON_BOOT)) { -- cgit v1.2.3 From 12825e6ba8ea8efbb6d81363b0cf6b5bf84c051a Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 3 Feb 2020 15:31:26 +0200 Subject: console: Use for_each_console() helper in unregister_console() We have rather open coded single linked list manipulations where we may simple use for_each_console() helper with properly set exit conditions. Replace open coded single-linked list handling with for_each_console() helper in use. Link: http://lkml.kernel.org/r/20200203133130.11591-3-andriy.shevchenko@linux.intel.com To: linux-kernel@vger.kernel.org To: Steven Rostedt Signed-off-by: Andy Shevchenko Reviewed-by: Petr Mladek Reviewed-by: Sergey Senozhatsky Signed-off-by: Petr Mladek --- kernel/printk/printk.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 51337ed426e0..d40a316908da 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2809,7 +2809,7 @@ EXPORT_SYMBOL(register_console); int unregister_console(struct console *console) { - struct console *a, *b; + struct console *con; int res; pr_info("%sconsole [%s%d] disabled\n", @@ -2825,11 +2825,10 @@ int unregister_console(struct console *console) if (console_drivers == console) { console_drivers=console->next; res = 0; - } else if (console_drivers) { - for (a=console_drivers->next, b=console_drivers ; - a; b=a, a=b->next) { - if (a == console) { - b->next = a->next; + } else { + for_each_console(con) { + if (con->next == console) { + con->next = console->next; res = 0; break; } -- cgit v1.2.3 From d58ad10122e6f8f84b1ef60227233b7c5de2bc02 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 3 Feb 2020 15:31:27 +0200 Subject: console: Drop misleading comment /* find the last or real console */ This comment is misleading. The purpose of the loop is to check if we are trying to register boot console after a real one has already been registered. This is already mentioned in a comment above. Link: http://lkml.kernel.org/r/20200203133130.11591-4-andriy.shevchenko@linux.intel.com To: linux-kernel@vger.kernel.org To: Steven Rostedt Suggested-by: Petr Mladek Signed-off-by: Andy Shevchenko [pmladek@suse.com: Updated commit message.] Reviewed-by: Sergey Senozhatsky Signed-off-by: Petr Mladek --- kernel/printk/printk.c | 1 - 1 file changed, 1 deletion(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index d40a316908da..b0c12c08cac4 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2661,7 +2661,6 @@ void register_console(struct console *newcon) * already have a valid console */ if (newcon->flags & CON_BOOT) { - /* find the last or real console */ for_each_console(bcon) { if (!(bcon->flags & CON_BOOT)) { pr_info("Too late to register bootconsole %s%d\n", -- cgit v1.2.3 From bb72e3981d8e7bb80db7cebc57a4e10769a509c9 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 3 Feb 2020 15:31:28 +0200 Subject: console: Avoid positive return code from unregister_console() There are only two callers that use the returned code from unregister_console(): - unregister_early_console() in arch/m68k/kernel/early_printk.c - kgdb_unregister_nmi_console() in drivers/tty/serial/kgdb_nmi.c They both expect to get "0" on success and a non-zero value on error. But the current behavior is confusing and buggy: - _braille_unregister_console() returns "1" on success - unregister_console() returns "1" on error Fix and clean up the behavior: - Return success when _braille_unregister_console() succeeded - Return a meaningful error code when the console was not registered before Link: http://lkml.kernel.org/r/20200203133130.11591-5-andriy.shevchenko@linux.intel.com To: linux-kernel@vger.kernel.org To: Steven Rostedt Signed-off-by: Andy Shevchenko Reviewed-by: Sergey Senozhatsky Signed-off-by: Petr Mladek --- kernel/printk/printk.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index b0c12c08cac4..61d188f4c672 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2816,10 +2816,12 @@ int unregister_console(struct console *console) console->name, console->index); res = _braille_unregister_console(console); - if (res) + if (res < 0) return res; + if (res > 0) + return 0; - res = 1; + res = -ENODEV; console_lock(); if (console_drivers == console) { console_drivers=console->next; -- cgit v1.2.3 From e78bedbd42b7caa65551f3d56cbff451ccbd0aee Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 3 Feb 2020 15:31:29 +0200 Subject: console: Don't notify user space when unregister non-listed console If console is not on the list then there is nothing for us to do and sysfs notify is pointless. Note, that nr_ext_console_drivers is being changed only for listed consoles. Suggested-by: Sergey Senozhatsky Link: http://lkml.kernel.org/r/20200203133130.11591-6-andriy.shevchenko@linux.intel.com To: linux-kernel@vger.kernel.org To: Steven Rostedt Signed-off-by: Andy Shevchenko Reviewed-by: Sergey Senozhatsky Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek --- kernel/printk/printk.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 61d188f4c672..f435a17ef988 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2836,7 +2836,10 @@ int unregister_console(struct console *console) } } - if (!res && (console->flags & CON_EXTENDED)) + if (res) + goto out_disable_unlock; + + if (console->flags & CON_EXTENDED) nr_ext_console_drivers--; /* @@ -2849,6 +2852,13 @@ int unregister_console(struct console *console) console->flags &= ~CON_ENABLED; console_unlock(); console_sysfs_notify(); + + return res; + +out_disable_unlock: + console->flags &= ~CON_ENABLED; + console_unlock(); + return res; } EXPORT_SYMBOL(unregister_console); -- cgit v1.2.3 From ed31685c96e18f773ca11dd1a637974d62130673 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 3 Feb 2020 15:31:30 +0200 Subject: console: Introduce ->exit() callback Some consoles might require special operations on unregistering. For instance, serial console, when registered in the kernel, keeps power on for entire time, until it gets unregistered. Example of use: ->setup(console): pm_runtime_get(...); ->exit(console): pm_runtime_put(...); For such cases to have a balance we would provide ->exit() callback. Link: http://lkml.kernel.org/r/20200203133130.11591-7-andriy.shevchenko@linux.intel.com To: linux-kernel@vger.kernel.org To: Steven Rostedt Signed-off-by: Andy Shevchenko Reviewed-by: Sergey Senozhatsky Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek --- kernel/printk/printk.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index f435a17ef988..633f41a11d75 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2853,6 +2853,9 @@ int unregister_console(struct console *console) console_unlock(); console_sysfs_notify(); + if (console->exit) + res = console->exit(console); + return res; out_disable_unlock: -- cgit v1.2.3 From 5661dd95a2958634485bb1a53f90a6ab621d4b0c Mon Sep 17 00:00:00 2001 From: Nathan Chancellor Date: Thu, 30 Jan 2020 15:16:44 -0700 Subject: printk: Convert a use of sprintf to snprintf in console_unlock When CONFIG_PRINTK is disabled (e.g. when building allnoconfig), clang warns: ../kernel/printk/printk.c:2416:10: warning: 'sprintf' will always overflow; destination buffer has size 0, but format string expands to at least 33 [-Wfortify-source] len = sprintf(text, ^ 1 warning generated. It is not wrong; text has a zero size when CONFIG_PRINTK is disabled because LOG_LINE_MAX and PREFIX_MAX are both zero. Change to snprintf so that this case is explicitly handled without any risk of overflow. Link: https://github.com/ClangBuiltLinux/linux/issues/846 Link: https://github.com/llvm/llvm-project/commit/6d485ff455ea2b37fef9e06e426dae6c1241b231 Link: http://lkml.kernel.org/r/20200130221644.2273-1-natechancellor@gmail.com Cc: Steven Rostedt Cc: linux-kernel@vger.kernel.org Cc: clang-built-linux@googlegroups.com Signed-off-by: Nathan Chancellor Reviewed-by: Sergey Senozhatsky Signed-off-by: Petr Mladek --- kernel/printk/printk.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index fada22dc4ab6..a44094727a5c 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2413,9 +2413,9 @@ again: printk_safe_enter_irqsave(flags); raw_spin_lock(&logbuf_lock); if (console_seq < log_first_seq) { - len = sprintf(text, - "** %llu printk messages dropped **\n", - log_first_seq - console_seq); + len = snprintf(text, sizeof(text), + "** %llu printk messages dropped **\n", + log_first_seq - console_seq); /* messages are gone, move to first one */ console_seq = log_first_seq; -- cgit v1.2.3 From ad8cd1db80cc33337bdbee63c453ef6d5132474b Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Thu, 13 Feb 2020 10:51:31 +0100 Subject: printk: Move console matching logic into a separate function This moves the loop that tries to match a newly registered console with the command line or add_preferred_console list into a separate helper, in order to be able to call it multiple times in subsequent patches. Link: https://lore.kernel.org/r/20200213095133.23176-2-pmladek@suse.com Signed-off-by: Benjamin Herrenschmidt Reviewed-by: Sergey Senozhatsky Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek --- kernel/printk/printk.c | 105 ++++++++++++++++++++++++++++++------------------- 1 file changed, 65 insertions(+), 40 deletions(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index fada22dc4ab6..0ebcdf53e75d 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -280,6 +280,7 @@ static struct console *exclusive_console; static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES]; static int preferred_console = -1; +static bool has_preferred_console; int console_set_on_cmdline; EXPORT_SYMBOL(console_set_on_cmdline); @@ -2626,6 +2627,60 @@ static int __init keep_bootcon_setup(char *str) early_param("keep_bootcon", keep_bootcon_setup); +/* + * This is called by register_console() to try to match + * the newly registered console with any of the ones selected + * by either the command line or add_preferred_console() and + * setup/enable it. + * + * Care need to be taken with consoles that are statically + * enabled such as netconsole + */ +static int try_enable_new_console(struct console *newcon) +{ + struct console_cmdline *c; + int i; + + for (i = 0, c = console_cmdline; + i < MAX_CMDLINECONSOLES && c->name[0]; + i++, c++) { + if (!newcon->match || + newcon->match(newcon, c->name, c->index, c->options) != 0) { + /* default matching */ + BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name)); + if (strcmp(c->name, newcon->name) != 0) + continue; + if (newcon->index >= 0 && + newcon->index != c->index) + continue; + if (newcon->index < 0) + newcon->index = c->index; + + if (_braille_register_console(newcon, c)) + return 0; + + if (newcon->setup && + newcon->setup(newcon, c->options) != 0) + return -EIO; + } + newcon->flags |= CON_ENABLED; + if (i == preferred_console) { + newcon->flags |= CON_CONSDEV; + has_preferred_console = true; + } + return 0; + } + + /* + * Some consoles, such as pstore and netconsole, can be enabled even + * without matching. + */ + if (newcon->flags & CON_ENABLED) + return 0; + + return -ENOENT; +} + /* * The console driver calls this routine during kernel initialization * to register the console printing procedure with printk() and to @@ -2647,11 +2702,9 @@ early_param("keep_bootcon", keep_bootcon_setup); */ void register_console(struct console *newcon) { - int i; unsigned long flags; struct console *bcon = NULL; - struct console_cmdline *c; - static bool has_preferred; + int err; if (console_drivers) for_each_console(bcon) @@ -2678,15 +2731,15 @@ void register_console(struct console *newcon) if (console_drivers && console_drivers->flags & CON_BOOT) bcon = console_drivers; - if (!has_preferred || bcon || !console_drivers) - has_preferred = preferred_console >= 0; + if (!has_preferred_console || bcon || !console_drivers) + has_preferred_console = preferred_console >= 0; /* * See if we want to use this console driver. If we * didn't select a console we take the first one * that registers here. */ - if (!has_preferred) { + if (!has_preferred_console) { if (newcon->index < 0) newcon->index = 0; if (newcon->setup == NULL || @@ -2694,47 +2747,19 @@ void register_console(struct console *newcon) newcon->flags |= CON_ENABLED; if (newcon->device) { newcon->flags |= CON_CONSDEV; - has_preferred = true; + has_preferred_console = true; } } } /* - * See if this console matches one we selected on - * the command line. + * See if this console matches one we selected on + * the command line or if it was statically enabled */ - for (i = 0, c = console_cmdline; - i < MAX_CMDLINECONSOLES && c->name[0]; - i++, c++) { - if (!newcon->match || - newcon->match(newcon, c->name, c->index, c->options) != 0) { - /* default matching */ - BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name)); - if (strcmp(c->name, newcon->name) != 0) - continue; - if (newcon->index >= 0 && - newcon->index != c->index) - continue; - if (newcon->index < 0) - newcon->index = c->index; - - if (_braille_register_console(newcon, c)) - return; - - if (newcon->setup && - newcon->setup(newcon, c->options) != 0) - break; - } - - newcon->flags |= CON_ENABLED; - if (i == preferred_console) { - newcon->flags |= CON_CONSDEV; - has_preferred = true; - } - break; - } + err = try_enable_new_console(newcon); - if (!(newcon->flags & CON_ENABLED)) + /* printk() messages are not printed to the Braille console. */ + if (err || newcon->flags & CON_BRL) return; /* -- cgit v1.2.3 From e369d8227fd211be36242fc44a9dc2209e246b9a Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Thu, 13 Feb 2020 10:51:32 +0100 Subject: printk: Fix preferred console selection with multiple matches In the following circumstances, the rule of selecting the console corresponding to the last "console=" entry on the command line as the preferred console (CON_CONSDEV, ie, /dev/console) fails. This is a specific example, but it could happen with different consoles that have a similar name aliasing mechanism. - The kernel command line has both console=tty0 and console=ttyS0 in that order (the latter with speed etc... arguments). This is common with some cloud setups such as Amazon Linux. - add_preferred_console is called early to register "uart0". In our case that happens from acpi_parse_spcr() on arm64 since the "enable_console" argument is true on that architecture. This causes "uart0" to become entry 0 of the console_cmdline array. Now, because of the above, what happens is: - add_preferred_console is called by the cmdline parsing for tty0 and ttyS0 respectively, thus occupying entries 1 and 2 of the console_cmdline array (since this happens after ACPI SPCR parsing). At that point preferred_console is set to 2 as expected. - When the tty layer kicks in, it will call register_console for tty0. This will match entry 1 in console_cmdline array. It isn't our preferred console but because it's our only console at this point, it will end up "first" in the consoles list. - When 8250 probes the actual serial port later on, it calls register_console for ttyS0. At that point the loop in register_console tries to match it with the entries in the console_cmdline array. Ideally this should match ttyS0 in entry 2, which is preferred, causing it to be inserted first and to replace tty0 as CONSDEV. However, 8250 provides a "match" hook in its struct console, and that hook will match "uart" as an alias to "ttyS". So we match uart0 at entry 0 in the array which is not the preferred console and will not match entry 2 which is since we break out of the loop on the first match. As a result, we don't set CONSDEV and don't insert it first, but second in the console list. As a result, we end up with tty0 remaining first in the array, and thus /dev/console going there instead of the last user specified one which is ttyS0. This tentative fix register_console() to scan first for consoles specified on the command line, and only if none is found, to then scan for consoles specified by the architecture. Link: https://lore.kernel.org/r/20200213095133.23176-3-pmladek@suse.com Signed-off-by: Benjamin Herrenschmidt Reviewed-by: Petr Mladek Reviewed-by: Sergey Senozhatsky Signed-off-by: Petr Mladek --- kernel/printk/console_cmdline.h | 1 + kernel/printk/printk.c | 29 ++++++++++++++++++----------- 2 files changed, 19 insertions(+), 11 deletions(-) (limited to 'kernel/printk') diff --git a/kernel/printk/console_cmdline.h b/kernel/printk/console_cmdline.h index 11f19c466af5..3ca74ad391d6 100644 --- a/kernel/printk/console_cmdline.h +++ b/kernel/printk/console_cmdline.h @@ -6,6 +6,7 @@ struct console_cmdline { char name[16]; /* Name of the driver */ int index; /* Minor dev. to use */ + bool user_specified; /* Specified by command line vs. platform */ char *options; /* Options for the driver */ #ifdef CONFIG_A11Y_BRAILLE_CONSOLE char *brl_options; /* Options for braille driver */ diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 0ebcdf53e75d..f76ef3f0efca 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2116,7 +2116,7 @@ asmlinkage __visible void early_printk(const char *fmt, ...) #endif static int __add_preferred_console(char *name, int idx, char *options, - char *brl_options) + char *brl_options, bool user_specified) { struct console_cmdline *c; int i; @@ -2131,6 +2131,8 @@ static int __add_preferred_console(char *name, int idx, char *options, if (strcmp(c->name, name) == 0 && c->index == idx) { if (!brl_options) preferred_console = i; + if (user_specified) + c->user_specified = true; return 0; } } @@ -2140,6 +2142,7 @@ static int __add_preferred_console(char *name, int idx, char *options, preferred_console = i; strlcpy(c->name, name, sizeof(c->name)); c->options = options; + c->user_specified = user_specified; braille_set_options(c, brl_options); c->index = idx; @@ -2194,7 +2197,7 @@ static int __init console_setup(char *str) idx = simple_strtoul(s, NULL, 10); *s = 0; - __add_preferred_console(buf, idx, options, brl_options); + __add_preferred_console(buf, idx, options, brl_options, true); console_set_on_cmdline = 1; return 1; } @@ -2215,7 +2218,7 @@ __setup("console=", console_setup); */ int add_preferred_console(char *name, int idx, char *options) { - return __add_preferred_console(name, idx, options, NULL); + return __add_preferred_console(name, idx, options, NULL, false); } bool console_suspend_enabled = true; @@ -2636,7 +2639,7 @@ early_param("keep_bootcon", keep_bootcon_setup); * Care need to be taken with consoles that are statically * enabled such as netconsole */ -static int try_enable_new_console(struct console *newcon) +static int try_enable_new_console(struct console *newcon, bool user_specified) { struct console_cmdline *c; int i; @@ -2644,6 +2647,8 @@ static int try_enable_new_console(struct console *newcon) for (i = 0, c = console_cmdline; i < MAX_CMDLINECONSOLES && c->name[0]; i++, c++) { + if (c->user_specified != user_specified) + continue; if (!newcon->match || newcon->match(newcon, c->name, c->index, c->options) != 0) { /* default matching */ @@ -2673,9 +2678,10 @@ static int try_enable_new_console(struct console *newcon) /* * Some consoles, such as pstore and netconsole, can be enabled even - * without matching. + * without matching. Accept the pre-enabled consoles only when match() + * and setup() had a change to be called. */ - if (newcon->flags & CON_ENABLED) + if (newcon->flags & CON_ENABLED && c->user_specified == user_specified) return 0; return -ENOENT; @@ -2752,11 +2758,12 @@ void register_console(struct console *newcon) } } - /* - * See if this console matches one we selected on - * the command line or if it was statically enabled - */ - err = try_enable_new_console(newcon); + /* See if this console matches one we selected on the command line */ + err = try_enable_new_console(newcon, true); + + /* If not, try to match against the platform default(s) */ + if (err == -ENOENT) + err = try_enable_new_console(newcon, false); /* printk() messages are not printed to the Braille console. */ if (err || newcon->flags & CON_BRL) -- cgit v1.2.3 From 33225d7b0ac9903c5701bbede7dfdaeba74ad6c3 Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Thu, 13 Feb 2020 10:51:33 +0100 Subject: printk: Correctly set CON_CONSDEV even when preferred console was not registered CON_CONSDEV flag was historically used to put/keep the preferred console first in console_drivers list. Where the preferred console is the last on the command line. The ordering is important only when opening /dev/console: + tty_kopen() + tty_lookup_driver() + console_device() The flag was originally an implementation detail. But it was later made accessible from userspace via /proc/consoles. It was used, for example, by the tool "showconsole" to show the real tty accessible via /dev/console, see https://github.com/bitstreamout/showconsole Now, the current code sets CON_CONSDEV only for the preferred console or when a fallback console is added. The flag is not set when the preferred console is defined on the command line but it is not registered from some reasons. Simple solution is to set CON_CONSDEV flag for the first registered console. It will work most of the time because: + Most real consoles have console->device defined. + Boot consoles are removed in printk_late_init(). + unregister_console() moves CON_CONSDEV flag to the next console. Clean solution would require checking con->device when the preferred console is registered and in unregister_console(). Conclusion: Use the simple solution for now. It is better than the current state and good enough. The clean solution is not worth it. It would complicate the already complicated code without too much gain. Instead the code would deserve a complete rewrite. Link: https://lore.kernel.org/r/20200213095133.23176-4-pmladek@suse.com Signed-off-by: Benjamin Herrenschmidt [pmladek@suse.com: Correct reasoning in the commit message, comment update.] Reviewed-by: Petr Mladek Reviewed-by: Sergey Senozhatsky Signed-off-by: Petr Mladek --- kernel/printk/printk.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index f76ef3f0efca..cf0ceacdae2f 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2788,6 +2788,8 @@ void register_console(struct console *newcon) console_drivers = newcon; if (newcon->next) newcon->next->flags &= ~CON_CONSDEV; + /* Ensure this flag is always set for the head of the list */ + newcon->flags |= CON_CONSDEV; } else { newcon->next = console_drivers->next; console_drivers->next = newcon; -- cgit v1.2.3 From ab6f762f0f53162d41497708b33c9a3236d3609e Mon Sep 17 00:00:00 2001 From: Sergey Senozhatsky Date: Tue, 3 Mar 2020 20:30:02 +0900 Subject: printk: queue wake_up_klogd irq_work only if per-CPU areas are ready printk_deferred(), similarly to printk_safe/printk_nmi, does not immediately attempt to print a new message on the consoles, avoiding calls into non-reentrant kernel paths, e.g. scheduler or timekeeping, which potentially can deadlock the system. Those printk() flavors, instead, rely on per-CPU flush irq_work to print messages from safer contexts. For same reasons (recursive scheduler or timekeeping calls) printk() uses per-CPU irq_work in order to wake up user space syslog/kmsg readers. However, only printk_safe/printk_nmi do make sure that per-CPU areas have been initialised and that it's safe to modify per-CPU irq_work. This means that, for instance, should printk_deferred() be invoked "too early", that is before per-CPU areas are initialised, printk_deferred() will perform illegal per-CPU access. Lech Perczak [0] reports that after commit 1b710b1b10ef ("char/random: silence a lockdep splat with printk()") user-space syslog/kmsg readers are not able to read new kernel messages. The reason is printk_deferred() being called too early (as was pointed out by Petr and John). Fix printk_deferred() and do not queue per-CPU irq_work before per-CPU areas are initialized. Link: https://lore.kernel.org/lkml/aa0732c6-5c4e-8a8b-a1c1-75ebe3dca05b@camlintechnologies.com/ Reported-by: Lech Perczak Signed-off-by: Sergey Senozhatsky Tested-by: Jann Horn Reviewed-by: Petr Mladek Cc: Greg Kroah-Hartman Cc: Theodore Ts'o Cc: John Ogness Signed-off-by: Linus Torvalds --- kernel/printk/internal.h | 5 +++++ kernel/printk/printk.c | 34 ++++++++++++++++++++++++++++++++++ kernel/printk/printk_safe.c | 11 +---------- 3 files changed, 40 insertions(+), 10 deletions(-) (limited to 'kernel/printk') diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h index c8e6ab689d42..b2b0f526f249 100644 --- a/kernel/printk/internal.h +++ b/kernel/printk/internal.h @@ -23,6 +23,9 @@ __printf(1, 0) int vprintk_func(const char *fmt, va_list args); void __printk_safe_enter(void); void __printk_safe_exit(void); +void printk_safe_init(void); +bool printk_percpu_data_ready(void); + #define printk_safe_enter_irqsave(flags) \ do { \ local_irq_save(flags); \ @@ -64,4 +67,6 @@ __printf(1, 0) int vprintk_func(const char *fmt, va_list args) { return 0; } #define printk_safe_enter_irq() local_irq_disable() #define printk_safe_exit_irq() local_irq_enable() +static inline void printk_safe_init(void) { } +static inline bool printk_percpu_data_ready(void) { return false; } #endif /* CONFIG_PRINTK */ diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 633f41a11d75..9a9b6156270b 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -460,6 +460,18 @@ static char __log_buf[__LOG_BUF_LEN] __aligned(LOG_ALIGN); static char *log_buf = __log_buf; static u32 log_buf_len = __LOG_BUF_LEN; +/* + * We cannot access per-CPU data (e.g. per-CPU flush irq_work) before + * per_cpu_areas are initialised. This variable is set to true when + * it's safe to access per-CPU data. + */ +static bool __printk_percpu_data_ready __read_mostly; + +bool printk_percpu_data_ready(void) +{ + return __printk_percpu_data_ready; +} + /* Return log buffer address */ char *log_buf_addr_get(void) { @@ -1146,12 +1158,28 @@ static void __init log_buf_add_cpu(void) static inline void log_buf_add_cpu(void) {} #endif /* CONFIG_SMP */ +static void __init set_percpu_data_ready(void) +{ + printk_safe_init(); + /* Make sure we set this flag only after printk_safe() init is done */ + barrier(); + __printk_percpu_data_ready = true; +} + void __init setup_log_buf(int early) { unsigned long flags; char *new_log_buf; unsigned int free; + /* + * Some archs call setup_log_buf() multiple times - first is very + * early, e.g. from setup_arch(), and second - when percpu_areas + * are initialised. + */ + if (!early) + set_percpu_data_ready(); + if (log_buf != __log_buf) return; @@ -2975,6 +3003,9 @@ static DEFINE_PER_CPU(struct irq_work, wake_up_klogd_work) = { void wake_up_klogd(void) { + if (!printk_percpu_data_ready()) + return; + preempt_disable(); if (waitqueue_active(&log_wait)) { this_cpu_or(printk_pending, PRINTK_PENDING_WAKEUP); @@ -2985,6 +3016,9 @@ void wake_up_klogd(void) void defer_console_output(void) { + if (!printk_percpu_data_ready()) + return; + preempt_disable(); __this_cpu_or(printk_pending, PRINTK_PENDING_OUTPUT); irq_work_queue(this_cpu_ptr(&wake_up_klogd_work)); diff --git a/kernel/printk/printk_safe.c b/kernel/printk/printk_safe.c index b4045e782743..d9a659a686f3 100644 --- a/kernel/printk/printk_safe.c +++ b/kernel/printk/printk_safe.c @@ -27,7 +27,6 @@ * There are situations when we want to make sure that all buffers * were handled or when IRQs are blocked. */ -static int printk_safe_irq_ready __read_mostly; #define SAFE_LOG_BUF_LEN ((1 << CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT) - \ sizeof(atomic_t) - \ @@ -51,7 +50,7 @@ static DEFINE_PER_CPU(struct printk_safe_seq_buf, nmi_print_seq); /* Get flushed in a more safe context. */ static void queue_flush_work(struct printk_safe_seq_buf *s) { - if (printk_safe_irq_ready) + if (printk_percpu_data_ready()) irq_work_queue(&s->work); } @@ -402,14 +401,6 @@ void __init printk_safe_init(void) #endif } - /* - * In the highly unlikely event that a NMI were to trigger at - * this moment. Make sure IRQ work is set up before this - * variable is set. - */ - barrier(); - printk_safe_irq_ready = 1; - /* Flush pending messages that did not have scheduled IRQ works. */ printk_safe_flush(); } -- cgit v1.2.3 From 32927393dc1ccd60fb2bdc05b9e8e88753761469 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Fri, 24 Apr 2020 08:43:38 +0200 Subject: sysctl: pass kernel pointers to ->proc_handler Instead of having all the sysctl handlers deal with user pointers, which is rather hairy in terms of the BPF interaction, copy the input to and from userspace in common code. This also means that the strings are always NUL-terminated by the common code, making the API a little bit safer. As most handler just pass through the data to one of the common handlers a lot of the changes are mechnical. Signed-off-by: Christoph Hellwig Acked-by: Andrey Ignatov Signed-off-by: Al Viro --- kernel/printk/printk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 9a9b6156270b..471f649b5868 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -173,7 +173,7 @@ __setup("printk.devkmsg=", control_devkmsg); char devkmsg_log_str[DEVKMSG_STR_MAX_SIZE] = "ratelimit"; int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write, - void __user *buffer, size_t *lenp, loff_t *ppos) + void *buffer, size_t *lenp, loff_t *ppos) { char old_str[DEVKMSG_STR_MAX_SIZE]; unsigned int old; -- cgit v1.2.3 From 8c4e93c362ff114def211d4629b120af86eb1275 Mon Sep 17 00:00:00 2001 From: Petr Mladek Date: Mon, 24 Feb 2020 13:13:31 +0100 Subject: printk: Prepare for nested printk_nmi_enter() There is plenty of space in the printk_context variable. Reserve one byte there for the NMI context to be on the safe side. It should never overflow. The BUG_ON(in_nmi() == NMI_MASK) in nmi_enter() will trigger much earlier. Signed-off-by: Petr Mladek Signed-off-by: Peter Zijlstra (Intel) Signed-off-by: Thomas Gleixner Reviewed-by: Alexandre Chartre Link: https://lkml.kernel.org/r/20200505134100.681374113@linutronix.de --- kernel/printk/internal.h | 8 +++++--- kernel/printk/printk_safe.c | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'kernel/printk') diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h index b2b0f526f249..660f9a6bf73a 100644 --- a/kernel/printk/internal.h +++ b/kernel/printk/internal.h @@ -6,9 +6,11 @@ #ifdef CONFIG_PRINTK -#define PRINTK_SAFE_CONTEXT_MASK 0x3fffffff -#define PRINTK_NMI_DIRECT_CONTEXT_MASK 0x40000000 -#define PRINTK_NMI_CONTEXT_MASK 0x80000000 +#define PRINTK_SAFE_CONTEXT_MASK 0x007ffffff +#define PRINTK_NMI_DIRECT_CONTEXT_MASK 0x008000000 +#define PRINTK_NMI_CONTEXT_MASK 0xff0000000 + +#define PRINTK_NMI_CONTEXT_OFFSET 0x010000000 extern raw_spinlock_t logbuf_lock; diff --git a/kernel/printk/printk_safe.c b/kernel/printk/printk_safe.c index d9a659a686f3..e8791f206417 100644 --- a/kernel/printk/printk_safe.c +++ b/kernel/printk/printk_safe.c @@ -295,12 +295,12 @@ static __printf(1, 0) int vprintk_nmi(const char *fmt, va_list args) void notrace printk_nmi_enter(void) { - this_cpu_or(printk_context, PRINTK_NMI_CONTEXT_MASK); + this_cpu_add(printk_context, PRINTK_NMI_CONTEXT_OFFSET); } void notrace printk_nmi_exit(void) { - this_cpu_and(printk_context, ~PRINTK_NMI_CONTEXT_MASK); + this_cpu_sub(printk_context, PRINTK_NMI_CONTEXT_OFFSET); } /* -- cgit v1.2.3 From b0f51883f551b900a04a80f49fb0886caf7e9a12 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Mon, 24 Feb 2020 22:25:03 +0100 Subject: printk: Disallow instrumenting print_nmi_enter() It happens early in nmi_enter(), no tracing, probing or other funnies allowed. Specifically as nmi_enter() will be used in do_debug(), which would cause recursive exceptions when kprobed. Signed-off-by: Peter Zijlstra (Intel) Signed-off-by: Thomas Gleixner Reviewed-by: Alexandre Chartre Link: https://lkml.kernel.org/r/20200505134101.139720912@linutronix.de --- kernel/printk/printk_safe.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk_safe.c b/kernel/printk/printk_safe.c index e8791f206417..4242403316bb 100644 --- a/kernel/printk/printk_safe.c +++ b/kernel/printk/printk_safe.c @@ -10,6 +10,7 @@ #include #include #include +#include #include "internal.h" @@ -293,12 +294,12 @@ static __printf(1, 0) int vprintk_nmi(const char *fmt, va_list args) return printk_safe_log_store(s, fmt, args); } -void notrace printk_nmi_enter(void) +void noinstr printk_nmi_enter(void) { this_cpu_add(printk_context, PRINTK_NMI_CONTEXT_OFFSET); } -void notrace printk_nmi_exit(void) +void noinstr printk_nmi_exit(void) { this_cpu_sub(printk_context, PRINTK_NMI_CONTEXT_OFFSET); } -- cgit v1.2.3 From 325606af573152e02f44d791f152b7f9564bcb30 Mon Sep 17 00:00:00 2001 From: Ethon Paul Date: Sat, 18 Apr 2020 19:35:36 +0800 Subject: printk: Fix a typo in comment "interator"->"iterator" There is a typo in comment, fix it. Signed-off-by: Ethon Paul Cc: Steven Rostedt Reviewed-by: Sergey Senozhatsky Signed-off-by: Petr Mladek --- kernel/printk/printk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 9a9b6156270b..525038745a14 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -3360,7 +3360,7 @@ out: EXPORT_SYMBOL_GPL(kmsg_dump_get_buffer); /** - * kmsg_dump_rewind_nolock - reset the interator (unlocked version) + * kmsg_dump_rewind_nolock - reset the iterator (unlocked version) * @dumper: registered kmsg dumper * * Reset the dumper's iterator so that kmsg_dump_get_line() and @@ -3378,7 +3378,7 @@ void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper) } /** - * kmsg_dump_rewind - reset the interator + * kmsg_dump_rewind - reset the iterator * @dumper: registered kmsg dumper * * Reset the dumper's iterator so that kmsg_dump_get_line() and -- cgit v1.2.3 From 8ece3b3eb576a78d2e67ad4c3a80a39fa6708809 Mon Sep 17 00:00:00 2001 From: Bruno Meneguele Date: Tue, 17 Mar 2020 07:33:44 -0300 Subject: kernel/printk: add kmsg SEEK_CUR handling Userspace libraries, e.g. glibc's dprintf(), perform a SEEK_CUR operation over any file descriptor requested to make sure the current position isn't pointing to junk due to previous manipulation of that same fd. And whenever that fd doesn't have support for such operation, the userspace code expects -ESPIPE to be returned. However, when the fd in question references the /dev/kmsg interface, the current kernel code state returns -EINVAL instead, causing an unexpected behavior in userspace: in the case of glibc, when -ESPIPE is returned it gets ignored and the call completes successfully, while returning -EINVAL forces dprintf to fail without performing any action over that fd: if (_IO_SEEKOFF (fp, (off64_t)0, _IO_seek_cur, _IOS_INPUT|_IOS_OUTPUT) == _IO_pos_BAD && errno != ESPIPE) return NULL; With this patch we make sure to return the correct value when SEEK_CUR is requested over kmsg and also add some kernel doc information to formalize this behavior. Link: https://lore.kernel.org/r/20200317103344.574277-1-bmeneg@redhat.com Cc: linux-kernel@vger.kernel.org Cc: rostedt@goodmis.org, Cc: David.Laight@ACULAB.COM Signed-off-by: Bruno Meneguele Acked-by: Sergey Senozhatsky Signed-off-by: Petr Mladek --- kernel/printk/printk.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 525038745a14..35cc5f548860 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -974,6 +974,16 @@ static loff_t devkmsg_llseek(struct file *file, loff_t offset, int whence) user->idx = log_next_idx; user->seq = log_next_seq; break; + case SEEK_CUR: + /* + * It isn't supported due to the record nature of this + * interface: _SET _DATA and _END point to very specific + * record positions, while _CUR would be more useful in case + * of a byte-based log. Because of that, return the default + * errno value for invalid seek operation. + */ + ret = -ESPIPE; + break; default: ret = -EINVAL; } -- cgit v1.2.3 From 48021f98130880dd74286459a1ef48b5e9bc374f Mon Sep 17 00:00:00 2001 From: Shreyas Joshi Date: Fri, 22 May 2020 16:53:06 +1000 Subject: printk: handle blank console arguments passed in. If uboot passes a blank string to console_setup then it results in a trashed memory. Ultimately, the kernel crashes during freeing up the memory. This fix checks if there is a blank parameter being passed to console_setup from uboot. In case it detects that the console parameter is blank then it doesn't setup the serial device and it gracefully exits. Link: https://lore.kernel.org/r/20200522065306.83-1-shreyas.joshi@biamp.com Signed-off-by: Shreyas Joshi Acked-by: Sergey Senozhatsky [pmladek@suse.com: Better format the commit message and code, remove unnecessary brackets.] Signed-off-by: Petr Mladek --- kernel/printk/printk.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 35cc5f548860..a3990505abdf 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2200,6 +2200,9 @@ static int __init console_setup(char *str) char *s, *options, *brl_options = NULL; int idx; + if (str[0] == 0) + return 1; + if (_braille_console_setup(&str, &brl_options)) return 1; -- cgit v1.2.3 From b1f6f161b236d0e5a9222fb8b482e65aaff13689 Mon Sep 17 00:00:00 2001 From: Pavel Tatashin Date: Tue, 5 May 2020 11:45:06 -0400 Subject: printk: honor the max_reason field in kmsg_dumper kmsg_dump() allows to dump kmesg buffer for various system events: oops, panic, reboot, etc. It provides an interface to register a callback call for clients, and in that callback interface there is a field "max_reason", but it was getting ignored when set to any "reason" higher than KMSG_DUMP_OOPS unless "always_kmsg_dump" was passed as kernel parameter. Allow clients to actually control their "max_reason", and keep the current behavior when "max_reason" is not set. Signed-off-by: Pavel Tatashin Link: https://lore.kernel.org/lkml/20200515184434.8470-3-keescook@chromium.org/ Reviewed-by: Petr Mladek Signed-off-by: Kees Cook --- kernel/printk/printk.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 9a9b6156270b..a121c2255737 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -3157,12 +3157,19 @@ void kmsg_dump(enum kmsg_dump_reason reason) struct kmsg_dumper *dumper; unsigned long flags; - if ((reason > KMSG_DUMP_OOPS) && !always_kmsg_dump) - return; - rcu_read_lock(); list_for_each_entry_rcu(dumper, &dump_list, list) { - if (dumper->max_reason && reason > dumper->max_reason) + enum kmsg_dump_reason max_reason = dumper->max_reason; + + /* + * If client has not provided a specific max_reason, default + * to KMSG_DUMP_OOPS, unless always_kmsg_dump was set. + */ + if (max_reason == KMSG_DUMP_UNDEF) { + max_reason = always_kmsg_dump ? KMSG_DUMP_MAX : + KMSG_DUMP_OOPS; + } + if (reason > max_reason) continue; /* initialize iterator with data about the stored records */ -- cgit v1.2.3 From fb13cb8a0482105a415e24042209d02a684255e2 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 7 May 2020 19:36:22 -0700 Subject: printk: Introduce kmsg_dump_reason_str() The pstore subsystem already had a private version of this function. With the coming addition of the pstore/zone driver, this needs to be shared. As it really should live with printk, move it there instead. Link: https://lore.kernel.org/lkml/20200515184434.8470-4-keescook@chromium.org/ Acked-by: Petr Mladek Acked-by: Sergey Senozhatsky Reviewed-by: Pavel Tatashin Signed-off-by: Kees Cook --- kernel/printk/printk.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index a121c2255737..14ca4d05d902 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -3144,6 +3144,23 @@ EXPORT_SYMBOL_GPL(kmsg_dump_unregister); static bool always_kmsg_dump; module_param_named(always_kmsg_dump, always_kmsg_dump, bool, S_IRUGO | S_IWUSR); +const char *kmsg_dump_reason_str(enum kmsg_dump_reason reason) +{ + switch (reason) { + case KMSG_DUMP_PANIC: + return "Panic"; + case KMSG_DUMP_OOPS: + return "Oops"; + case KMSG_DUMP_EMERG: + return "Emergency"; + case KMSG_DUMP_SHUTDOWN: + return "Shutdown"; + default: + return "Unknown"; + } +} +EXPORT_SYMBOL_GPL(kmsg_dump_reason_str); + /** * kmsg_dump - dump kernel log to kernel message dumpers. * @reason: the reason (oops, panic etc) for dumping -- cgit v1.2.3 From 2a9e5ded9543436620a7fbc9329ddcc32bf97bc7 Mon Sep 17 00:00:00 2001 From: Petr Mladek Date: Wed, 20 May 2020 12:22:33 +0200 Subject: printk/kdb: Redirect printk messages into kdb in any context kdb has to get messages on consoles even when the system is stopped. It uses kdb_printf() internally and calls console drivers on its own. It uses a hack to reuse an existing code. It sets "kdb_trap_printk" global variable to redirect even the normal printk() into the kdb_printf() variant. The variable "kdb_trap_printk" is checked in printk_default() and it is ignored when printk is redirected to printk_safe in NMI context. Solve this by moving the check into printk_func(). It is obvious that it is not fully safe. But it does not make things worse. The console drivers are already called in this context by db_printf() direct calls. Reported-by: Sumit Garg Tested-by: Sumit Garg Reviewed-by: Daniel Thompson Acked-by: Sergey Senozhatsky Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20200520102233.GC3464@linux-b0ei --- kernel/printk/printk.c | 14 +------------- kernel/printk/printk_safe.c | 7 +++++++ 2 files changed, 8 insertions(+), 13 deletions(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 9fdd6a42ad6a..2167bb528dd3 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -35,7 +35,6 @@ #include #include #include -#include #include #include #include @@ -2047,18 +2046,7 @@ EXPORT_SYMBOL(vprintk); int vprintk_default(const char *fmt, va_list args) { - int r; - -#ifdef CONFIG_KGDB_KDB - /* Allow to pass printk() to kdb but avoid a recursion. */ - if (unlikely(kdb_trap_printk && kdb_printf_cpu < 0)) { - r = vkdb_printf(KDB_MSGSRC_PRINTK, fmt, args); - return r; - } -#endif - r = vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, 0, fmt, args); - - return r; + return vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, 0, fmt, args); } EXPORT_SYMBOL_GPL(vprintk_default); diff --git a/kernel/printk/printk_safe.c b/kernel/printk/printk_safe.c index d9a659a686f3..7ccb821d0bfe 100644 --- a/kernel/printk/printk_safe.c +++ b/kernel/printk/printk_safe.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -359,6 +360,12 @@ void __printk_safe_exit(void) __printf(1, 0) int vprintk_func(const char *fmt, va_list args) { +#ifdef CONFIG_KGDB_KDB + /* Allow to pass printk() to kdb but avoid a recursion. */ + if (unlikely(kdb_trap_printk && kdb_printf_cpu < 0)) + return vkdb_printf(KDB_MSGSRC_PRINTK, fmt, args); +#endif + /* * Try to use the main logbuf even in NMI. But avoid calling console * drivers that might have their own locks. -- cgit v1.2.3 From 625d3449788f85569096780592549d0340e9c0c7 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Sun, 21 Jun 2020 21:02:22 -0600 Subject: Revert "kernel/printk: add kmsg SEEK_CUR handling" This reverts commit 8ece3b3eb576a78d2e67ad4c3a80a39fa6708809. This commit broke userspace. Bash uses ESPIPE to determine whether or not the file should be read using "unbuffered I/O", which means reading 1 byte at a time instead of 128 bytes at a time. I used to use bash to read through kmsg in a really quite nasty way: while read -t 0.1 -r line 2>/dev/null || [[ $? -ne 142 ]]; do echo "SARU $line" done < /dev/kmsg This will show all lines that can fit into the 128 byte buffer, and skip lines that don't. That's pretty awful, but at least it worked. With this change, bash now tries to do 1-byte reads, which means it skips all the lines, which is worse than before. Now, I don't really care very much about this, and I'm already look for a workaround. But I did just spend an hour trying to figure out why my scripts were broken. Either way, it makes no difference to me personally whether this is reverted, but it might be something to consider. If you declare that "trying to read /dev/kmsg with bash is terminally stupid anyway," I might be inclined to agree with you. But do note that bash uses lseek(fd, 0, SEEK_CUR)==>ESPIPE to determine whether or not it's reading from a pipe. Cc: Bruno Meneguele Cc: Sergey Senozhatsky Cc: Steven Rostedt Cc: David Laight Cc: Sergey Senozhatsky Cc: Petr Mladek Signed-off-by: Jason A. Donenfeld Signed-off-by: Linus Torvalds --- kernel/printk/printk.c | 10 ---------- 1 file changed, 10 deletions(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 8c14835be46c..b71eaf5f5a86 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -974,16 +974,6 @@ static loff_t devkmsg_llseek(struct file *file, loff_t offset, int whence) user->idx = log_next_idx; user->seq = log_next_seq; break; - case SEEK_CUR: - /* - * It isn't supported due to the record nature of this - * interface: _SET _DATA and _END point to very specific - * record positions, while _CUR would be more useful in case - * of a byte-based log. Because of that, return the default - * errno value for invalid seek operation. - */ - ret = -ESPIPE; - break; default: ret = -EINVAL; } -- cgit v1.2.3 From bba18a1af33e0f0a1ad7d028208b306ef3f3df12 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 18 Jun 2020 19:47:50 +0300 Subject: console: Propagate error code from console ->setup() Since console ->setup() hook returns meaningful error codes, propagate it to the caller of try_enable_new_console(). Signed-off-by: Andy Shevchenko Reviewed-by: Petr Mladek Acked-by: Benjamin Herrenschmidt Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20200618164751.56828-6-andriy.shevchenko@linux.intel.com --- kernel/printk/printk.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 8c14835be46c..aaea3ad182e1 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2668,7 +2668,7 @@ early_param("keep_bootcon", keep_bootcon_setup); static int try_enable_new_console(struct console *newcon, bool user_specified) { struct console_cmdline *c; - int i; + int i, err; for (i = 0, c = console_cmdline; i < MAX_CMDLINECONSOLES && c->name[0]; @@ -2691,8 +2691,8 @@ static int try_enable_new_console(struct console *newcon, bool user_specified) return 0; if (newcon->setup && - newcon->setup(newcon, c->options) != 0) - return -EIO; + (err = newcon->setup(newcon, c->options)) != 0) + return err; } newcon->flags |= CON_ENABLED; if (i == preferred_console) { -- cgit v1.2.3 From 504603767ab639c47912be08d62e02b43125e82e Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 18 Jun 2020 19:47:51 +0300 Subject: console: Fix trivia typo 'change' -> 'chance' I bet the word 'chance' has to be used in 'had a chance to be called', but, alas, I'm not native speaker... Signed-off-by: Andy Shevchenko Reviewed-by: Petr Mladek Acked-by: Benjamin Herrenschmidt Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20200618164751.56828-7-andriy.shevchenko@linux.intel.com --- kernel/printk/printk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index aaea3ad182e1..6623e975675a 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2705,7 +2705,7 @@ static int try_enable_new_console(struct console *newcon, bool user_specified) /* * Some consoles, such as pstore and netconsole, can be enabled even * without matching. Accept the pre-enabled consoles only when match() - * and setup() had a change to be called. + * and setup() had a chance to be called. */ if (newcon->flags & CON_ENABLED && c->user_specified == user_specified) return 0; -- cgit v1.2.3 From bc885f1ab6de0d38c6956a71b0126543b64875b0 Mon Sep 17 00:00:00 2001 From: Bruno Meneguele Date: Fri, 10 Jul 2020 14:44:23 -0300 Subject: doc:kmsg: explicitly state the return value in case of SEEK_CUR The commit 625d3449788f ("Revert "kernel/printk: add kmsg SEEK_CUR handling"") reverted a change done to the return value in case a SEEK_CUR operation was performed for kmsg buffer based on the fact that different userspace apps were handling the new return value (-ESPIPE) in different ways, breaking them. At the same time -ESPIPE was the wrong decision because kmsg /does support/ seek() but doesn't follow the "normal" behavior userspace is used to. Because of that and also considering the time -EINVAL has been used, it was decided to keep this way to avoid more userspace breakage. This patch adds an official statement to the kmsg documentation pointing to the current return value for SEEK_CUR, -EINVAL, thus userspace libraries and apps can refer to it for a definitive guide on what to expect. Signed-off-by: Bruno Meneguele Reviewed-by: Sergey Senozhatsky Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20200710174423.10480-1-bmeneg@redhat.com --- kernel/printk/printk.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'kernel/printk') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 8c14835be46c..5f6eca65dd9a 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -943,6 +943,14 @@ out: return ret; } +/* + * Be careful when modifying this function!!! + * + * Only few operations are supported because the device works only with the + * entire variable length messages (records). Non-standard values are + * returned in the other cases and has been this way for quite some time. + * User space applications might depend on this behavior. + */ static loff_t devkmsg_llseek(struct file *file, loff_t offset, int whence) { struct devkmsg_user *user = file->private_data; -- cgit v1.2.3