Re: [PATCH v3] seq_buf: Mark a seq_buf overflowed when terminating it loses a byte

[Alejandro Colomar <alx@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2392 bytes --]

Hi Kees,

> Date: 2026-11-03 19:31:13-0800
> From: Kees Cook <kees@kernel.org>
>
> When there is no room after the data for a NUL, __seq_buf_terminate()
> writes the NUL over the last byte of the buffer. An overflowed seq_buf
> has already lost data, but one that is exactly full, as seq_buf_putc(),
> seq_buf_putmem(), and seq_buf_commit() can leave it, has not: the NUL
> replaces the last byte written, and the seq_buf still does not report an
> overflow. seq_buf_str(), seq_buf_strlen(), and seq_buf_terminate() all
> go through it, so each of them can silently truncate what was written,
> and seq_buf_pop() then returns the NUL in place of the byte it replaced.
> 
> Mark the seq_buf overflowed in that case with seq_buf_set_overflow().
> Nothing is left between "len" and "size" for it to clear there, and
> seq_buf_used() still counts the lost byte. Data that already ends in a
> NUL is left as is.
> 
> No current caller checks for overflow after terminating a seq_buf that
> can be left exactly full, so none of them behaves differently.
> 
> Add a test that fills a seq_buf exactly and checks that each of the
> three accessors marks it overflowed, that seq_buf_pop() then returns
> -1, and that a seq_buf with room for the NUL is not marked, and another
> that checks the same accessors on data ending in its own NUL. Update the
> existing tests that terminate an exactly full seq_buf to expect it, and
> move the length check in the seq_buf_do_printk() test after the run,
> so that the buffer still reaches seq_buf_do_printk() exactly full.
> 
> 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 new
> tests fail with the fix reverted.
> 
> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> Closes: https://lore.kernel.org/all/20261016145928.16eddc9a@gandalf.local.home/
> Fixes: f2616c772c768 ("seq_buf: Add seq_buf_terminate() API")
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
>  v3:
>  - Reword comments: "left as is", and no "must" (Steven Rostedt)
>  - Fixes: the bug predates seq_buf_strlen(), back to the addition of
>    seq_buf_terminate() (Alejandro Colomar)

Please include:

Cc: Alejandro Colomar <alx@kernel.org>

(or perhaps a second Reported-by.)


Cheers,
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]