linux-kernel - Re: [PATCH] debug: Allow forcing entering debug mode on panic/exception

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] [day] [month] [year] [list]

Message-ID: <CAD=FV=U5eQWh40vru1H8tfd37e8kZiwsd9zSVWy-ikk9b3PCKA@mail.gmail.com>
Date:   Tue, 18 Dec 2018 15:53:26 -0800
From:   Doug Anderson <dianders@...omium.org>
To:     Daniel Thompson <daniel.thompson@...aro.org>
Cc:     Jason Wessel <jason.wessel@...driver.com>,
        Brian Norris <briannorris@...omium.org>,
        kgdb-bugreport@...ts.sourceforge.net,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] debug: Allow forcing entering debug mode on panic/exception

Hi,

On Tue, Dec 18, 2018 at 9:05 AM Daniel Thompson
<daniel.thompson@...aro.org> wrote:
>
> On Sun, Dec 09, 2018 at 06:36:49PM -0800, Douglas Anderson wrote:
> > Ever since commit 5516fd7b92a7 ("debug: prevent entering debug mode on
> > panic/exception.") (yes, years ago) my kgdb workflow has been broken.
> > On Chrome OS we have 'kernel.panic = -1' in
> > '/etc/sysctl.d/00-sysctl.conf'.  That means that when userspace starts
> > up it will tell the kernel "please reboot on panic".  ...and so when I
> > get a panic then the system reboots instead of letting me debug it.
> >
> > While I could go in an change the 'sysctl.conf' and I could go in and
> > hack the kernel myself, these things are inconvenient.  I either need
> > to keep a private kernel patch or or remember to edit a file every
> > time I install an updated version of Chrome OS.  What is convenient
> > (for me) is to have a CONFIG option that makes kgdb override the panic
> > request.  This is because the Chrome OS build system makes it very
> > easy for me to add some extra CONFIG "fragments" to my debug kernels.
> >
> > Hopefully having this extra config option is OK and useful to others
> > who would also prefer to make sure that kgdb is always entered on a
> > panic no matter what userspace might request.
> >
> > Signed-off-by: Douglas Anderson <dianders@...omium.org>
>
> Sorry to be late with this review. I forgot to search for "debug:" when
> I was checking for missed patches earlier.
>
> Mind you... one of the reasons I deferred review when you first sent it
> in was that my gut reaction was "I don't like it" so I decided to wait
> until I could offer a head reaction instead.
>
> Ultimately I'm not sure this should be solved within kgdb. Perhaps best
> phrased as: is the problem that kgdb *misinterprets* panic_timeout or is
> the problem that Doug wants to *override* panic_timeout?
>
> I think the answer to this question is the later meaning I'm interested
> in what happens if you introduce a CONFIG_PANIC_TIMEOUT_FORCE (c.f.
> CONFIG_CMDLINE_FORCE) to prevent the userspace changing the
> panic_timeout (either by avoiding registering the panic sysctl or, if
> that is a huge ABI problem attaching it to a different variable).
>
> TBH I'm not sure if such a patch would be accepted but I think it makes
> more semantic sense!
>
> (there is a small review comment below but the above is more important)

Thanks for the review.  Yeah, it definitely was a bit of a
questionable patch but I figured I'd throw it out there to see what
folks thought.  I think we should just drop it.  I talked with Brian
about this offline and we agree that it actually should be OK to just
drop the setting from '00-sysctl.conf'.  I have my patch at
crrev.com/c/1382879 and it looks like folks are happy with it.  :-)


-Doug