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

[Nickolai Zeldovich <nickolai@csail.mit.edu>]

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")
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.
    
 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);
 
 	if (!buf) {
 		panic("Failed to allocate taint string buffer");

base-commit: 26344c0dd32af8790d8474d233619db825441138
-- 
2.53.0