Quoting Alexandre SIMON ([email protected]):
> commit 4234697a64df3b1524aa047448930ff940c1bb92 upstream.
>
> This patch corrects a buffer overflow in kernels from 3.0 to 3.4 when calling
> log_prefix() function from call_console_drivers().
>
> This bug existed in previous releases but has been revealed with commit
> 162a7e7500f9664636e649ba59defe541b7c2c60 (2.6.39 => 3.0) that made changes
> about how to allocate memory for early printk buffer (use of memblock_alloc).
> It disappears with commit 7ff9554bb578ba02166071d2d487b7fc7d860d62 (3.4 =>
> 3.5)
> that does a refactoring of printk buffer management.
>
> In log_prefix(), the access to "p[0]", "p[1]", "p[2]" or
> "simple_strtoul(&p[1], &endp, 10)" may cause a buffer overflow as this
> function is called from call_console_drivers by passing "&LOG_BUF(cur_index)"
> where the index must be masked to do not exceed the buffer's boundary.
>
> The trick is to prepare in call_console_drivers() a buffer with the necessary
> data (PRI field of syslog message) to be safely evaluated in log_prefix().
>
> This patch can be applied to stable kernel branches 3.0.y, 3.2.y and 3.4.y.
>
> Without this patch, one can freeze a server running this loop from shell :
> $ export DUMMY=`cat /dev/urandom | tr -dc
> '12345AZERTYUIOPQSDFGHJKLMWXCVBNazertyuiopqsdfghjklmwxcvbn' | head -c255`
> $ while true do ; echo $DUMMY > /dev/kmsg ; done
>
> The "server freeze" depends on where memblock_alloc does allocate printk
> buffer :
> if the buffer overflow is inside another kernel allocation the problem may not
> be revealed, else the server may hangs up.
>
> Signed-off-by: Alexandre SIMON <[email protected]>
> ---
> include/linux/syslog.h | 6 ++++++
> kernel/printk.c | 12 +++++++++++-
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/syslog.h b/include/linux/syslog.h
> index 3891139..ce4c665 100644
> --- a/include/linux/syslog.h
> +++ b/include/linux/syslog.h
> @@ -47,6 +47,12 @@
> #define SYSLOG_FROM_CALL 0
> #define SYSLOG_FROM_FILE 1
>
> +/*
> + * Syslog priority (PRI) maximum length in char : '<[0-9]{1,3}>'
> + * See RFC5424 for details
> +*/
> +#define SYSLOG_PRI_MAX_LENGTH 5
> +
> int do_syslog(int type, char __user *buf, int count, bool from_file);
>
> #endif /* _LINUX_SYSLOG_H */
> diff --git a/kernel/printk.c b/kernel/printk.c
> index 3fc4708..130b436 100644
> --- a/kernel/printk.c
> +++ b/kernel/printk.c
> @@ -633,8 +633,18 @@ static void call_console_drivers(unsigned start,
> unsigned end)
> start_print = start;
> while (cur_index != end) {
> if (msg_level < 0 && ((end - cur_index) > 2)) {
> + /*
> + * prepare buf_prefix, as a contiguous array,
> + * to be processed by log_prefix function
> + */
> + char buf_prefix[SYSLOG_PRI_MAX_LENGTH+1];
> + unsigned i;
> + for (i = 0; i < ((end - cur_index)) && (i <
> SYSLOG_PRI_MAX_LENGTH); i++) {
> + buf_prefix[i] = LOG_BUF(cur_index + i);
> + }
You're not guaranteeing that bug_prefix ends with a '\0'. (I'd assume
LOG_BUF(end) is '\0', and you don't copy it) Is that guaranteed some
other way? If not, does that not introduce new potential bugginess when
doing the simple_strtoul()?
> +
> /* strip log prefix */
> - cur_index += log_prefix(&LOG_BUF(cur_index),
> &msg_level, NULL);
> + cur_index += log_prefix((const char *)&buf_prefix,
> &msg_level, NULL);
> start_print = cur_index;
> }
> while (cur_index != end) {
> --
> 1.7.9.6
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
