linux-kernel - Re: RFC: printk: kmsg_dump_get_line

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]

Message-ID: <87im7zecec.fsf@jogness.linutronix.de>
Date:   Thu, 14 Jan 2021 17:06:59 +0106
From:   John Ogness <john.ogness@...utronix.de>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-kernel@...r.kernel.org
Subject: Re: RFC: printk: kmsg_dump_get_line_nolock() buffer overflow

On 2021-01-14, Petr Mladek <pmladek@...e.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