Re: RFC: printk: kmsg_dump_get_line_nolock() buffer overflow

[John Ogness <john.ogness@linutronix.de>]

On 2021-01-14, Petr Mladek <pmladek@suse.com> wrote:
> It is pitty that I have missed this. I remember that I discussed
> exactly this problem before, see
> https://lore.kernel.org/lkml/20190710080402.ab3f4qfnvez6dhtc@axis.com/
>
> And I did exactly the same mistake. I have missed the two users in
> "arch/powerpc" and "arch/um".
>
> It is clear that this problem happens repeatedly.

Yes, because the semantics are poor and undocumented.

> Now, the change in record_printk_text() behavior affects also other
> callers. For example, syslog_print() fills the buffer completely
> as well now. I could imagine a userspace code that does the same
> mistake and it works just by chance.

No, syslog_print() works fine. There are only 2 users that think they
can blindly add a byte at buffer[len]. Their code looks scary just
seeing it.

> We should restore the original record_printk_text() behavior
> and add the comment explaining why it is done this way.

OK.

> And I would even explicitly add the trailing '\0' as suggested at
> https://lore.kernel.org/lkml/20190710121049.rwhk7fknfzn3cfkz@pathway.suse.cz/#t

OK. But then this becomes official semantics so powerpc/um no longer
need to append a terminator.

John Ogness