seq_buf_used() return value on overflow

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

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

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.

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>

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