linux-kernel - Re: [PATCH 1/3] printk: Fix kdb_trap

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] [day] [month] [year] [list]

Message-ID: <20171013142356.GD2795@pathway.suse.cz>
Date:   Fri, 13 Oct 2017 16:23:56 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Joe Perches <joe@...ches.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Kay Sievers <kay@...y.org>, sergey.senozhatsky@...il.com,
        linux-kernel@...r.kernel.org, rostedt@...dmis.org,
        mingo@...nel.org, tglx@...utronix.de,
        Jason Wessel <jason.wessel@...driver.com>
Subject: Re: [PATCH 1/3] printk: Fix kdb_trap_printk placement

On Thu 2017-10-12 11:11:00, Joe Perches wrote:
> On Thu, 2017-10-12 at 14:08 +0200, Greg Kroah-Hartman wrote:
> > On Thu, Oct 12, 2017 at 01:52:29PM +0200, Greg Kroah-Hartman wrote:
> > > On Thu, Oct 12, 2017 at 01:34:39PM +0200, Peter Zijlstra wrote:
> > > > On Thu, Oct 12, 2017 at 12:03:04PM +0200, Petr Mladek wrote:
> > > > > On Thu 2017-10-12 11:45:37, Petr Mladek wrote:
> > > > > Well, I wonder if we should go even further and stop exporting
> > > > > vprintk_emit(). IMHO, the only reason was dev_print_emit() and
> > > > > the ability to pass the extra "dict" parameter.
> > > > 
> > > > You have my blessing there, but the device folks might have an opinion
> > > > on that; Cc'ed Gregkh.
> > > 
> > > Hm, we "need" that dict option, otherwise the whole dev_printk() family
> > > of messages will not work properly, right?
> > > 
> > > Or am I missing something?  If you can figure out a way to still support
> > > the same thing (we need a prefix at the beginning of the message that
> > > shows the device/driver/binding/etc that emitted the message), that's
> > > fine with me, I'm not wed to vprintk_emit() :)
> > 
> > Nope, this doesn't seem to deal with the prefix, except in some odd way
> > that is tied to the dynamic debugging logic.  I really don't know what
> > this does anymore.  Joe wrote it in 2012 as part of the dynamic debug
> > code.
> > 
> > Joe, any thoughts?
>
> Man I hate rabbit-holes.  I need a few days as I'm
> otherwise busy.

Joe, you were added in the middle of the thread and probably
do not have the right context. IMHO, Greg did not want any code.
He just wanted to know if the code is still needed at all.

dev_vprintk_emit() is the only external user of vprintk_emit().
Also it is the only user that sets the "dict" parameter.

If I get it correctly, it is not about message prefix but
about some extra info, see create_syslog_header().
It displayed only on /dev/kmsg and consoles with
CON_EXTENDED flag, see msg_print_ext_header().

One question is if people use it and if it is worth
the complexity.


> This stuff has been broken for half a decade now.
> Perhaps it doesn't need fixing?

This is more about cleaning the interfaces. vprintk_emit() is
a low level API and it would help a lot it is not called
directly outside printk code.

But I already have some idea how to solve this.

> In any case, printk needs a thorough breaking up
> and refactoring.
>
> Pushing around at its edges just makes it worse.

In this case, the edge blocks the refactoring.


> vprintk_emit came from Kay Sievers.
> 
> commit 7ff9554bb578ba02166071d2d487b7fc7d860d62
> Author: Kay Sievers <kay@...y.org>
> Date:   Thu May 3 02:29:13 2012 +0200
> 
>     printk: convert byte-buffer to variable-length record buffer
> 
> It seems printk_emit is also exported and unused.

Yes, this was the infamous commit that complicated printk
code a lot.

Best Regards,
Petr