Hi Woradorn,
Thanks for this cleanup — replacing strlcat with seq_buf is a nice
improvement in general, and it's good to see the KSPP goal of removing
strlcat making progress.
I came across an automated AI code review of this patch on the Sashiko
platform[1] that flagged a regression risk, and I was able to verify it
by testing the patched kernel in QEMU.
TL;DR: This patch introduces an unavoidable WARN_ON on every normal
boot (without the trace_event= parameter), which will crash systems
configured with panic_on_warn.
> @@ -4501,13 +4502,23 @@ extern struct trace_event_call
*__start_ftrace_events[];
> extern struct trace_event_call *__stop_ftrace_events[];
>
> static char bootup_event_buf[COMMAND_LINE_SIZE] __initdata;
> +static struct seq_buf bootup_event_seq;
> +static bool bootup_event_seq_initialized;
The new seq_buf and the bool are not annotated __initdata, unlike
bootup_event_buf. If bootup_event_buf is reclaimed after init, the
seq_buf will hold a dangling pointer to freed memory indefinitely.
> static __init int setup_trace_event(char *str)
> {
> - if (bootup_event_buf[0] != '\0')
> - strlcat(bootup_event_buf, ",", COMMAND_LINE_SIZE);
> + if (!bootup_event_seq_initialized) {
> + seq_buf_init(&bootup_event_seq, bootup_event_buf,
COMMAND_LINE_SIZE);
> + bootup_event_seq_initialized = true;
> + }
> +
> + if (seq_buf_used(&bootup_event_seq) > 0)
> + seq_buf_puts(&bootup_event_seq, ",");
>
> - strlcat(bootup_event_buf, str, COMMAND_LINE_SIZE);
> + seq_buf_puts(&bootup_event_seq, str);
> @@ -4766,7 +4777,7 @@ static __init int event_trace_enable(void)
> - early_enable_events(tr, bootup_event_buf, false);
> + early_enable_events(tr, (char *)seq_buf_str(&bootup_event_seq),
false);
> @@ -4794,7 +4805,7 @@ static __init int event_trace_enable_again(void)
> - early_enable_events(tr, bootup_event_buf, true);
> + early_enable_events(tr, (char *)seq_buf_str(&bootup_event_seq),
true);
This is the main issue: both event_trace_enable() and
event_trace_enable_again() unconditionally call seq_buf_str() on the
still-zero-initialized seq_buf. When booting without trace_event= on
the kernel command line, setup_trace_event() never runs, so
bootup_event_seq remains zero-initialized with size==0. The
seq_buf_str() function hits WARN_ON(s->size == 0) in
include/linux/seq_buf.h:100.
I built a kernel with this patch applied and booted it in QEMU without
the trace_event= parameter. The WARN_ON fires twice on every boot:
Kernel version: 7.1.0-next-20260618-gaa8f0a630773 #1 PREEMPT(full)
Boot 1, WARN 1 — trace_event_init():
```
[ 4.339900][ T0] ------------[ cut here ]------------
[ 4.340592][ T0] s->size == 0
[ 4.340596][ T0] WARNING: include/linux/seq_buf.h:100
at trace_event_init+0x1370/0x16c0,
CPU#0: swapper/0/0
[ 4.344985][ T0] RIP: 0010:trace_event_init+0x1370/0x16c0
[ 4.355743][ T0] Call Trace:
[ 4.356106][ T0] <TASK>
[ 4.356441][ T0] trace_init+0x95/0x10c0
[ 4.361793][ T0] start_kernel+0x225/0x4e0
[ 4.362409][ T0] x86_64_start_reservations+0x18/0x30
[ 4.362979][ T0] x86_64_start_kernel+0x11a/0x160
[ 4.363544][ T0] common_startup_64+0x13e/0x158
[ 4.363872][ T0] </TASK>
```
Boot 1, WARN 2 — event_trace_enable_again():
```
[ 4.760317][ T1] ------------[ cut here ]------------
[ 4.760944][ T1] s->size == 0
[ 4.760947][ T1] WARNING: include/linux/seq_buf.h:100
at event_trace_enable_again+0x1c2/0x230,
CPU#0: swapper/0/1
[ 4.765947][ T1] RIP: 0010:event_trace_enable_again+0x1c2/0x230
[ 4.777045][ T1] Call Trace:
[ 4.777759][ T1] do_one_initcall+0x128/0x700
[ 4.779617][ T1] kernel_init_freeable+0x398/0x940
[ 4.780790][ T1] kernel_init+0x21/0x2c0
[ 4.781862][ T1] ret_from_fork+0xb2c/0xdd0
[ 4.782908][ T1] </TASK>
```
These two WARN_ONs fire identically on every boot (verified across
three separate QEMU sessions). On systems with panic_on_warn=1 this
will cause a kernel panic during every boot.
This matches what Jori already asked about in their review —
if the trace_event parameter is not provided, setup_trace_event() never
runs, and the uninitialized seq_buf triggers WARN_ON at both call sites.
Full PoC (compile with: gcc -Wall -o poc poc.c):
```c
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/syscall.h>
int main(void)
{
char *log_buf;
ssize_t log_size;
const size_t buf_size = 4194304; /* 4 MB, enough for full dmesg */
log_buf = calloc(1, buf_size);
if (!log_buf) {
perror("calloc");
return 1;
}
/* SYSLOG_ACTION_READ_ALL = 3: read the full kernel ring buffer */
log_size = syscall(SYS_syslog, 3, log_buf, buf_size - 1);
if (log_size < 0) {
perror("syslog(SYSLOG_ACTION_READ_ALL)");
free(log_buf);
return 1;
}
log_buf[log_size] = '\0';
/*
* The WARN_ON(s->size == 0) in include/linux/seq_buf.h:100
* produces a message containing "s->size == 0" followed by
* "WARNING:" within a few lines. Search for this signature.
*/
if (strstr(log_buf, "s->size == 0") &&
strstr(log_buf, "seq_buf.h")) {
printf("BUG TRIGGERED: WARN_ON in seq_buf_str() found in kernel
log\n");
/* Print the first occurrence with context */
char *p = strstr(log_buf, "s->size == 0");
char *start = p;
while (start > log_buf && *start != '\n' && (p - start) < 512)
start--;
if (*start == '\n') start++;
char *end = strstr(p, "---[ end trace");
size_t len = end ? (size_t)(end - start + 30) : 2048;
if (len > (size_t)(log_buf + log_size - start))
len = log_buf + log_size - start;
printf("%.*s\n", (int)len, start);
free(log_buf);
return 0;
}
printf("BUG NOT DETECTED: seq_buf WARN_ON not found.\n"
"Either the kernel is unpatched or trace_event= was on the
command line.\n");
free(log_buf);
return 1;
}
```
[1]
https://sashiko.dev/#/patchset/20260620175441.223342-1-woradorn.laon%40gmail.com
Thanks,
Xiao
