Re: seq_buf_used() return value on overflow

[Alejandro Colomar <alx+linux-hardening@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2984 bytes --]

> Date: 2026-10-28 21:59:44+0100
> From: Alejandro Colomar <alx+linux-hardening@kernel.org>
>
> Hi Steven,
> 
> 
> Another question that arised while rewriting seq_buf.
> 
> There's seq_buf_used(), which replaced (in 2014) the previous
> SEQ_BUF_USED().  That commit changed the value from
> 	min((s)->len, (s)->size - 1)
> to
> 	min(s->len, s->size)
> but didn't change any other code.  I don't see any mentions about that
> off-by-one change in the commit message.  (There are two 'Link:' tags,
> but I didn't see any discussion about this patch in those; they seem to
> focus on other patches of the same set.)
> 
> I wonder, because when changing to use pointers, the good behavior would
> be with the -1, so I wonder whether we can go back to the old behavior
> easily, or if anything depends on that value.  Was this a regression?
> Or was it a silent fix?  Or is it unimportant at the moment?
> 
> This is the commit that I'm talking about:
> 
> eeab98154dc0 (2014-11-19; "seq_buf: Create seq_buf_used() to find out how much was written")
> 
> Here's what I'd like to change:
> 
> 	diff --git i/include/linux/seq_buf.h w/include/linux/seq_buf.h
> 	index 05e70ac18ddb..63d077dd8a74 100644
> 	--- i/include/linux/seq_buf.h
> 	+++ w/include/linux/seq_buf.h
> 	@@ -78,7 +78,7 @@
> 	 /* How much buffer was written? */
> 	 static inline unsigned int seq_buf_used(struct seq_buf *s)
> 	 {
> 	-       return min(s->len, s->size);
> 	+       return min(s->len, s->size - 1);
> 	 }
> 	 
> 	 /**
> 
> The rationale for having that -1 is the following:
> Consider a representation of a buffer where s->p holds either a pointer
> to the terminating null byte (or the position where it should be written,
> if not yet written), or NULL if the buffer has overflown, and s->end
> holds unconditionally ARRAY_END() of the buffer.
> 
> By doing s->size-1 on overflow, we'd translate it to s->end-1 with the
> pointer representation, which happens to point to the null byte (or
> where it should be written), which would make this offset quite
> consistent.
> 
> If we do s->size without the -1, then that offset puts us one-past-the-
> last element, which isn't usable in the same places.

Oh, I think I see the reason.  If ->size==0, then -1 would wrap around
and give a huge value.

So, we can't do this for now.  I'll have to do the change at the same
time that I do the other changes (I was hoping I could do some parts
gradually).


Cheers,
Alex

> 
> If you would accept a patch to add back that -1, let me know and I'll
> send it, or feel free to do it yourself adding the following:
> 
> 	Reported-by: Alejandro Colomar <alx@kernel.org>
> 	Suggested-by: Alejandro Colomar <alx@kernel.org>
> 
> I guess it would be better that I send a patch with a proper commit
> message.
> 
> 
> Have a lovely night!
> Alex
> 
> -- 
> <https://www.alejandro-colomar.es>



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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]