[PATCH] tracing: Use seq_buf for string concatenation

[PATCH] tracing: Use seq_buf for string concatenation

[Woradorn Laodhanadhaworn]

In preparation for removing the strlcat API[1],
replace the string concatenation logic with a struct seq_buf,
which tracks the current position and the remaining space internally.

The backing buffer bootup_event_buf allocation is unchanged.
Use seq_buf_str() to NUL-terminate before passing to early_enable_events().

Link: https://github.com/KSPP/linux/issues/370 [1]

Signed-off-by: Woradorn Laodhanadhaworn <woradorn.laon@gmail.com>
---
 kernel/trace/trace_events.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index c46e623e7e0d..15164723e028 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -22,6 +22,7 @@
 #include <linux/sort.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
+#include <linux/seq_buf.h>
 
 #include <trace/events/sched.h>
 #include <trace/syscall.h>
@@ -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;
 
 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);
+
+	if (seq_buf_has_overflowed(&bootup_event_seq))
+		return -ENOMEM;
 
 	trace_set_ring_buffer_expanded(NULL);
 	disable_tracing_selftest("running event tracing");
@@ -4766,7 +4777,7 @@ static __init int event_trace_enable(void)
 	 */
 	__trace_early_add_events(tr);
 
-	early_enable_events(tr, bootup_event_buf, false);
+	early_enable_events(tr, (char *)seq_buf_str(&bootup_event_seq), false);
 
 	trace_printk_start_comm();
 
@@ -4794,7 +4805,7 @@ static __init int event_trace_enable_again(void)
 	if (!tr)
 		return -ENODEV;
 
-	early_enable_events(tr, bootup_event_buf, true);
+	early_enable_events(tr, (char *)seq_buf_str(&bootup_event_seq), true);
 
 	return 0;
 }
-- 
2.43.0

Re: [PATCH] tracing: Use seq_buf for string concatenation

[Jori Koolstra]

> Op 20-06-2026 19:54 CEST schreef Woradorn Laodhanadhaworn <woradorn.laon@gmail.com>:
> 
>  
> In preparation for removing the strlcat API[1],
> replace the string concatenation logic with a struct seq_buf,
> which tracks the current position and the remaining space internally.
> 
> The backing buffer bootup_event_buf allocation is unchanged.
> Use seq_buf_str() to NUL-terminate before passing to early_enable_events().
> 
> Link: https://github.com/KSPP/linux/issues/370 [1]
> 
> Signed-off-by: Woradorn Laodhanadhaworn <woradorn.laon@gmail.com>
> ---
>  kernel/trace/trace_events.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index c46e623e7e0d..15164723e028 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -22,6 +22,7 @@
>  #include <linux/sort.h>
>  #include <linux/slab.h>
>  #include <linux/delay.h>
> +#include <linux/seq_buf.h>
>  
>  #include <trace/events/sched.h>
>  #include <trace/syscall.h>
> @@ -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;

Isn't this now unused?

> +static struct seq_buf bootup_event_seq;
> +static bool bootup_event_seq_initialized;
>  

I think this can be refactored to avoid the bool. And should bootup_event_seq not be
__initdata?

>  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);
> +
> +	if (seq_buf_has_overflowed(&bootup_event_seq))
> +		return -ENOMEM;
>  
>  	trace_set_ring_buffer_expanded(NULL);
>  	disable_tracing_selftest("running event tracing");
> @@ -4766,7 +4777,7 @@ static __init int event_trace_enable(void)
>  	 */
>  	__trace_early_add_events(tr);
>  
> -	early_enable_events(tr, bootup_event_buf, false);
> +	early_enable_events(tr, (char *)seq_buf_str(&bootup_event_seq), false);

What if trace_event is empty? Then setup_trace_event does not run AFAIK. See the
WARN_ON in seq_buf_str too. Have you tested this?

>  
>  	trace_printk_start_comm();
>  
> @@ -4794,7 +4805,7 @@ static __init int event_trace_enable_again(void)
>  	if (!tr)
>  		return -ENODEV;
>  
> -	early_enable_events(tr, bootup_event_buf, true);
> +	early_enable_events(tr, (char *)seq_buf_str(&bootup_event_seq), true);
>  
>  	return 0;
>  }
> -- 
> 2.43.0

Thanks,
Jori.

Re: [PATCH] tracing: Use seq_buf for string concatenation

[XIAO WU]

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

Re: [PATCH] tracing: Use seq_buf for string concatenation

