Messages in this thread | | | From | John Ogness <> | Subject | Re: RFC: printk: kmsg_dump_get_line_nolock() buffer overflow | Date | Thu, 14 Jan 2021 17:06:59 +0106 |
| |
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
| |