linux-kernel - Re: [GIT PULL] seq_buf: Fix seq_buf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 04 Mar 2015 12:21:21 -0800
From:	Joe Perches <joe@...ches.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [GIT PULL] seq_buf: Fix seq_buf_vprintf() truncation

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@...dmis.org>
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/