Re: [PATCH] Fix faulty logic in the case of recursive printk

From: Jan Kara
Date: Tue Aug 26 2014 - 08:56:28 EST


On Tue 26-08-14 07:23:22, Patrick Palka wrote:
> Thanks for reviewing! I have put back the call to strlen() and adjusted
> the commit message accordingly.
>
> Patrick
>
> -- >8 --
>
> We shouldn't set text_len in the code path that detects printk recursion
> because text_len corresponds to the length of the string inside textbuf.
> A few lines down from the line
>
> text_len = strlen(recursion_msg);
>
> is the line
>
> text_len += vscnprintf(text + text_len, ...);
>
> So if printk detects recursion, it sets text_len to 29 (the length of
> recursion_msg) and logs an error. Then the message supplied by the
> caller of printk is stored inside textbuf but offset by 29 bytes. This
> means that the output of the recursive call to printk will contain 29
> bytes of garbage in front of it.
>
> This defect is caused by commit 458df9fd ("printk: remove separate
> printk_sched buffers and use printk buf instead") which turned the line
>
> text_len = vscnprintf(text, ...);
>
> into
>
> text_len += vscnprintf(text + text_len, ...);
>
> To fix this, this patch avoids setting text_len when logging the printk
> recursion error. This patch also marks unlikely() the branch leading
> up to this code.
>
> Signed-off-by: Patrick Palka <patrick@xxxxxxxxxxxx>
The patch looks good. You can add:
Reviewed-by: Jan Kara <jack@xxxxxxx>

Plus you could also add tags:
Fixes: 458df9fd4815b47809875d57f42e16401674b621
CC: stable@xxxxxxxxxxxxxxx

Honza
> ---
> kernel/printk/printk.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index e04c455..1ce7706 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1665,15 +1665,15 @@ asmlinkage int vprintk_emit(int facility, int level,
> raw_spin_lock(&logbuf_lock);
> logbuf_cpu = this_cpu;
>
> - if (recursion_bug) {
> + if (unlikely(recursion_bug)) {
> static const char recursion_msg[] =
> "BUG: recent printk recursion!";
>
> recursion_bug = 0;
> - text_len = strlen(recursion_msg);
> /* emit KERN_CRIT message */
> printed_len += log_store(0, 2, LOG_PREFIX|LOG_NEWLINE, 0,
> - NULL, 0, recursion_msg, text_len);
> + NULL, 0, recursion_msg,
> + strlen(recursion_msg));
> }
>
> /*
> --
> 2.1.0
>
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
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/