From 3d6f83df8ff2d5de84b50377e4f0d45e25311c7a Mon Sep 17 00:00:00 2001 From: Kuan-Wei Chiu Date: Sat, 28 Sep 2024 19:36:08 +0800 Subject: [PATCH 1/3] printk: Fix signed integer overflow when defining LOG_BUF_LEN_MAX Shifting 1 << 31 on a 32-bit int causes signed integer overflow, which leads to undefined behavior. To prevent this, cast 1 to u32 before performing the shift, ensuring well-defined behavior. This change explicitly avoids any potential overflow by ensuring that the shift occurs on an unsigned 32-bit integer. Signed-off-by: Kuan-Wei Chiu Acked-by: Petr Mladek Link: https://lore.kernel.org/r/20240928113608.1438087-1-visitorckw@gmail.com Signed-off-by: Petr Mladek --- 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 80910bc3470c..d8d82dd39dee 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -523,7 +523,7 @@ static struct latched_seq clear_seq = { /* record buffer */ #define LOG_ALIGN __alignof__(unsigned long) #define __LOG_BUF_LEN (1 << CONFIG_LOG_BUF_SHIFT) -#define LOG_BUF_LEN_MAX (u32)(1 << 31) +#define LOG_BUF_LEN_MAX ((u32)1 << 31) static char __log_buf[__LOG_BUF_LEN] __aligned(LOG_ALIGN); static char *log_buf = __log_buf; static u32 log_buf_len = __LOG_BUF_LEN; From f1c21cf470595c4561d4671fd499af94152175d5 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Mon, 9 Dec 2024 12:23:45 +0106 Subject: [PATCH 2/3] printk: Remove redundant deferred check in vprintk() The helper printk_get_console_flush_type() is already calling is_printk_legacy_deferred() to determine if legacy printing is to be offloaded. Therefore there is no need for vprintk() to perform this check as well. Remove the redundant check from vprintk(). Signed-off-by: John Ogness Reviewed-by: Petr Mladek Link: https://lore.kernel.org/r/20241209111746.192559-2-john.ogness@linutronix.de Signed-off-by: Petr Mladek --- kernel/printk/printk_safe.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/kernel/printk/printk_safe.c b/kernel/printk/printk_safe.c index 6f94418d53ff..6283bc0b55e6 100644 --- a/kernel/printk/printk_safe.c +++ b/kernel/printk/printk_safe.c @@ -74,15 +74,6 @@ asmlinkage int vprintk(const char *fmt, va_list args) if (unlikely(kdb_trap_printk && kdb_printf_cpu < 0)) return vkdb_printf(KDB_MSGSRC_PRINTK, fmt, args); #endif - - /* - * Use the main logbuf even in NMI. But avoid calling console - * drivers that might have their own locks. - */ - if (is_printk_legacy_deferred()) - return vprintk_deferred(fmt, args); - - /* No obstacles. */ return vprintk_default(fmt, args); } EXPORT_SYMBOL(vprintk); From 0161e2d6950fe66cf6ac1c10d945bae971f33667 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Mon, 9 Dec 2024 12:23:46 +0106 Subject: [PATCH 3/3] printk: Defer legacy printing when holding printk_cpu_sync The documentation of printk_cpu_sync_get() clearly states that the owner must never perform any activities where it waits for a CPU. For legacy printing there can be spinning on the console_lock and on the port lock. Therefore legacy printing must be deferred when holding the printk_cpu_sync. Note that in the case of emergency states, atomic consoles are not prevented from printing when printk is deferred. This is appropriate because they do not spin-wait indefinitely for other CPUs. Reported-by: Rik van Riel Closes: https://lore.kernel.org/r/20240715232052.73eb7fb1@imladris.surriel.com Signed-off-by: John Ogness Fixes: 55d6af1d6688 ("lib/nmi_backtrace: explicitly serialize banner and regs") Reviewed-by: Petr Mladek Link: https://lore.kernel.org/r/20241209111746.192559-3-john.ogness@linutronix.de Signed-off-by: Petr Mladek --- kernel/printk/internal.h | 6 ++++++ kernel/printk/printk.c | 5 +++++ kernel/printk/printk_safe.c | 7 ++++++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h index c6bb47666aef..a91bdf802967 100644 --- a/kernel/printk/internal.h +++ b/kernel/printk/internal.h @@ -338,3 +338,9 @@ bool printk_get_next_message(struct printk_message *pmsg, u64 seq, void console_prepend_dropped(struct printk_message *pmsg, unsigned long dropped); void console_prepend_replay(struct printk_message *pmsg); #endif + +#ifdef CONFIG_SMP +bool is_printk_cpu_sync_owner(void); +#else +static inline bool is_printk_cpu_sync_owner(void) { return false; } +#endif diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 80910bc3470c..f446a06b4da8 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -4922,6 +4922,11 @@ void console_try_replay_all(void) static atomic_t printk_cpu_sync_owner = ATOMIC_INIT(-1); static atomic_t printk_cpu_sync_nested = ATOMIC_INIT(0); +bool is_printk_cpu_sync_owner(void) +{ + return (atomic_read(&printk_cpu_sync_owner) == raw_smp_processor_id()); +} + /** * __printk_cpu_sync_wait() - Busy wait until the printk cpu-reentrant * spinning lock is not owned by any CPU. diff --git a/kernel/printk/printk_safe.c b/kernel/printk/printk_safe.c index 6283bc0b55e6..32a28f563b13 100644 --- a/kernel/printk/printk_safe.c +++ b/kernel/printk/printk_safe.c @@ -61,10 +61,15 @@ bool is_printk_legacy_deferred(void) /* * The per-CPU variable @printk_context can be read safely in any * context. CPU migration is always disabled when set. + * + * A context holding the printk_cpu_sync must not spin waiting for + * another CPU. For legacy printing, it could be the console_lock + * or the port lock. */ return (force_legacy_kthread() || this_cpu_read(printk_context) || - in_nmi()); + in_nmi() || + is_printk_cpu_sync_owner()); } asmlinkage int vprintk(const char *fmt, va_list args)