Re: [PATCH v8 18/21] printk: Deconstruct kernel.printk into discrete sysctl controls

[Petr Mladek <pmladek@suse.com>]

Adding Joel into Cc. Joel, see the original patch at
https://lore.kernel.org/all/c3e5cc507eb3fd7db0a002f31d7e47d764cad176.1764272407.git.chris@chrisdown.name/

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@suse.com>
> Tested-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Chris Down <chris@chrisdown.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:

--- 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