From dc453dd89daacdc0da6d66234aa27e417df7edcd Mon Sep 17 00:00:00 2001 From: Jian Shen Date: Tue, 16 Aug 2022 22:45:57 +0800 Subject: [PATCH 1/8] lib/vnsprintf: add const modifier for param 'bitmap' There is no modification for param bitmap in function bitmap_string() and bitmap_list_string(), so add const modifier for it. Signed-off-by: Jian Shen Signed-off-by: Guangbin Huang Reviewed-by: Steven Rostedt (Google) Reviewed-by: Sergey Senozhatsky Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220816144557.30779-1-huangguangbin2@huawei.com --- lib/vsprintf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 3c1853a9d1c0..07b36d8b29c3 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -1189,7 +1189,7 @@ char *hex_string(char *buf, char *end, u8 *addr, struct printf_spec spec, } static noinline_for_stack -char *bitmap_string(char *buf, char *end, unsigned long *bitmap, +char *bitmap_string(char *buf, char *end, const unsigned long *bitmap, struct printf_spec spec, const char *fmt) { const int CHUNKSZ = 32; @@ -1233,7 +1233,7 @@ char *bitmap_string(char *buf, char *end, unsigned long *bitmap, } static noinline_for_stack -char *bitmap_list_string(char *buf, char *end, unsigned long *bitmap, +char *bitmap_list_string(char *buf, char *end, const unsigned long *bitmap, struct printf_spec spec, const char *fmt) { int nr_bits = max_t(int, spec.field_width, 0); From e4279b599863dd1aa71fb8e35bffa943545bbaeb Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Tue, 27 Sep 2022 12:49:11 +0200 Subject: [PATCH 2/8] lib/vsprintf: Remove static_branch_likely() from __ptr_to_hashval(). Using static_branch_likely() to signal that ptr_key has been filled is a bit much given that it is not a fast path. Replace static_branch_likely() with bool for condition and a memory barrier for ptr_key. Suggested-by: Petr Mladek Signed-off-by: Sebastian Andrzej Siewior Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220927104912.622645-2-bigeasy@linutronix.de --- lib/vsprintf.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 3c1853a9d1c0..bce63cbf2377 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -750,12 +750,7 @@ static int __init debug_boot_weak_hash_enable(char *str) } early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable); -static DEFINE_STATIC_KEY_FALSE(filled_random_ptr_key); - -static void enable_ptr_key_workfn(struct work_struct *work) -{ - static_branch_enable(&filled_random_ptr_key); -} +static bool filled_random_ptr_key __read_mostly; /* Maps a pointer to a 32 bit unique identifier. */ static inline int __ptr_to_hashval(const void *ptr, unsigned long *hashval_out) @@ -763,24 +758,26 @@ static inline int __ptr_to_hashval(const void *ptr, unsigned long *hashval_out) static siphash_key_t ptr_key __read_mostly; unsigned long hashval; - if (!static_branch_likely(&filled_random_ptr_key)) { + if (!READ_ONCE(filled_random_ptr_key)) { static bool filled = false; static DEFINE_SPINLOCK(filling); - static DECLARE_WORK(enable_ptr_key_work, enable_ptr_key_workfn); unsigned long flags; - if (!system_unbound_wq || !rng_is_initialized() || + if (!rng_is_initialized() || !spin_trylock_irqsave(&filling, flags)) return -EAGAIN; if (!filled) { get_random_bytes(&ptr_key, sizeof(ptr_key)); - queue_work(system_unbound_wq, &enable_ptr_key_work); + /* Pairs with smp_rmb() before reading ptr_key. */ + smp_wmb(); + WRITE_ONCE(filled_random_ptr_key, true); filled = true; } spin_unlock_irqrestore(&filling, flags); } - + /* Pairs with smp_wmb() after writing ptr_key. */ + smp_rmb(); #ifdef CONFIG_64BIT hashval = (unsigned long)siphash_1u64((u64)ptr, &ptr_key); From 6f0ac3b52a9075b7291a72fb338d08491c1f0a64 Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Tue, 27 Sep 2022 12:49:12 +0200 Subject: [PATCH 3/8] lib/vsprintf: Initialize vsprintf's pointer hash once the random core is ready. The printk code invokes vnsprintf in order to compute the complete string before adding it into its buffer. This happens in an IRQ-off region which leads to a warning on PREEMPT_RT in the random code if the format strings contains a %p for pointer printing. This happens because the random core acquires locks which become sleeping locks on PREEMPT_RT which must not be acquired with disabled interrupts and or preemption disabled. By default the pointers are hashed which requires a random value on the first invocation (either by printk or another user which comes first. One could argue that there is no need for printk to disable interrupts during the vsprintf() invocation which would fix the just mentioned problem. However printk itself can be invoked in a context with disabled interrupts which would lead to the very same problem. Move the initialization of ptr_key into a worker and schedule it from subsys_initcall(). This happens early but after the workqueue subsystem is ready. Use get_random_bytes() to retrieve the random value if the RNG core is ready, otherwise schedule a worker in two seconds and try again. Another advantage is that it removes a lock from the vsprintf() code path. It prevents a possible deadlock when printk("%p", ptr) is called under the lock taken in get_random_bytes(). Reported-by: Mike Galbraith Signed-off-by: Sebastian Andrzej Siewior Acked-by: Jason A. Donenfeld Reviewed-by: Petr Mladek [pmladek@suse.com: Added a note about the it prevented a possible deadlock in printk().] Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220927104912.622645-3-bigeasy@linutronix.de --- lib/vsprintf.c | 44 ++++++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/lib/vsprintf.c b/lib/vsprintf.c index bce63cbf2377..44b39ba56b79 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -751,31 +751,39 @@ static int __init debug_boot_weak_hash_enable(char *str) early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable); static bool filled_random_ptr_key __read_mostly; +static siphash_key_t ptr_key __read_mostly; +static void fill_ptr_key_workfn(struct work_struct *work); +static DECLARE_DELAYED_WORK(fill_ptr_key_work, fill_ptr_key_workfn); + +static void fill_ptr_key_workfn(struct work_struct *work) +{ + if (!rng_is_initialized()) { + queue_delayed_work(system_unbound_wq, &fill_ptr_key_work, HZ * 2); + return; + } + + get_random_bytes(&ptr_key, sizeof(ptr_key)); + + /* Pairs with smp_rmb() before reading ptr_key. */ + smp_wmb(); + WRITE_ONCE(filled_random_ptr_key, true); +} + +static int __init vsprintf_init_hashval(void) +{ + fill_ptr_key_workfn(NULL); + return 0; +} +subsys_initcall(vsprintf_init_hashval) /* Maps a pointer to a 32 bit unique identifier. */ static inline int __ptr_to_hashval(const void *ptr, unsigned long *hashval_out) { - static siphash_key_t ptr_key __read_mostly; unsigned long hashval; - if (!READ_ONCE(filled_random_ptr_key)) { - static bool filled = false; - static DEFINE_SPINLOCK(filling); - unsigned long flags; + if (!READ_ONCE(filled_random_ptr_key)) + return -EBUSY; - if (!rng_is_initialized() || - !spin_trylock_irqsave(&filling, flags)) - return -EAGAIN; - - if (!filled) { - get_random_bytes(&ptr_key, sizeof(ptr_key)); - /* Pairs with smp_rmb() before reading ptr_key. */ - smp_wmb(); - WRITE_ONCE(filled_random_ptr_key, true); - filled = true; - } - spin_unlock_irqrestore(&filling, flags); - } /* Pairs with smp_wmb() after writing ptr_key. */ smp_rmb(); From c60ba2d34608dc6e224440267ed392adf65e05f2 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sat, 24 Sep 2022 02:10:37 +0206 Subject: [PATCH 4/8] printk: Make pr_flush() static No user outside the printk code and no reason to export this. Signed-off-by: Thomas Gleixner Signed-off-by: John Ogness Reviewed-by: Sergey Senozhatsky Reviewed-by: Petr Mladek Reviewed-by: Greg Kroah-Hartman Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220924000454.3319186-2-john.ogness@linutronix.de --- include/linux/printk.h | 7 ------- kernel/printk/printk.c | 5 +++-- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/include/linux/printk.h b/include/linux/printk.h index 091fba7283e1..b70a42f94031 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -170,8 +170,6 @@ extern void __printk_safe_exit(void); #define printk_deferred_enter __printk_safe_enter #define printk_deferred_exit __printk_safe_exit -extern bool pr_flush(int timeout_ms, bool reset_on_progress); - /* * Please don't use printk_ratelimit(), because it shares ratelimiting state * with all other unrelated printk_ratelimit() callsites. Instead use @@ -222,11 +220,6 @@ static inline void printk_deferred_exit(void) { } -static inline bool pr_flush(int timeout_ms, bool reset_on_progress) -{ - return true; -} - static inline int printk_ratelimit(void) { return 0; diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index a1a81fd9889b..14d7d39d118d 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2296,6 +2296,7 @@ asmlinkage __visible int _printk(const char *fmt, ...) } EXPORT_SYMBOL(_printk); +static bool pr_flush(int timeout_ms, bool reset_on_progress); static bool __pr_flush(struct console *con, int timeout_ms, bool reset_on_progress); #else /* CONFIG_PRINTK */ @@ -2330,6 +2331,7 @@ static void call_console_driver(struct console *con, const char *text, size_t le { } static bool suppress_message_printing(int level) { return false; } +static bool pr_flush(int timeout_ms, bool reset_on_progress) { return true; } static bool __pr_flush(struct console *con, int timeout_ms, bool reset_on_progress) { return true; } #endif /* CONFIG_PRINTK */ @@ -3438,11 +3440,10 @@ static bool __pr_flush(struct console *con, int timeout_ms, bool reset_on_progre * Context: Process context. May sleep while acquiring console lock. * Return: true if all enabled printers are caught up. */ -bool pr_flush(int timeout_ms, bool reset_on_progress) +static bool pr_flush(int timeout_ms, bool reset_on_progress) { return __pr_flush(NULL, timeout_ms, reset_on_progress); } -EXPORT_SYMBOL(pr_flush); /* * Delayed printk version, for scheduler-internal messages: From e3f12f0602833c88ad2cbe3aff397acd0cfd2695 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sat, 24 Sep 2022 02:10:38 +0206 Subject: [PATCH 5/8] printk: Declare log_wait properly kernel/printk/printk.c:365:1: warning: symbol 'log_wait' was not declared. Should it be static? Signed-off-by: Thomas Gleixner Signed-off-by: John Ogness Reviewed-by: Sergey Senozhatsky Reviewed-by: Petr Mladek Reviewed-by: Greg Kroah-Hartman Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220924000454.3319186-3-john.ogness@linutronix.de --- fs/proc/kmsg.c | 2 -- include/linux/syslog.h | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/proc/kmsg.c b/fs/proc/kmsg.c index b38ad552887f..9d6950ac10fe 100644 --- a/fs/proc/kmsg.c +++ b/fs/proc/kmsg.c @@ -18,8 +18,6 @@ #include #include -extern wait_queue_head_t log_wait; - static int kmsg_open(struct inode * inode, struct file * file) { return do_syslog(SYSLOG_ACTION_OPEN, NULL, 0, SYSLOG_FROM_PROC); diff --git a/include/linux/syslog.h b/include/linux/syslog.h index 86af908e2663..955f80e34d4f 100644 --- a/include/linux/syslog.h +++ b/include/linux/syslog.h @@ -8,6 +8,8 @@ #ifndef _LINUX_SYSLOG_H #define _LINUX_SYSLOG_H +#include + /* Close the log. Currently a NOP. */ #define SYSLOG_ACTION_CLOSE 0 /* Open the log. Currently a NOP. */ @@ -35,5 +37,6 @@ #define SYSLOG_FROM_PROC 1 int do_syslog(int type, char __user *buf, int count, int source); +extern wait_queue_head_t log_wait; #endif /* _LINUX_SYSLOG_H */ From 7fc11a521e7c1b8cec5904b28dba9b85186f37d7 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sat, 24 Sep 2022 02:10:39 +0206 Subject: [PATCH 6/8] printk: Remove write only variable nr_ext_console_drivers Commit a699449bb13b ("printk: refactor and rework printing logic") removed the need for @nr_ext_console_drivers. Remove the unneeded variable. Signed-off-by: Thomas Gleixner Signed-off-by: John Ogness Reviewed-by: Sergey Senozhatsky Reviewed-by: Petr Mladek Reviewed-by: Greg Kroah-Hartman Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220924000454.3319186-4-john.ogness@linutronix.de --- kernel/printk/printk.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 14d7d39d118d..d6bba2ea14e8 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -220,9 +220,6 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write, } #endif /* CONFIG_PRINTK && CONFIG_SYSCTL */ -/* Number of registered extended console drivers. */ -static int nr_ext_console_drivers; - /* * Helper macros to handle lockdep when locking/unlocking console_sem. We use * macros instead of functions so that _RET_IP_ contains useful information. @@ -3188,9 +3185,6 @@ void register_console(struct console *newcon) console_drivers->next = newcon; } - if (newcon->flags & CON_EXTENDED) - nr_ext_console_drivers++; - newcon->dropped = 0; if (newcon->flags & CON_PRINTBUFFER) { /* Get a consistent copy of @syslog_seq. */ @@ -3256,9 +3250,6 @@ int unregister_console(struct console *console) if (res) goto out_disable_unlock; - if (console->flags & CON_EXTENDED) - nr_ext_console_drivers--; - /* * If this isn't the last console and it has CON_CONSDEV set, we * need to set it on the next preferred console. From eb4531b346c93fd01edc0cb155330bbee102d0bd Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sat, 24 Sep 2022 02:10:40 +0206 Subject: [PATCH 7/8] printk: Remove bogus comment vs. boot consoles The comment about unregistering boot consoles is just not matching the reality. Remove it. Signed-off-by: Thomas Gleixner Signed-off-by: John Ogness Reviewed-by: Sergey Senozhatsky Reviewed-by: Petr Mladek Reviewed-by: Greg Kroah-Hartman Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220924000454.3319186-5-john.ogness@linutronix.de --- kernel/printk/printk.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index d6bba2ea14e8..770511b89504 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -3209,9 +3209,6 @@ void register_console(struct console *newcon) if (bootcon_enabled && ((newcon->flags & (CON_CONSDEV | CON_BOOT)) == CON_CONSDEV) && !keep_bootcon) { - /* We need to iterate through all boot consoles, to make - * sure we print everything out, before we unregister them. - */ for_each_console(con) if (con->flags & CON_BOOT) unregister_console(con); From 78ba392c84c74903b979c1ef5ded93430acd9ad6 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sat, 24 Sep 2022 02:10:41 +0206 Subject: [PATCH 8/8] printk: Mark __printk percpu data ready __ro_after_init This variable cannot change post boot. Signed-off-by: Thomas Gleixner Signed-off-by: John Ogness Reviewed-by: Sergey Senozhatsky Reviewed-by: Petr Mladek Reviewed-by: Greg Kroah-Hartman Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220924000454.3319186-6-john.ogness@linutronix.de --- kernel/printk/printk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 770511b89504..e4f1e7478b52 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -430,7 +430,7 @@ static struct printk_ringbuffer *prb = &printk_rb_static; * 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; +static bool __printk_percpu_data_ready __ro_after_init; bool printk_percpu_data_ready(void) {