Re: [PATCH] printk: don't unconditionally shortcut print_time()

From: Sergey Senozhatsky
Date: Thu Nov 29 2018 - 23:25:56 EST


On (11/29/18 15:19), Petr Mladek wrote:
> This fixes a real problem. It might even avoid a crash.
>
> See syslog_print_all() and kmsg_dump_get_buffer(). They
> start with calling msg_print_text() many times to calculate
> how many messages fit into the given buffer. Then they
> blindly copy the messages into the buffer.

Hmm. Interesting find.

So can we just use a pessimistic approach then?

If IS_ENABLED(CONFIG_PRINTK_TIME) then always calculate
buffer size as if printk_time was true; but use the actual
printk_time when we copy out messages. Yes, the buffer can
be larger than needed, but at least we don't care about any
races anymore. And we also don't need to pass printk_time
snapshot while we copy out messages.

I really don't know who and why would enable CONFIG_PRINTK_TIME
but then disable printk_time via param. So most of the time that
pessimistic approach should calculate the right buffer size.
Probably.

[..]
> It seems worth supporting the disabled printk_time() properly.
> I am not sure who is disabling the timestamps.

Yeah, I'm curious as well. Can't see why would anyone switch
it back and forth. Kconfig and boot param work just fine.

-ss