linux-kernel - Re: [PATCH v2 4/6] printk: honor LOG_PREFIX in msg_print

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Fri, 18 Jul 2014 13:03:09 +0200
From:	Petr Mládek <pmladek@...e.cz>
To:	Alex Elder <elder@...aro.org>
Cc:	akpm@...ux-foundation.org, bp@...e.de, john.stultz@...aro.org,
	jack@...e.cz, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/6] printk: honor LOG_PREFIX in msg_print_text()

On Thu 2014-07-17 09:09:09, Alex Elder wrote:
> This patch fixes a problem similar to what was addressed in the
> previous patch.
> 
> All paths that read and format log records (for consoles, and for
> reading via syslog and /dev/kmsg) go through msg_print_text().  That
> function starts with some logic to determine whether the given log
> record when formatted should begin with a "prefix" string, and
> whether it should end with a newline.  That logic has a bug.
> 
> The LOG_PREFIX flag in a log record indicates that when it's
> formatted, a log record should include a prefix.  This is used to
> force a record to begin a new line--even if its preceding log record
> contained LOG_CONT (indicating its content should be combined with
> the next record).
> 
> Like the previous patch, the problem occurs when these flags are
> all set:
>     prev & LOG_CONT
>     msg->flags & LOG_PREFIX
>     msg->flags & LOG_CONT
> In that case, "prefix" should be true but it is not.
> 
> The fix involves checking LOG_PREFIX when a message has its LOG_CONT
> flag set, and in that case allowing "prefix" to become false only
> if LOG_PREFIX is not set.  I.e., the logic for "prefix" would become:
> 
>     if (prev & LOG_CONT && !(msg->flags & LOG_PREFIX))
>         prefix = false;
>     if (msg->flags & LOG_CONT)
>         if (prev & LOG_CONT && !(msg->flags & LOG_PREFIX))
>             prefix = false;
> 
> However, note that this makes the (msg->flags & LOG_CONT) block
> redunant--it's handled by the test just above it.  The result
> becomes quite a bit simpler than before.
> 
> The following table concisely defines the problem:
> 
>      prev | msg  | msg  ||
>      CONT |PREFIX| CONT ||prefix
>     ------+------+------++------
>      clear| clear| clear||  true
>      clear| clear|  set ||  true
>      clear|  set | clear||  true
>      clear|  set |  set ||  true
>       set | clear| clear|| false
>       set | clear|  set || false
>       set |  set | clear||  true
>       set |  set |  set || false    <-- should be true
> 
> Signed-off-by: Alex Elder <elder@...aro.org>
> ---
>  kernel/printk/printk.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 5d719bf..e5ee1cb 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1006,11 +1006,8 @@ static size_t msg_print_text(const struct printk_log *msg, enum log_flags prev,
>  	if ((prev & LOG_CONT) && !(msg->flags & LOG_PREFIX))
>  		prefix = false;
>  
> -	if (msg->flags & LOG_CONT) {
> -		if (prev & LOG_CONT)
> -			prefix = false;
> +	if (msg->flags & LOG_CONT)
>  		newline = false;
> -	}

I agree. It does not make sense to remove prefix just because someone
forgot to finish the previous continuous line. Instead, we should add
'\n" and you will do this in the next patch.

Reviewed-by: Petr Mladek <pmladek@...e.cz>

Best Regards,
Petr
--
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/