Re: [PATCH v5 06/12] seq_buf: Add seq_buf_strlen()
From: Steven Rostedt
Date: Thu Oct 22 2026 - 15:58:21 EST
On Thu, 22 Oct 2026 21:41:53 +0200
Alejandro Colomar <alx+linux-hardening@xxxxxxxxxx> wrote:
> > > Hmm, I just realized that this could inadvertently overflow the buffer. If
> > > len == size, it may not be overflowed. If using seq_buf_putmem() and
> > > filling the entire buffer, calling this would force an overflow as we just
> > > truncated the last byte.
> >
> > Hm, yes. So the "else" branch needs to include:
> >
> > s->len = s->size + 1;
>
> Please think again about the possibility of using pointers instead of
> lengths. This is a source of off-by-one bugs that's only caused by
> the choice of data, I believe. If using pointers, truncation would be
> stored as a NULL pointer, and off-by-ones would be less likely.
>
> Consider this off-by-one to be just a sympthom of the underlying
> problematic design. Whenever a design forces you to write +1 or -1,
> there are high chances that the design is sub-optimal regarding safety.
It's not an off-by-one error here. The s->len = size - 1 is correct, but it
also has a side effect of removing a character without marking it
overflowed. The seq_buf defines being "overflowed" as s->len == s->size + 1.
What the else statement should have is:
else
seq_buf_set_overflow(s);
Which would be less confusing to reviewers as that is defined as:
static inline void
seq_buf_set_overflow(struct seq_buf *s)
{
s->len = s->size + 1;
}
-- Steve