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

[Steven Rostedt <rostedt@goodmis.org>]

On Thu, 29 Oct 2026 12:47:04 +0100
Alejandro Colomar <alx+linux-hardening@kernel.org> wrote:

> 
> This fix in theory solves the issue.  However, I wonder if this is maybe
> too late to report the overflow.  Callers normally expect the overflow
> to happen while they're performing their writes, and maybe they don't
> expect the overflow to happen at this point, so they might forget to
> test, and keep going with truncated data.

No, the overflow should be checked when they are done with the seq_buf.
Which means after they do either seq_buf_str() or seq_buf_terimnate().

> 
> How about not allowing a full buffer without a NUL terminator in the
> first place?  That is, we could report a _putmem or _putc operation that
> writes a non-zero byte to the last element in the buffer as being
> overflowed.
> 
> This would waste one byte, but would prevent users from having to expect
> a possible overflow when reading a buffer.

This only happens when the user expects the string to have a nul
terminating byte. Don't think too much into this.


> > @@ -137,10 +137,16 @@ static inline size_t __seq_buf_terminate(struct seq_buf *s)
> >  {
> >  	size_t end;
> >  

Hmm, I was thinking that this should first check if the string is already
terminated with a nul byte. Why add another one if it is?

> > -	if (seq_buf_buffer_left(s))
> > +	if (seq_buf_buffer_left(s)) {

		if (s->len && !s->buffer[s->len - 1])
			return;

> >  		end = s->len;
> > -	else
> > +	} else {
> >  		end = s->size - 1;

		if (!s->buffer[end])
			return;

Otherwise this overflows the buffer even when it is already terminated.

-- Steve


> > +		/*
> > +		 * About to overwrite a written byte in the last position
> > +		 * in the buffer, so it must be considered overflowed now.
> > +		 */
> > +		seq_buf_set_overflow(s);
> > +	}
> >  
> >  	s->buffer[end] = 0;
> >