Re: [PATCH v3] seq_buf: Mark a seq_buf overflowed when terminating it loses a byte
From: Alejandro Colomar
Date: Wed Nov 04 2026 - 06:33:53 EST
Hi Kees,
> Date: 2026-11-03 19:31:13-0800
> From: Kees Cook <kees@xxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> Closes: https://lore.kernel.org/all/20261016145928.16eddc9a@xxxxxxxxxxxxxxxxxx/
> Fixes: f2616c772c768 ("seq_buf: Add seq_buf_terminate() API")
> Signed-off-by: Kees Cook <kees@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
(or perhaps a second Reported-by.)
Cheers,
Alex
--
<https://www.alejandro-colomar.es>
Attachment:
signature.asc
Description: PGP signature