Re: [PATCH -v4 2/2] printk: Add kernel parameter to control writes to /dev/kmsg

[Borislav Petkov]

On Thu, Jul 14, 2016 at 01:23:23PM -0700, Andrew Morton wrote:
> Which changes current kernel behaviour.  Can we please get some
> discussion into this changelog which justifies that decision?  What's
> the reasoning and how do we know it won't break existing stuff?

What Rostedt said and in addition, since we do offer this /dev/kmsg
facility to userspace and since userspace likes to (ab-)use it from time
to time, we're adding a controlling cmdline param which should make
everyone happy as it is a tristate.

> Please also include a description of the reasoning behind this design
> decision.

See above. It is really that simple - we want to control how it is being
logged to that interface, thus a tristate. For example, the "on" usecase
I know of is people wanting to debug systemd and they want to see *all*
output go to dmesg.

The "off" use case is for kernel people not wanting to have any
userspace lines in the logs when debugging the kernel.

The "ratelimit" being the default...

> > Signed-off-by: Borislav Petkov <bp@xxxxxxx>
> > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > Cc: Franck Bui <fbui@xxxxxxxx>
> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> > Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> > Cc: Uwe Kleine-K__nig <u.kleine-koenig@xxxxxxxxxxxxxx>
> 
> Did nobody ack any of this?

I'm sure they all want this but are tired of the bikeshedding :-)

> > +	printk.devkmsg={on,off}
> > +			Control writing to /dev/kmsg.
> > +			on - unlimited logging to /dev/kmsg from userspace
> > +			off - logging to /dev/kmsg disabled
> > +			Default: ratelimited logging.
> 
> printk.devkmsg=ratelimit is undocumented?

Fixed.

> > +printk_devkmsg:
> > +
> > +Control the logging to /dev/kmsg from userspace:
> > +
> > +0: default, ratelimited
> > +1: unlimited logging to /dev/kmsg from userspace
> > +2: logging to /dev/kmsg disabled
> 
> hm.  If we're going to make this a pain old 0/1/2 then perhaps the boot
> parameter should also simply accept 0/1/2.

So my reasoning to have the string option names for the cmdline
parameter is so that you don't have to always go lookup what was what,
was 1 off or on, etc.

So actually, I should make the sysctl accept "on", "off" and "ratelimit"
instead too.

Agreed?

> > +The kernel command line parameter printk.devkmsg= overrides this and is
> > +a one-time setting until next reboot: once set, it cannot be changed by
> > +this sysctl interface anymore.
> 
> Why?

What Rostedt said.

> Please put these forward declarations near top-of-file.  Otherwise we
> can later get duplicates.

Done.

> Please enhance the comment to describe why this is being done.  The
> reasoning behind it.

I changed Rostedt's text a bit:

        /*
         * Sysctl cannot change it anymore. The kernel command line setting of
         * this parameter is to force the setting to be permanent throughout the
         * runtime of the system. This is a precation measure against userspace
         * trying to be a smarta** and attempting to change it up on us.
         */

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--