[Woradorn Laodhanadhaworn]

On 22/6/2569 BE 00:16, Jori Koolstra wrote:
> 
>> Op 20-06-2026 19:54 CEST schreef Woradorn Laodhanadhaworn <woradorn.laon@gmail.com>:
>>
>>  
>> In preparation for removing the strlcat API[1],
>> replace the string concatenation logic with a struct seq_buf,
>> which tracks the current position and the remaining space internally.
>>
>> The backing buffer bootup_event_buf allocation is unchanged.
>> Use seq_buf_str() to NUL-terminate before passing to early_enable_events().
>>
>> Link: https://github.com/KSPP/linux/issues/370 [1]
>>
>> Signed-off-by: Woradorn Laodhanadhaworn <woradorn.laon@gmail.com>
>> ---
>>  kernel/trace/trace_events.c | 21 ++++++++++++++++-----
>>  1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
>> index c46e623e7e0d..15164723e028 100644
>> --- a/kernel/trace/trace_events.c
>> +++ b/kernel/trace/trace_events.c
>> @@ -22,6 +22,7 @@
>>  #include <linux/sort.h>
>>  #include <linux/slab.h>
>>  #include <linux/delay.h>
>> +#include <linux/seq_buf.h>
>>  
>>  #include <trace/events/sched.h>
>>  #include <trace/syscall.h>
>> @@ -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;
> 
> Isn't this now unused?
> 
>> +static struct seq_buf bootup_event_seq;
>> +static bool bootup_event_seq_initialized;
>>  
> 
> I think this can be refactored to avoid the bool. And should bootup_event_seq not be
> __initdata?
> 
>>  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);
>> +
>> +	if (seq_buf_has_overflowed(&bootup_event_seq))
>> +		return -ENOMEM;
>>  
>>  	trace_set_ring_buffer_expanded(NULL);
>>  	disable_tracing_selftest("running event tracing");
>> @@ -4766,7 +4777,7 @@ static __init int event_trace_enable(void)
>>  	 */
>>  	__trace_early_add_events(tr);
>>  
>> -	early_enable_events(tr, bootup_event_buf, false);
>> +	early_enable_events(tr, (char *)seq_buf_str(&bootup_event_seq), false);
> 
> What if trace_event is empty? Then setup_trace_event does not run AFAIK. See the
> WARN_ON in seq_buf_str too. Have you tested this?
> 
>>  
>>  	trace_printk_start_comm();
>>  
>> @@ -4794,7 +4805,7 @@ static __init int event_trace_enable_again(void)
>>  	if (!tr)
>>  		return -ENODEV;
>>  
>> -	early_enable_events(tr, bootup_event_buf, true);
>> +	early_enable_events(tr, (char *)seq_buf_str(&bootup_event_seq), true);
>>  
>>  	return 0;
>>  }
>> -- 
>> 2.43.0
> 
> Thanks,
> Jori.

Thank you, Jori, for your review. I will send v2.
I tested both empty and non-empty trace_event cases with QEMU, and both now boot successfully. 

The logs below demonstrate this.

Non-empty trace_event:

qemu-system-x86_64 \
    -kernel arch/x86/boot/bzImage \
    -initrd initramfs.cpio.gz \
    -append "console=ttyS0 trace_event=:mod:rproc_qcom_common,:mod:qrtr,:mod:qcom_aoss trace_event=:mod:rproc_qcom_common" \
    -nographic \
    -m 512M

[    0.082316] Kernel command line: console=ttyS0 trace_event=:mod:rproc_qcom_common,:mod:qrtr,:mod:qcom_aoss trace_event=:mod:rproc_qcom_common
[    0.083324] bootup_event_buf: :mod:rproc_qcom_common,:mod:qrtr,:mod:qcom_aoss
[    0.083413] bootup_event_buf: :mod:rproc_qcom_common,:mod:qrtr,:mod:qcom_aoss,:mod:rproc_qcom_common
[    0.083689] printk: log buffer data + meta data: 262144 + 917504 = 1179648 bytes

Empty trace_event:

qemu-system-x86_64 \
    -kernel arch/x86/boot/bzImage \
    -initrd initramfs.cpio.gz \
    -append "console=ttyS0" \
    -nographic \
    -m 512M

[    0.085213] Kernel command line: console=ttyS0
[    0.086458] printk: log buffer data + meta data: 262144 + 917504 = 1179648 bytes

Thanks,
Woradorn