Thread (27 messages) 27 messages, 6 authors, 3d ago

Re: [PATCH v4 05/11] seq_buf: Add seq_buf_strlen()

flat view

From: Alejandro Colomar <alx@kernel.org>
Date: 2026-10-05 16:43:45
Also in: bpf, linux-doc, linux-hardening, linux-security-module, linux-trace-kernel, lkml, nvdimm

Hi Kees,
Date: 2026-10-05 08:58:45-0700
From: Kees Cook <kees@kernel.org>

On Mon, Oct 05, 2026 at 01:34:10PM +0200, Alejandro Colomar wrote:
quoted
quoted
Yeah, reasonable. :) For v5 I've added this to seq_buf_str()'s kernel-doc:

 * A zero-sized seq_buf has nowhere to put a NUL, so the empty string
 * is returned instead of writing to @s->buffer. Any other seq_buf
 * returns @s->buffer, even when it holds an empty string, so callers
 * always get their own buffer back.
Are such buffers actually used on purpose anywhere?  Why not keep the
WARN_ON?
Yes, though not often: the sched_ext debug dump builds a nested per-CPU
seq_buf from seq_buf_get_buf(), which returns a size of 0 once the dump
buffer has overflowed, and it handles that case on purpose (see the
"$s may already have overflowed" comment in kernel/sched/ext/ext.c).
Hmmmm, so IIUC, it is a sentinel value, and not really a buffer size?

Did you consider not holding the size of the buffer, but rather an end
pointer?  It also allows you to find how much you can write to the
buffer, but has slightly better semantics: the end doesn't change if you
advance the position.  'end' can only change through reallocations
(which I don't know if you do in the kernel).

It's fundamentally the same issue as was discussed with ARRAY_END() vs
ARRAY_SIZE(), where ARRAY_END() is more robust.  Also, using NULL as a
sentinel value is more readable; otherwise, you wonder why a buffer can
have 0 bytes.

I think it'd simplify the implementation of the buffer.
Greg asked about the WARN_ON() in v2[1]: a size that comes from a device
or from userspace would have to be checked before every seq_buf_init(),
or it becomes a crash under panic_on_warn, and an empty buffer can just
hold the empty string. So the accessors do that instead.
Makes sense.  Thanks for clarifying!


Have a lovely night!
Alex
[1] https://lore.kernel.org/all/2026091953-cherub-empty-ef35@gregkh/ (local)

-Kees

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

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help