linux-kernel - Re: [PATCH v3 5/5] panic: add note that panic

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aJ3-5mSqEu4kegx4@pathway>
Date: Thu, 14 Aug 2025 17:21:10 +0200
From: Petr Mladek <pmladek@...e.com>
To: Feng Tang <feng.tang@...ux.alibaba.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Lance Yang <lance.yang@...ux.dev>, Jonathan Corbet <corbet@....net>,
	linux-kernel@...r.kernel.org, paulmck@...nel.org,
	john.ogness@...utronix.de
Subject: Re: [PATCH v3 5/5] panic: add note that panic_print sysctl interface
 is deprecated

On Wed 2025-08-13 09:05:30, Feng Tang wrote:
> On Tue, Aug 12, 2025 at 01:52:58PM +0200, Petr Mladek wrote:
> > On Thu 2025-07-03 10:10:04, Feng Tang wrote:
> > > Add a dedicated core parameter 'panic_console_replay' for controlling
> > > console replay, and add note that 'panic_print' sysctl interface  will
> > > be obsoleted by 'panic_sys_info' and 'panic_console_replay'.  When it
> > > happens, the SYS_INFO_PANIC_CONSOLE_REPLAY can be removed as well.
> > > 
> > > --- a/kernel/panic.c
> > > +++ b/kernel/panic.c
> > > @@ -77,6 +78,13 @@ ATOMIC_NOTIFIER_HEAD(panic_notifier_list);
> > >  EXPORT_SYMBOL(panic_notifier_list);
> > >  
> > >  #ifdef CONFIG_SYSCTL
> > > +static int sysctl_panic_print_handler(const struct ctl_table *table, int write,
> > > +			   void *buffer, size_t *lenp, loff_t *ppos)
> > > +{
> > > +	pr_info_once("Kernel: 'panic_print' sysctl interface will be obsoleted by both 'panic_sys_info' and 'panic_console_replay'\n");
> > > +	return proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
> > > +}
> > 
> > This warning is printed "only" when the value is accessed via the
> > procfs. It would be great to print it also when it is set
> > via the command line parameter.
> 
> Yes, this is indeed a remaining issue to be solved, as mentioned in
> the cover letter.

I see now.

> > It would require replacing
> > 
> > core_param(panic_print, panic_print, ulong, 0644);
> > 
> > with
> > 
> > core_param_cb(panic_print, &panic_print_ops, &panic_print, 0644);
> 
> When testing the change, I found  a problem:  'core_param_cb' is not
> the real counterpart of 'core_param', that it is a module parameter
> instead of kernel/core parameter, and adds the module.prefix to the
> parameter, say, the effective cmdline parameter is changed to
> 'panic.panic_print=' instead of 'panic_print='.

I see. It is pity that it is messed like this.

> While below patch of adding a new 'kernel_param_cb' can work without
> the "panic" prefix, but I'm not sure if it is worth the change:

I think that it is worth adding. IMHO, the parameter will primary be used
from the command line. So, this is an important path how to make people
aware of the obsoleting.

> ---
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index bfb85fd13e1f..71053d078cea 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -194,6 +194,9 @@ struct kparam_array
>  #define core_param_cb(name, ops, arg, perm)		\
>  	__level_param_cb(name, ops, arg, perm, 1)
>  
> +#define kernel_param_cb(name, ops, arg, perm) \
> +	__module_param_call("", name, ops, arg, perm, -1, 0)
> +

I would call it __core_param_cb(). And I move the definition
down to the section where core_param() and core_param_unsafe()
are defined. Also it would deserve a comment explaining
why the "__" prefix is used.

>  /**
>   * postcore_param_cb - general callback for a module/cmdline parameter
>   *                     to be evaluated before postcore initcall level

Best Regards,
Petr