Re: [PATCH v5 06/12] seq_buf: Add seq_buf_strlen()

[Steven Rostedt <rostedt@goodmis.org>]

On Thu, 22 Oct 2026 21:41:53 +0200
Alejandro Colomar <alx+linux-hardening@kernel.org> 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