linux-kernel - Re: [PATCH V4 6/6] x86/Hyper-V: Report crash data in die() when panic_on

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, 1 Apr 2020 19:53:36 +0100
From:   Wei Liu <wei.liu@...nel.org>
To:     Tianyu Lan <ltykernel@...il.com>
Cc:     Michael Kelley <mikelley@...rosoft.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Wei Liu <liuwe@...rosoft.com>,
        Tianyu Lan <Tianyu.Lan@...rosoft.com>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        vkuznets <vkuznets@...hat.com>, Wei Liu <wei.liu@...nel.org>
Subject: Re: [PATCH V4 6/6] x86/Hyper-V: Report crash data in die() when
 panic_on_oops is set

On Tue, Mar 31, 2020 at 10:26:06PM +0800, Tianyu Lan wrote:
> 
> 
> On 3/31/2020 9:51 PM, Michael Kelley wrote:
> > From: Tianyu Lan <Tianyu.Lan@...rosoft.com> Sent: Tuesday, March 31, 2020 12:39 AM
> > > 
> > > When oops happens with panic_on_oops unset, the oops
> > > thread is killed by die() and system continues to run.
> > > In such case, guest should not report crash register
> > > data to host since system still runs. Fix it.
> > > 
> > > Reviewed-by: Michael Kelley <mikelley@...rosoft.com>
> > > Signed-off-by: Tianyu Lan <Tianyu.Lan@...rosoft.com>
> > > ---
> > > Change since v3:
> > > 	Fix compile error
> > > ---
> > >   drivers/hv/vmbus_drv.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > > index 172ceae69abb..4bc02aea2098 100644
> > > --- a/drivers/hv/vmbus_drv.c
> > > +++ b/drivers/hv/vmbus_drv.c
> > > @@ -31,6 +31,7 @@
> > >   #include <linux/kdebug.h>
> > >   #include <linux/efi.h>
> > >   #include <linux/random.h>
> > > +#include <linux/kernel.h>
> > 
> > Unfortunately, adding the #include doesn't solve the problem.  The error occurs when
> > CONFIG_HYPERV=m, because panic_on_oops is not exported.  I haven't thought it
> > through, but hopefully there's a solution where panic_on_oops can be tested in
> > hyperv_report_panic() or some other Hyper-V specific function that's never in a
> > module, so that we don't need to export panic_on_oops.
> 
> Yes, I don't consider modules case. I think we may introduce a check
> function of panic_on_oops in the mshyperv.c and expose it to module.
> 

Why expose something new? You can just test panic_on_oops in
hyperv_report_panic and bail if it is false, right?

Something like the following (not compiled) diff:

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index b0da5320bcff..0dc229a9142c 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -424,6 +424,9 @@ void hyperv_report_panic(struct pt_regs *regs, long err)
        static bool panic_reported;
        u64 guest_id;

+       if (!panic_on_oops)
+               return;
+
        /*
         * We prefer to report panic on 'die' chain as we have proper
         * registers to report, but if we miss it (e.g. on BUG()) we need

I haven't checked all the error reporting paths and don't know if this
works or not though.

Wei.