| 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
| ||
|
Message-ID: <t54jl4sa44xqhmgi4pcco725xntufli2ht4uaymcz55in63znh@tw42mlpbuxhe>
Date: Mon, 15 Dec 2025 11:08:12 +0100
From: Joel Granados <joel.granados@...nel.org>
To: Petr Mladek <pmladek@...e.com>
Cc: Chris Down <chris@...isdown.name>, linux-kernel@...r.kernel.org,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Sergey Senozhatsky <senozhatsky@...omium.org>,
Steven Rostedt <rostedt@...dmis.org>, John Ogness <john.ogness@...utronix.de>,
Geert Uytterhoeven <geert@...ux-m68k.org>, Tony Lindgren <tony.lindgren@...ux.intel.com>,
kernel-team@...com
Subject: Re: [PATCH v8 18/21] printk: Deconstruct kernel.printk into discrete
sysctl controls
On Fri, Dec 12, 2025 at 04:24:17PM +0100, Petr Mladek wrote:
> Adding Joel into Cc. Joel, see the original patch at
> https://lore.kernel.org/all/c3e5cc507eb3fd7db0a002f31d7e47d764cad176.1764272407.git.chris@chrisdown.name/
Thx for adding me. Please CC me in the next versions of this series as
the interface might change (slightly) again. By having this on my radar
we can better coordinate (if needed) linux-next and PRs for Linus.
>
> On Fri 2025-11-28 03:44:25, Chris Down wrote:
> > Introduce two new sysctl interfaces for configuring global loglevels:
> >
> > - kernel.console_loglevel: Sets the global console loglevel, determining
> > the minimum priority of messages printed to consoles. Messages with a
> > loglevel lower than this value will be printed.
> > - kernel.default_message_loglevel: Sets the default loglevel for
> > messages that do not specify an explicit loglevel.
> >
> > The kernel.printk sysctl was previously used to set multiple loglevel
> > parameters simultaneously, but it was confusing and lacked proper
> > validation. By introducing these dedicated sysctl interfaces, we provide
> > a clearer and more granular way to configure the loglevels.
> >
> > Reviewed-by: Petr Mladek <pmladek@...e.com>
> > Tested-by: Petr Mladek <pmladek@...e.com>
> > Signed-off-by: Chris Down <chris@...isdown.name>
>
> This patch requires non-trivial changes when rebased on top of v6.19.
> There have been some conflicting changes in the sysctl API, see
> https://lore.kernel.org/lkml/20251016-jag-sysctl_conv-v2-0-a2f16529acc4@kernel.org/
>
> > --- a/include/linux/sysctl.h
> > +++ b/include/linux/sysctl.h
> > @@ -235,6 +235,13 @@ extern struct ctl_table_header *register_sysctl_mount_point(const char *path);
> >
> > void do_sysctl_args(void);
> > bool sysctl_is_alias(char *param);
> > +int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp, int *valp,
> > + int write, void *data);
> > +int do_proc_dointvec(const struct ctl_table *table, int write,
> > + void *buffer, size_t *lenp, loff_t *ppos,
> > + int (*conv)(bool *negp, unsigned long *lvalp, int *valp,
> > + int write, void *data),
> > + void *data);
> > int do_proc_douintvec(const struct ctl_table *table, int write,
> > void *buffer, size_t *lenp, loff_t *ppos,
> > int (*conv)(unsigned long *lvalp,
>
> This hunk can be removed in v6.19. There is another interface in 6.19,
> see below.
>
> > diff --git a/kernel/printk/sysctl.c b/kernel/printk/sysctl.c
> > index da77f3f5c1fe..034739939a61 100644
> > --- a/kernel/printk/sysctl.c
> > +++ b/kernel/printk/sysctl.c
> > @@ -11,6 +11,9 @@
> >
> > static const int ten_thousand = 10000;
> >
> > +static int min_msg_loglevel = LOGLEVEL_EMERG;
> > +static int max_msg_loglevel = LOGLEVEL_DEBUG;
> > +
> > static int proc_dointvec_minmax_sysadmin(const struct ctl_table *table, int write,
> > void *buffer, size_t *lenp, loff_t *ppos)
> > {
> > @@ -20,6 +23,50 @@ static int proc_dointvec_minmax_sysadmin(const struct ctl_table *table, int writ
> > return proc_dointvec_minmax(table, write, buffer, lenp, ppos);
> > }
>
> The new way is to define the needed helpers using macros. We need this:
>
> static SYSCTL_USER_TO_KERN_INT_CONV(, SYSCTL_CONV_IDENTITY)
> static SYSCTL_KERN_TO_USER_INT_CONV(, SYSCTL_CONV_IDENTITY)
> static SYSCTL_INT_CONV_CUSTOM(, sysctl_user_to_kern_int_conv,
> sysctl_kern_to_user_int_conv, false)
>
>
> > +static int do_proc_dointvec_console_loglevel(bool *negp, unsigned long *lvalp,
> > + int *valp,
> > + int write, void *data)
>
> There parameters have got renamed and @table is passed instead or @data:
>
> static int do_proc_dointvec_console_loglevel(bool *negp, unsigned long *u_ptr,
> int *k_ptr, int dir,
> const struct ctl_table *table)
>
>
> > +{
> > + int level, ret;
> > +
> > + /*
> > + * If writing, first do so via a temporary local int so we can
> > + * bounds-check it before touching *valp.
> > + */
> > + int *intp = write ? &level : valp;
>
> The direction is newly checked by macro:
>
> int *int_ptr = SYSCTL_USER_TO_KERN(dir) ? &level : k_ptr;
>
> > + ret = do_proc_dointvec_conv(negp, lvalp, intp, write, data);
>
> The following helper is defined by the above mentioned macros:
>
> ret = do_proc_int_conv(negp, u_ptr, int_ptr, dir, table);
>
> > + if (ret)
> > + return ret;
> > +
> > + if (write) {
>
> The new way:
>
> if (SYSCTL_USER_TO_KERN(dir)) {
>
> > + if (level != console_clamp_loglevel(level))
> > + return -ERANGE;
> > +
> > + /*
> > + * Honour the administrator-configured minimum console
> > + * loglevel (third element of kernel.printk). This mirrors
> > + * the syslog() and sysfs control paths so that once the floor
> > + * is raised we do not let this sysctl silently bypass it.
> > + */
> > + if (minimum_console_loglevel > CONSOLE_LOGLEVEL_MIN &&
> > + level < minimum_console_loglevel)
> > + level = minimum_console_loglevel;
> > +
> > + WRITE_ONCE(*valp, level);
>
> New parameter name:
>
> WRITE_ONCE(*k_ptr, level);
>
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int proc_dointvec_console_loglevel(const struct ctl_table *table,
> > + int write, void *buffer, size_t *lenp,
> > + loff_t *ppos)
> > +{
> > + return do_proc_dointvec(table, write, buffer, lenp, ppos,
> > + do_proc_dointvec_console_loglevel, NULL);
>
> There is a new function where the last NULL parameter is not longer passed:
>
> return proc_dointvec_conv(table, write, buffer, lenp, ppos,
> do_proc_dointvec_console_loglevel);
>
> > +}
> > +
> > static const struct ctl_table printk_sysctls[] = {
> > {
> > .procname = "printk",
>
>
> Here are the above described changes made by diff:
The changes look ok, but there is a lot of context that I'm missing from
just looking at this change. Will there be another version re-based on
top of v19-rc1?
>
> --- a/kernel/printk/sysctl.c
> +++ b/kernel/printk/sysctl.c
> @@ -24,9 +24,14 @@ static int proc_dointvec_minmax_sysadmin(const struct ctl_table *table, int writ
> return proc_dointvec_minmax(table, write, buffer, lenp, ppos);
> }
>
> -static int do_proc_dointvec_console_loglevel(bool *negp, unsigned long *lvalp,
> - int *valp,
> - int write, void *data)
> +static SYSCTL_USER_TO_KERN_INT_CONV(, SYSCTL_CONV_IDENTITY)
> +static SYSCTL_KERN_TO_USER_INT_CONV(, SYSCTL_CONV_IDENTITY)
> +static SYSCTL_INT_CONV_CUSTOM(, sysctl_user_to_kern_int_conv,
> + sysctl_kern_to_user_int_conv, false)
> +
> +static int do_proc_dointvec_console_loglevel(bool *negp, unsigned long *u_ptr,
> + int *k_ptr, int dir,
> + const struct ctl_table *table)
> {
> int level, ret;
>
> @@ -34,13 +39,13 @@ static int do_proc_dointvec_console_loglevel(bool *negp, unsigned long *lvalp,
> * If writing, first do so via a temporary local int so we can
> * bounds-check it before touching *valp.
> */
> - int *intp = write ? &level : valp;
> + int *int_ptr = SYSCTL_USER_TO_KERN(dir) ? &level : k_ptr;
>
> - ret = do_proc_dointvec_conv(negp, lvalp, intp, write, data);
> + ret = do_proc_int_conv(negp, u_ptr, int_ptr, dir, table);
> if (ret)
> return ret;
>
> - if (write) {
> + if (SYSCTL_USER_TO_KERN(dir)) {
> if (level != console_clamp_loglevel(level))
> return -ERANGE;
>
> @@ -54,7 +59,7 @@ static int do_proc_dointvec_console_loglevel(bool *negp, unsigned long *lvalp,
> level < minimum_console_loglevel)
> level = minimum_console_loglevel;
>
> - WRITE_ONCE(*valp, level);
> + WRITE_ONCE(*k_ptr, level);
> }
>
> return 0;
> @@ -64,8 +69,8 @@ static int proc_dointvec_console_loglevel(const struct ctl_table *table,
> int write, void *buffer, size_t *lenp,
> loff_t *ppos)
> {
> - return do_proc_dointvec(table, write, buffer, lenp, ppos,
> - do_proc_dointvec_console_loglevel, NULL);
> + return proc_dointvec_conv(table, write, buffer, lenp, ppos,
> + do_proc_dointvec_console_loglevel);
> }
>
> static int proc_dointvec_printk_deprecated(const struct ctl_table *table, int write,
>
>
> > void __init printk_sysctl_init(void)
> > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > index cb6196e3fa99..3ed010b8f6b3 100644
> > --- a/kernel/sysctl.c
> > +++ b/kernel/sysctl.c
> > @@ -354,9 +354,9 @@ static void proc_put_char(void **buf, size_t *size, char c)
> > }
> > }
> >
> > -static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp,
> > - int *valp,
> > - int write, void *data)
> > +int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp,
> > + int *valp,
> > + int write, void *data)
> > {
> > if (write) {
> > if (*negp) {
> > @@ -380,6 +380,7 @@ static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp,
> > }
> > return 0;
> > }
> > +EXPORT_SYMBOL(do_proc_dointvec_conv);
> >
> > static int do_proc_douintvec_conv(unsigned long *lvalp,
> > unsigned int *valp,
> > @@ -471,15 +472,16 @@ static int __do_proc_dointvec(void *tbl_data, const struct ctl_table *table,
> > return err;
> > }
> >
> > -static int do_proc_dointvec(const struct ctl_table *table, int write,
> > - void *buffer, size_t *lenp, loff_t *ppos,
> > - int (*conv)(bool *negp, unsigned long *lvalp, int *valp,
> > - int write, void *data),
> > - void *data)
> > +int do_proc_dointvec(const struct ctl_table *table, int write,
> > + void *buffer, size_t *lenp, loff_t *ppos,
> > + int (*conv)(bool *negp, unsigned long *lvalp, int *valp,
> > + int write, void *data),
> > + void *data)
> > {
> > return __do_proc_dointvec(table->data, table, write,
> > buffer, lenp, ppos, conv, data);
> > }
> > +EXPORT_SYMBOL(do_proc_dointvec);
> >
> > static int do_proc_douintvec_w(unsigned int *tbl_data,
> > const struct ctl_table *table,
>
> Also all these changes in kernel/sysctl.c are not loger needed.
>
> Best Regards,
> Petr
--
Joel Granados
Download attachment "signature.asc" of type "application/pgp-signature" (660 bytes)
Powered by blists - more mailing lists