linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Chris Down <chris@chrisdown.name>
Cc: linux-kernel@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	John Ogness <john.ogness@linutronix.de>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	kernel-team@fb.com
Subject: Re: design: was: Re: [RFC PATCH v2] printk: console: Allow each console to have its own loglevel
Date: Fri, 15 Jul 2022 14:49:35 +0200	[thread overview]
Message-ID: <20220715124935.GE2737@pathway.suse.cz> (raw)
In-Reply-To: <20220714154452.GB24338@pathway.suse.cz>

On Thu 2022-07-14 17:44:52, Petr Mladek wrote:
> On Wed 2022-07-13 15:49:19, Chris Down wrote:
> > Petr Mladek writes:
> > > IMHO, this makes things too complicated. A better solution is to do
> > > not allow to set any log level below this limit in the first place.
> > 
> > Hmm, how should we then handle the case that you have set the per-console
> > loglevel to 3 and minimum_console_loglevel later gets changed to 5?
> > 
> > We had this problem when designing cgroup v2 as well, for example in cases
> > where a child requests a higher memory protection than can be afforded by
> > the parent, or where a child sets a higher memory limit than a parent
> > specifies. We went back and forth and eventually settled on allowing these,
> > because the alternatives seemed difficult to reason about or unnecessarily
> > inflexible.
> 
> I see.
> 
> > From the per-console loglevel side, one option is to return ERANGE or EINVAL
> > on values we know won't be honoured when setting the per-console loglevel.
> > The problem with that is that it doesn't allow to specify a "desired" limit
> > in case the external factors (in this case, the minimum loglevel) change.
> > This is even more difficult to reason about in our case because the minimum
> > loglevel may be changed dynamically outside of user control.
> >
> > Another is to disallow setting the minimum loglevel without first resetting
> > consoles which are above the value that is desired to be set, but this seems
> > really cumbersome, and again it doesn't account for cases like panic() and
> > elsewhere where we blindly change it anyway.
> >
> > Maybe you have another idea about how it should work in the case that the
> > minimum loglevel would take precedence over an existing loglevel?
> 
> minimum_console_loglevel is currently used only in syslog interface.
> It is ignored when the levels are set using sysctl.
> 
> IMHO, the reason is that sysctl might eventually get called even with
>  less privileged user.
> 
> I would keep this behavior. I mean that a change of
> minimum_console_loglevel would not affect other values immediately.
> It will be used to crop the value when anyone wants to change
> the global loglevel via syslog later.
> 
> Well, it would make sense to crop the global or per-console
> loglevels even when they are later changed via the new sysctl
> or sysfs interface. It would be a limit for less privileged
> users.
> 
> Maybe, it is over-engineered. I wonder if anyone really uses
> the minimum level in practice.

I checked the history:

1. The possibility to change "console_loglevel" was added in 0.99.13k,
   1992. The syslog syscall was the only way to change the value
   at runtime at that time.

2. MINIMUM_CONSOLE_LOGLEVEL was added in 1.3.37, 1995
   It was hardcoded at that time.

3. minimum_console_loglevel was exported via sysctl in 2.1.32, between
   1996-1999. This version added the sysctl interface for all 4 values:
   console_loglevel, minimum_console_loglevel, default_console_loglevel,
   default_message_loglevel. Before it was possible to change only
   console_loglevel via the syslog syscall.

   sysctl allows to set all four values to any value. It does not
   check if they make any sense. And it does not check if
   console_loglevel is above minimum_console_loglevel.
   Only the syslog syscall checks minimum_console_loglevel.

   I guess that nobody really though how the interface could be used.
   It just made the four "configurable" values accessible.


IMHO, it confirms the theory that the minimum value should prevent less
privileged users to change the loglevel below this minimum. And I
would do it as I suggested. I mean that minimum_console_loglevel
will be used only when anyone tries to change the global
or per-console loglevel.

It would make sense to return -EINVAL when anyone wants to set
loglevel below the minimum value via sysctl or sysfs interface.

I suggested to just crop the value yesterday. It is what the syslog
sycall does. But I think that it is actually quite ugly and we could
do better in the new (more modern) interface.

Best Regards,
Petr

  reply	other threads:[~2022-07-15 12:49 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-20 12:57 [RFC PATCH v2] printk: console: Allow each console to have its own loglevel Chris Down
2022-05-20 16:06 ` Rik van Riel
2022-07-07 14:38   ` Petr Mladek
2022-07-13 14:32     ` Chris Down
2022-05-21 18:51 ` Greg Kroah-Hartman
2022-05-21 19:23   ` Chris Down
2022-05-23 13:09 ` Vincent Whitchurch
2022-05-23 14:24   ` Chris Down
2022-06-16 15:57 ` Chris Down
2022-07-08 15:23 ` design: was: " Petr Mladek
2022-07-08 19:04   ` John Ogness
2022-07-11  8:32     ` Petr Mladek
2022-07-11 10:17       ` Sergey Senozhatsky
2022-07-13 14:50       ` Chris Down
2022-07-13 14:49   ` Chris Down
2022-07-14 15:44     ` Petr Mladek
2022-07-15 12:49       ` Petr Mladek [this message]
2022-07-15 13:00         ` Chris Down
2022-07-19 12:11           ` Chris Down
2022-07-19 13:00             ` 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=20220715124935.GE2737@pathway.suse.cz \
    --to=pmladek@suse.com \
    --cc=chris@chrisdown.name \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=john.ogness@linutronix.de \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.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;
as well as URLs for NNTP newsgroup(s).