Re: [PATCH v4 04/11] seq_buf: Clear what a writer did not claim when a seq_buf overflows

[bot+bpf-ci]

> seq_buf: Clear what a writer did not claim when a seq_buf overflows

TL;DR: Commit message only: the claim that seq_buf_path() was "never exported,
unlike the other writers" looks inaccurate, since several other writers are
also unexported. The code and export look correct.

>     seq_buf: Clear what a writer did not claim when a seq_buf overflows
>
>     Using seq_buf_set_overflow() would leave the bytes between "len"
>     and "size" untouched, so if seq_buf_str() is used on an overflowed
>     seq_buf, those bytes may be exposed. For any paths that don't claim
>     partially written bytes, by setting "len = size" before calling
>     seq_buf_set_overflow(), wipe the unclaimed bytes. The seq_buf_puts()
>     and related APIs already claim those bytes now, so only the unclaimed
>     cases remain. A specific example of this was seq_buf_path() which uses
>     d_path() and would write to the tail before discovering it was out
>     of space, and would correctly mark a seq_buf as overflowed, but the
>     path fragment would be left over.
>
>     Clear from len to the end of the buffer in seq_buf_set_overflow(), which
>     every overflow goes through, including seq_buf_commit() with a negative
>     count.
>
>     Add a test that fills a seq_buf, leaves it too little room for a path, and
>     checks that nothing of the path is left in the buffer. The tests run before
>     anything writable is mounted, so it takes its file from shmem.
>
>     seq_buf_path() was never exported, unlike the other writers, so the
>     test failed to link as a module. Export it.
>
>     Tests passed under qemu on ARCH=x86_64 with GCC 16.2.0 and CONFIG_KASAN=y,
>     and on big-endian ARCH=s390 with GCC s390x-linux-gnu 16.2.0, and the
>     test builds as a module (CONFIG_SEQ_BUF_KUNIT_TEST=m).
>
>     Assisted-by: LLM
>     Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
>     Signed-off-by: Kees Cook <kees@xxxxxxxxxx>

This isn't a bug, but is the comparison in "seq_buf_path() was never
exported, unlike the other writers" accurate?

In lib/seq_buf.c the only writers with an export are seq_buf_printf(),
seq_buf_puts(), seq_buf_putc(), seq_buf_putmem() and seq_buf_putmem_hex()
(plus seq_buf_do_printk(), which is not a writer).  seq_buf_vprintf(),
seq_buf_bprintf() and seq_buf_hex_dump() also write into a seq_buf and have
no EXPORT_SYMBOL_GPL().

The reason for the export, that the new KUnit test calls seq_buf_path() and
has to link with CONFIG_SEQ_BUF_KUNIT_TEST=m, looks correct.  Could the
sentence drop the "unlike the other writers" part, for example:

  seq_buf_path() was not exported, so the test failed to link as a module.
  Export it.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/37096036516