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

[Steven Rostedt <rostedt@goodmis.org>]

On Mon,  5 Oct 2026 08:56:56 -0700
Kees Cook <kees@kernel.org> wrote:

> +/*
> + * NUL-terminate the buffer in @s: directly after the data when there is
> + * room for it, otherwise in the last byte of the buffer. @s->size must not
> + * be zero.
> + *
> + * Returns: the offset of the NUL.
> + */
> +static inline size_t __seq_buf_terminate(struct seq_buf *s)
> +{
> +	size_t end;
> +
> +	if (seq_buf_buffer_left(s))
> +		end = s->len;
> +	else
> +		end = s->size - 1;
> +
> +	s->buffer[end] = 0;

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.

> +
> +	return end;
> +}
> +
>  /**
>   * seq_buf_str - get NUL-terminated C string from seq_buf
>   * @s: the seq_buf handle
> @@ -98,7 +119,14 @@ static inline unsigned int seq_buf_used(struct seq_buf *s)
>   *
>   * Note, if this is called when the buffer has overflowed, then
>   * the last byte of the buffer is zeroed, and the len will still
> - * point passed it.
> + * point passed it. The same happens when the buffer is exactly
> + * full: the NUL takes the place of the last byte written, which is
> + * lost, though seq_buf_used() still counts it.
> + *
> + * 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.
>   *
>   * After this function is called, s->buffer is safe to use
>   * in string operations.
> @@ -107,17 +135,44 @@ static inline unsigned int seq_buf_used(struct seq_buf *s)
>   */
>  static inline const char *seq_buf_str(struct seq_buf *s)
>  {
> -	if (WARN_ON(s->size == 0))
> +	if (s->size == 0)
>  		return "";
>  
> -	if (seq_buf_buffer_left(s))
> -		s->buffer[s->len] = 0;
> -	else
> -		s->buffer[s->size - 1] = 0;
> +	__seq_buf_terminate(s);
>  
>  	return s->buffer;
>  }
>  
> +/**
> + * seq_buf_strlen - get the length of the NUL-terminated C string in seq_buf

Hmm,

I don't really like the way this modifies the string, as the request is
just to return the string length.

Just like the above, this could cause an inadvertent overflow.

I could envision a use case where we want to use the entire buffer and do
not expect it to be nul terminated. And this function could be used to get
the length to print. That is, it may want to use the entire length.

Having "strlen()" modify the string just seems wrong to me.

Is it really needed? If the user wants to make sure the string is
terminated, it should use the next patch's "seq_buf_terminate()".

And we need to update it so that if seq_buf_terminate() causes a
truncation, it needs to set that it overflowed.

-- Steve


> + * @s: the seq_buf handle
> + *
> + * This makes sure that the buffer in @s is NUL-terminated, exactly as
> + * seq_buf_str() does, and returns the length of the resulting string
> + * without walking it. Unlike seq_buf_used(), this does not count the byte
> + * given up to the NUL when the buffer is full or has overflowed. When the
> + * buffer is exactly full, that byte is the last one written, and calling
> + * either function loses it.
> + *
> + * A zero-sized seq_buf holds no string, so 0 is returned without writing
> + * to @s->buffer, matching what seq_buf_str() returns for one.
> + *
> + * After this function is called, s->buffer is safe to use
> + * in string operations.
> + *
> + * Returns: the offset of the NUL that terminates @s->buffer. That is the
> + * length of the string unless an earlier NUL is in the way, either one the
> + * data written to @s carried itself, or one seq_buf_set_overflow() left
> + * behind when it cleared what no writer had claimed.
> + */
> +static inline size_t seq_buf_strlen(struct seq_buf *s)
> +{
> +	if (s->size == 0)
> +		return 0;
> +
> +	return __seq_buf_terminate(s);
> +}
> +
>  /**
>   * seq_buf_get_buf - get buffer to write arbitrary data to
>   * @s: the seq_buf handle