linux-kernel - Re: [PATCH v5] printk: Userspace format enumeration support

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: <YH1Kex8NOr89BJXq@alley>
Date:   Mon, 19 Apr 2021 11:16:43 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     Chris Down <chris@...isdown.name>, linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        John Ogness <john.ogness@...utronix.de>,
        Johannes Weiner <hannes@...xchg.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kees Cook <keescook@...omium.org>, kernel-team@...com
Subject: Re: [PATCH v5] printk: Userspace format enumeration support

On Mon 2021-04-19 09:27:43, Rasmus Villemoes wrote:
> On 16/04/2021 15.56, Chris Down wrote:
> > Hey Petr, Rasmus,
> 
> >> This is great point! There are many other subsystem specific wrappers,
> >> e,g, ata_dev_printk(), netdev_printk(), snd_printk(), dprintk().
> >> We should make it easy to index them as well.
> > 
> > These would be nice to have, but we should agree about how we store
> > things internally.
> > 
> > For example, in printk we typically store the level inline as part of
> > the format string at compile time. However, for `dev_printk`, it's
> > passed entirely separately from the format string after preprocessing is
> > already concluded (or at least, not in a way we can easily parse it the
> > same way we do for printk()):
> > 
> >     void dev_printk(const char *level, const struct device *dev, const
> > char *fmt, ...)
> 
> Hm, yeah, for "naked" dev_printk() calls there's no easy way to grab the
> level, for dev_err and friends it's somewhat easier as you could just
> hook into the definition of the dev_err macro. I'm not saying you need
> to handle everything at once, but doing dev_err and netdev_err would get
> you a very long way

It is true that there are many messages printed using
dev_printk(). For example, these rough numbers:

$> git grep pr_err | wc -l
19885
$> git grep dev_err | wc -l
58153

> > One (ugly) way to handle this would be to have a new "level" field in
> > the printk index entry, with semantics that if it's some sentinel value,
> > look at the format itself for the format, otherwise if it's some other
> > value, the level field itself is the level.
> >
> > This will work, but it's pretty ugly. Any better suggestions? :-)

We should use the same algorithm that is used in parse_prefix() called
from vprintk_store(). parse_prefix() updates @level only when
the current value is LOGLEVEL_DEFAULT.

We should set the new field in the printk index entry to
LOGLEVEL_DEFAULT by default. It should be set to a particular level
when it is defined by an extra parameter, like in dev_printk().


> Well, that was more or less exactly what I suggested when I wrote
> 
> > One could also record the function a format is being used with - without
> > that, the display probably can't show a reasonable <level> for those
> > dev_* function.
> 
> But, I think the real question is, why are we/you interested in the
> level at all? Isn't the format string itself enough for the purpose of
> tracking which printks have come and gone? IOW, what about, on the
> display side, simply skipping over some KERN_* prefix if present?

Messages are filtered on consoles by console_loglevel. The loglevel
might be important to decide whether the message is visible or not.

Best Regards,
Petr