Re: [PATCH v1] panic: zero the dynamically allocated taint buffer

[Bradley Morgan <brads@mainlining.org>]

On 24 October 2026 03:01:52 BST, Nickolai Zeldovich
<nickolai@csail.mit.edu> wrote:
>A %s read of print_tainted()'s string can run past the end of the taint
>buffer while another CPU rebuilds a longer string in it, reading slab
>memory out of bounds into the kernel log (a KASAN slab-out-of-bounds
>report at taint_buf + size). A warning or an unhandled-signal dump on
>another CPU reaches print_tainted() through dump_stack_print_info().
>
>alloc_taint_buf() sizes the buffer for the longest taint string and
>allocates it with kmalloc(), so the bytes past that string are never
>written and the object holds no permanent NUL terminator. _print_tainted()
>rebuilds the string in place without locking and writes the terminator
>last, so a reader that overtakes a writer extending the string past the
>previous high-water mark finds no terminator inside the object and walks
>off its end. print_tainted()'s kernel-doc promises the string is "always
>NULL terminated", a guarantee the previous zero-filled static buffer kept.
>
>Allocate the buffer with kzalloc() so its trailing bytes stay NUL and
>every reader stops inside the object.
>
>Reproduced on QEMU riscv64 under KASAN by growing the taint set while
>processes took unhandled-signal dumps on several CPUs: the unpatched
>kernel reports a slab-out-of-bounds read at taint_buf + size, the patched
>one is clean.
>
>Fixes: a9dff0d0d11c ("kernel/panic: allocate taint string buffer dynamically")

Hi, let's review this code

>Signed-off-by: Nickolai Zeldovich <nickolai@csail.mit.edu>
>---
>
>Notes:
>    This is a race on the shared taint_buf: the fix removes the memory-safety
>    consequence (a buffer with no terminator inside it), but two concurrent
>    callers can still garble each other's "Tainted:" string. That is accepted
>    behaviour -- print_tainted()'s kernel-doc already says the string is
>    overwritten by the next call.
>    
>    taint_buf and taint_buf_size are published by plain stores in
>    alloc_taint_buf(). A hart racing the postcore_initcall could in principle
>    see the new pointer before the kzalloc'd contents, or see the torn
>    (pointer, size) pair; both are harmless now (every writer stops within the
>    smaller bound and the object is zeroed), so this patch does not add an
>    smp_store_release(). It can be added if that boot-time window is ever
>    tightened.
>    
>    Reproduced under KASAN on QEMU riscv64 (-cpu max -smp 4, slub_debug=P). The
>    window between the body and the terminating NUL in _print_tainted() is
>    narrow, so to hit it on demand a test-only mdelay() was inserted there (it
>    is not part of the patch). As root, grow the taint set while several
>    processes take unhandled-signal dumps on different CPUs:
>    
>    Reproducer (needs CONFIG_KASAN and a test-only mdelay() in _print_tainted()):
>    
>      #define _GNU_SOURCE
>      #include <fcntl.h>
>      #include <sched.h>
>      #include <string.h>
>      #include <unistd.h>
>      #include <stdint.h>
>      #include <sys/wait.h>
>    
>      static void grow_taint(void)
>      {
>      	int fd = open("/proc/sys/kernel/tainted", O_WRONLY);
>      	if (fd >= 0) { write(fd, "2097151\n", 8); close(fd); }
>      }
>    
>      static void crasher(int cpu)
>      {
>      	cpu_set_t s; CPU_ZERO(&s); CPU_SET(cpu, &s);
>      	sched_setaffinity(0, sizeof(s), &s);
>      	for (;;) {
>      		if (fork() == 0) {
>      			*(volatile long *)(uintptr_t)0xdeadbeef000UL = 1; /* SIGSEGV */
>      			_exit(0);
>      		}
>      		wait(0);
>      	}
>      }
>    
>      int main(void)
>      {
>      	long n = sysconf(_SC_NPROCESSORS_ONLN);
>      	grow_taint();
>      	for (long i = 0; i < n; i++)
>      		if (fork() == 0) { crasher(i); _exit(0); }
>      	for (int k = 0; k < 120; k++) { grow_taint(); usleep(50000); }
>      	return 0;
>      }
>    
>    Unpatched: a flood of
>    
>      BUG: KASAN: slab-out-of-bounds in string+0x..
>      Read of size 1 ... 0 bytes to the right of the 347-byte region
>      allocated by alloc_taint_buf(), from dump_stack_print_info() ->
>      vsnprintf() in a do_trap() dump.
>    
>    Patched: the same dumps run, no KASAN report.
>
>    Developed with the assistance of an LLM.

Thanx for the declaration.

>    
> kernel/panic.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/kernel/panic.c b/kernel/panic.c
>index a4dbb6b8cd2bf..f23f6bc38ab41 100644
>--- a/kernel/panic.c
>+++ b/kernel/panic.c
>@@ -886,7 +886,7 @@ static __init int alloc_taint_buf(void)
> 
> 	size += 1; /* For NULL terminator */
> 
>-	buf = kmalloc(size, GFP_KERNEL);
>+	buf = kzalloc(size, GFP_KERNEL);

yeah, makes sense. Thanks for fixing the bug.

Reviewed-by: Bradley Morgan <brads@mainlining.org>

Andrew: Perhaps wait for Petrs opinion on this patch too?

> 
> 	if (!buf) {
> 		panic("Failed to allocate taint string buffer");
>
>base-commit: 26344c0dd32af8790d8474d233619db825441138
>

--- Thanks!