Re: [GIT PULL] seq_buf: Fix seq_buf_vprintf() truncation

From: Joe Perches
Date: Wed Mar 04 2015 - 15:21:30 EST


On Wed, 2015-03-04 at 15:12 -0500, Steven Rostedt wrote:
> Linus,
>
> While working on merging seq_file and seq_buf code, I hit a bug and came
> down to a bug in seq_buf_printf(). This is suppose to be a all or nothing
> function. That is, either all of the string passed in is saved to the
> seq_buf buffer, or none of it. But due to the way vsnprintf() works in
> the kernel, if the line is truncated, the return value is the amount
> written to the buffer, including the '\0'. Then the test in seq_buf_printf()
> uses the returned length to see if it would fit in the buffer. It will,
> but it will also fill the buffer. The test to see if vsnprintf() overflowed
> or not is to see if the length returned is equal to or greater than
> the length of the buffer passed to it. Not just greater than.
>
> Please pull the latest trace-fixes-v4.0-rc2 tree, which can be found at:
[]
> commit 4a8fe4e1811c96ad0ad9f4083f2fe4fb43b2988d
> Author: Steven Rostedt (Red Hat) <rostedt@xxxxxxxxxxx>
> Date: Wed Mar 4 09:56:02 2015 -0500
>
> seq_buf: Fix seq_buf_vprintf() truncation
>
> In seq_buf_vprintf(), vsnprintf() is used to copy the format into the
> buffer remaining in the seq_buf structure. The return of vsnprintf()
> is the amount of characters written to the buffer excluding the '\0',
> unless the line was truncated!
>
> If the line copied does not fit, it is truncated, and a '\0' is added
> to the end of the buffer. But in this case, '\0' is included in the length
> of the line written. To know if the buffer had overflowed, the return
> length will be the same as the length of the buffer passed in.
>
> The check in seq_buf_vprintf() only checked if the length returned from
> vsnprintf() would fit in the buffer, as the seq_buf_vprintf() is only
> to be an all or nothing command. It either writes all the string into
> the seq_buf, or none of it. If the string is truncated, the pointers
> inside the seq_buf must be reset to what they were when the function was
> called. This is not the case. On overflow, it copies only part of the string.
>
> The fix is to change the overflow check to see if the length returned from
> vsnprintf() is less than the length remaining in the seq_buf buffer, and not
> if it is less than or equal to as it currently does. Then seq_buf_vprintf()
> will know if the write from vsnpritnf() was truncated or not.
[]
> diff --git a/lib/seq_buf.c b/lib/seq_buf.c
[]
> @@ -61,7 +61,7 @@ int seq_buf_vprintf(struct seq_buf *s, const char *fmt, va_list args)
>
> if (s->len < s->size) {
> len = vsnprintf(s->buffer + s->len, s->size - s->len, fmt, args);

Presumably the seq_buf_bprintf function has the same issue.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/