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

[Petr Mladek <pmladek@suse.com>]

On Fri 2026-10-23 22:01:52, Nickolai Zeldovich 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")
> 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.

Great catch and explanation.

The taint flags are only added and never cleared so the printed
string might only grow. The racy printing is acceptable. The last
backtrace should show the full string.

The racy string manipulations are not safe when the shared buffer is not zeroed
at the beginning. The use of kzalloc() seems to be the right fix.

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr