Re: design: was: Re: [RFC PATCH v2] printk: console: Allow each console to have its own loglevel

Petr Mladek
2022-07-15T12:49:44Z

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