linux-kernel - Re: [RFC][PATCHv2 7/8] printk: add printk emergency

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20170403152921.GF3452@pathway.suse.cz>
Date:   Mon, 3 Apr 2017 17:29:21 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>, Jan Kara <jack@...e.cz>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Eric Biederman <ebiederm@...ssion.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>, Pavel Machek <pavel@....cz>,
        Len Brown <len.brown@...el.com>, linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Subject: Re: [RFC][PATCHv2 7/8] printk: add printk emergency_mode parameter

On Wed 2017-03-29 18:25:10, Sergey Senozhatsky wrote:
> This param permits user-space to forcibly on/off printk emergency
> mode via /sys/module/printk/parameters/emergency_mode node.
> 
> We have annotated sections in the kernel that switch printk to
> emergency, but there might be places/cases when user space would
> want to have printk operate in emergency mode all the time.

Thanks a lot for the parameter.
 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> ---
>  kernel/printk/printk.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 1927b5cb5cbe..0d96839bb450 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -455,7 +455,7 @@ static struct task_struct *printk_kthread __read_mostly;
>  static atomic_t printk_emergency __read_mostly;
>  /*
>   * Disable printk_kthread permanently. Unlike `oops_in_progress'
> - * it doesn't go back to 0.
> + * it doesn't go back to 0 (unless enforced by user-space).
>   */
>  static bool printk_kthread_disabled __read_mostly;
>  
> @@ -483,6 +483,24 @@ void printk_emergency_end(void)
>  	atomic_dec(&printk_emergency);
>  }
>  
> +static int printk_kthread_disabled_set(const char *val,
> +					const struct kernel_param *kp)
> +{
> +	return param_set_bool(val, kp);
> +}
> +
> +static const struct kernel_param_ops printk_kthread_disabled_ops = {
> +	.set = printk_kthread_disabled_set,
> +	.get = param_get_bool,
> +};
> +
> +module_param_cb(emergency_mode,
> +		&printk_kthread_disabled_ops,
> +		&printk_kthread_disabled,
> +		0644);
> +MODULE_PARM_DESC(emergency_mode,
> +		"don't offload message printing to printk kthread");

I wonder if we could make this easier. Something like:

static bool printk_force_emergency;
module_param_named(force_emergency, printk_force_emergency,
		   bool, S_IRUGO | S_IWUSR);

and use it instead of printk_kthread_disabled variable. It was
confusing anyway. You already mentioned that it did not
stop the kthread.

Also the relation between the sysfs entry and printk code
will be cleaner. People might thing that emergency_mode
shows the immediate value of printk_emergency variable.

Best Regards,
Petr