From: John Ogness <john.ogness@linutronix.de>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
Steven Rostedt <rostedt@goodmis.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
linux-kernel@vger.kernel.org
Subject: Re: RFC: printk: kmsg_dump_get_line_nolock() buffer overflow
Date: Thu, 14 Jan 2021 17:06:59 +0106 [thread overview]
Message-ID: <87im7zecec.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <YABhMFlIpQ/5uQ7s@alley>
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
next prev parent reply other threads:[~2021-01-14 16:02 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-13 20:00 RFC: printk: kmsg_dump_get_line_nolock() buffer overflow John Ogness
2021-01-14 15:20 ` Petr Mladek
2021-01-14 16:00 ` John Ogness [this message]
2021-01-14 16:40 ` Petr Mladek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87im7zecec.fsf@jogness.linutronix.de \
--to=john.ogness@linutronix.de \
--cc=linux-kernel@vger.kernel.org \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
--cc=sergey.senozhatsky.work@gmail.com \
--cc=sergey.senozhatsky@gmail.com \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox