linux-kernel - Re: [PATCH] print kdump kernel loaded status in stack dump

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]

Date:   Wed, 17 Jan 2018 20:32:44 +0800
From:   Dave Young <dyoung@...hat.com>
To:     Petr Mladek <pmladek@...e.com>
Cc:     sergey.senozhatsky@...il.com, rostedt@...dmis.org,
        linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
        kexec@...ts.infradead.org
Subject: Re: [PATCH] print kdump kernel loaded status in stack dump

Hi,

Thanks for your comments.
On 01/17/18 at 09:57am, Petr Mladek wrote:
> On Wed 2018-01-17 12:50:57, Dave Young wrote:
> > It is useful to print kdump kernel loaded status in dump_stack() 
> > especially when panic happens so that we can  differenciate 
> > kdump kernel early hang and a normal panic in a bug report.
> >
> > Signed-off-by: Dave Young <dyoung@...hat.com>
> > ---
> >  kernel/printk/printk.c |    3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > --- linux-x86.orig/kernel/printk/printk.c
> > +++ linux-x86/kernel/printk/printk.c
> > @@ -48,6 +48,7 @@
> >  #include <linux/sched/clock.h>
> >  #include <linux/sched/debug.h>
> >  #include <linux/sched/task_stack.h>
> > +#include <linux/kexec.h>
> >  
> >  #include <linux/uaccess.h>
> >  #include <asm/sections.h>
> > @@ -3127,6 +3128,8 @@ void dump_stack_print_info(const char *l
> >  	if (dump_stack_arch_desc_str[0] != '\0')
> >  		printk("%sHardware name: %s\n",
> >  		       log_lvl, dump_stack_arch_desc_str);
> > +	if (kexec_crash_loaded())
> > +		printk("%skdump kernel loaded\n", log_lvl);
> 
> IMHO, it would be better to do it like for the workqueues.
> I mean to call printk_kexec_info(log_lv1, current) here
> that would be impletemented in kexec sources.
> Then it could be maintained by kexec people.
> 
> Anyway, I wonder if the info about kexec_crash_loaded() is
> enough. I am not much familiar with kexec. AFAIK,
> the image might be loaded long time before it
> is acutally used.

kexec_crash_loaded is enough, we only care if kdump kernel being
loaded or not, nothing else, no matter how long it has been loaded.
In Fedora/RHEL a kdump service takes care of loading the kernel but
it runs after networking is ready.  If people want to save
the vmcore to nfs/ssh then we need detect network and build the
initramfs. In the nfs/ssh case if some networking code panicked it
is possible that kdump service has not started, but sometimes bug
can not be easily reproduced thus nobody can know if kdump is active
or not.

Since kexec_crash_loaded() is already in kexec souce code, and it
is the only thing need to know, do you think it is really necessary
to add a printk_kexec_info()?  I can do it if you strongly suggest
to do so.

> 
> Finally, the style of the other lines is:
> 
>     Name: details
> 
> I would suggest to print something like:
> 
>     Kexec: details
> 
> , where the details might be whether the image is loaded,
> whether the loaded kernel is being executed, and
> other kexec-related flags.

Will do, it can be something like:
Kexec: kdump kernel loaded

> 
> How does that sound?
> 
> >  	print_worker_info(log_lvl, current);
> >  }
> 
> Best Regards,
> Petr

Thanks
Dave