seq_buf: Add seq_buf_terminate()

Seven callers call seq_buf_str() only to NUL-terminate the buffer,
discarding the returned pointer. Two of them need a comment to say so.

Add seq_buf_terminate(), wrapping the __seq_buf_terminate() helper that
seq_buf_str() and seq_buf_strlen() already use, and convert those
callers. It returns void: returning the offset would just be
seq_buf_strlen() under another name. A zero-sized seq_buf is left
untouched, as in the other accessors.

Add tests for the three cases: room for the NUL after the data, an
overflowed buffer where it lands in the last byte, and a zero-sized
buffer that must not be written to.

Build tested ARCH=x86_64 defconfig with GCC 16.2.0, plus
CONFIG_HIST_TRIGGERS=y and CONFIG_BPF_SYSCALL=y to reach the converted
call sites in kernel/trace/trace_events_hist.c and
kernel/bpf/diagnostics.c. Tests run 24/24 passing on ARCH=um.

Assisted-by: LLM
Acked-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261005155708.1471260-7-kees@kernel.org
Signed-off-by: Kees Cook <kees@kernel.org>
This commit is contained in:
Kees Cook 2026-10-05 08:56:57 -07:00
parent 8d1478bfd6
commit 88c652df02
5 changed files with 66 additions and 9 deletions

View File

@ -173,6 +173,29 @@ static inline size_t seq_buf_strlen(struct seq_buf *s)
return __seq_buf_terminate(s);
}
/**
* seq_buf_terminate - NUL-terminate the string in a seq_buf
* @s: the seq_buf handle
*
* Terminate @s->buffer exactly as seq_buf_str() and seq_buf_strlen() do,
* for callers that want neither the pointer nor the length and only need
* the buffer to be safe to read as a C string. A zero-sized seq_buf has
* nowhere to put a NUL and is left untouched.
*
* Nothing is returned on purpose: a caller that wants the length should
* use seq_buf_strlen(), which says so.
*
* After this function is called, s->buffer is safe to use
* in string operations.
*/
static inline void seq_buf_terminate(struct seq_buf *s)
{
if (s->size == 0)
return;
__seq_buf_terminate(s);
}
/**
* seq_buf_get_buf - get buffer to write arbitrary data to
* @s: the seq_buf handle

View File

@ -351,7 +351,7 @@ static void diag_fmt_restore(struct bpf_verifier_env *env, struct diag_fmt_mark
if (mark.chunk) {
mark.chunk->seq.len = mark.len;
seq_buf_str(&mark.chunk->seq);
seq_buf_terminate(&mark.chunk->seq);
}
}
@ -631,11 +631,11 @@ static void format_disasm_line(struct bpf_verifier_env *env, int insn_idx,
return;
print_bpf_insn(&cbs, insn, env->allow_ptr_leaks);
seq_buf_str(&ctx.seq);
seq_buf_terminate(&ctx.seq);
ctx.seq.len = strnlen(line->text, sizeof(line->text));
while (ctx.seq.len && line->text[ctx.seq.len - 1] == '\n')
seq_buf_pop(&ctx.seq);
seq_buf_str(&ctx.seq);
seq_buf_terminate(&ctx.seq);
line->valid = true;
}

View File

@ -4906,7 +4906,7 @@ static __init int event_trace_enable(void)
*/
__trace_early_add_events(tr);
seq_buf_str(&bootup_event_seq);
seq_buf_terminate(&bootup_event_seq);
early_enable_events(tr, bootup_event_buf, false);
trace_printk_start_comm();
@ -4935,7 +4935,7 @@ static __init int event_trace_enable_again(void)
if (!tr)
return -ENODEV;
seq_buf_str(&bootup_event_seq);
seq_buf_terminate(&bootup_event_seq);
early_enable_events(tr, bootup_event_buf, true);
return 0;

View File

@ -2988,8 +2988,7 @@ find_synthetic_field_var(struct hist_trigger_data *target_hist_data,
seq_buf_init(&s, synthetic_name, MAX_FILTER_STR_VAL);
seq_buf_printf(&s, "synthetic_%s", field_name);
/* Terminate synthetic_name with a NUL. */
seq_buf_str(&s);
seq_buf_terminate(&s);
if (seq_buf_has_overflowed(&s)) {
kfree(synthetic_name);
@ -3106,8 +3105,7 @@ create_field_var_hist(struct hist_trigger_data *target_hist_data,
if (saved_filter)
seq_buf_printf(&s, " if %s", saved_filter);
/* Terminate cmd with a NUL. */
seq_buf_str(&s);
seq_buf_terminate(&s);
if (seq_buf_has_overflowed(&s)) {
kfree(cmd);

View File

@ -632,6 +632,41 @@ static void seq_buf_strlen_zero_size_test(struct kunit *test)
KUNIT_EXPECT_STREQ(test, buf, "untouched");
}
static void seq_buf_terminate_test(struct kunit *test)
{
char buf[16];
struct seq_buf s;
/*
* seq_buf_putmem() writes no NUL, so only seq_buf_terminate() can
* end these strings early. The NUL in the last byte of buf keeps
* the comparisons in bounds if it does not.
*/
/* Terminates directly after the data when there is room. */
memset(buf, 'z', sizeof(buf) - 1);
buf[sizeof(buf) - 1] = '\0';
seq_buf_init(&s, buf, sizeof(buf));
seq_buf_putmem(&s, "ab", 2);
seq_buf_terminate(&s);
KUNIT_EXPECT_STREQ(test, buf, "ab");
/* Terminates in the last byte once the buffer has overflowed. */
memset(buf, 'z', sizeof(buf) - 1);
buf[sizeof(buf) - 1] = '\0';
seq_buf_init(&s, buf, 4);
seq_buf_putmem(&s, "abcdef", 6);
KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s));
seq_buf_terminate(&s);
KUNIT_EXPECT_STREQ(test, buf, "abc");
/* A zero-sized seq_buf is left alone. */
strscpy(buf, "untouched", sizeof(buf));
seq_buf_init(&s, buf, 0);
seq_buf_terminate(&s);
KUNIT_EXPECT_STREQ(test, buf, "untouched");
}
static struct kunit_case seq_buf_test_cases[] = {
KUNIT_CASE(seq_buf_init_test),
KUNIT_CASE(seq_buf_declare_test),
@ -655,6 +690,7 @@ static struct kunit_case seq_buf_test_cases[] = {
KUNIT_CASE(seq_buf_strlen_puts_overflow_test),
KUNIT_CASE(seq_buf_strlen_embedded_nul_test),
KUNIT_CASE(seq_buf_strlen_zero_size_test),
KUNIT_CASE(seq_buf_terminate_test),
KUNIT_CASE(seq_buf_do_printk_test),
{}
};