linux-kernel - Re: [RESEND PATCH] arm64: kgdb: fix single stepping

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: <20160915075617.GJ16712@linaro.org>
Date:   Thu, 15 Sep 2016 16:56:18 +0900
From:   AKASHI Takahiro <takahiro.akashi@...aro.org>
To:     Will Deacon <will.deacon@....com>
Cc:     "jason.wessel@...driver.com" <jason.wessel@...driver.com>,
        Catalin Marinas <Catalin.Marinas@....com>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "yong.zhao@....com" <yong.zhao@....com>,
        "Vijaya.Kumar@...iumnetworks.com" <Vijaya.Kumar@...iumnetworks.com>,
        "kgdb-bugreport@...ts.sourceforge.net" 
        <kgdb-bugreport@...ts.sourceforge.net>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linaro-kernel@...ts.linaro.org" <linaro-kernel@...ts.linaro.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH] arm64: kgdb: fix single stepping

On Wed, Sep 14, 2016 at 03:58:51PM +0100, Will Deacon wrote:
> Hi Akashi,
> 
> On Tue, Apr 21, 2015 at 02:13:13AM +0100, AKASHI Takahiro wrote:
> > Could you please review my patch below?
> > See also arm64 maintainer's comment:
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/313712.html
> 
> -ETIMEDOUT waiting for the kdgb folk to comment. Ppeople have reported
> that this patch is required for kgdb to work correctly on arm64, so I'm
> happy to merge it.

I'm happy, too.

> However, as detailed in your comment log:
> 
> > This patch
> > (1) moves kgdb_disable_single_step() from 'c' command handling to single
> >     step handler.
> >     This makes sure that single stepping gets effective at every 's' command.
> >     Please note that, under the current implementation, single step bit in
> >     spsr, which is cleared by the first single stepping, will not be set
> >     again for the consecutive 's' commands because single step bit in mdscr
> >     is still kept on (that is, kernel_active_single_step() in
> >     kgdb_arch_handle_exception() is true).
> > (2) re-implements kgdb_roundup_cpus() because the current implementation
> >     enabled interrupts naively. See below.
> > (3) removes 'enable_dbg' in el1_dbg.
> >     Single step bit in mdscr is turned on in do_handle_exception()->
> >     kgdb_handle_expection() before returning to debugged context, and if
> >     debug exception is enabled in el1_dbg, we will see unexpected single-
> >     stepping in el1_dbg.
> >     Since v3.18, the following patch does the same:
> >       commit 1059c6bf8534 ("arm64: debug: don't re-enable debug exceptions
> >       on return from el1_dbg)
> > (4) masks interrupts while single-stepping one instruction.
> >     If an interrupt is caught during processing a single-stepping, debug
> >     exception is unintentionally enabled by el1_irq's 'enable_dbg' before
> >     returning to debugged context.
> >     Thus, like in (2), we will see unexpected single-stepping in el1_irq.
> 
> this patch is doing *far* too much in one go. Could you please repost it
> as a series of self-contained fixes with clear commit messages, so I can
> queue them and cc stable where appropriate?

Sure, but I need to refresh my memory here.

-Takahiro AKASHI

> Thanks,
> 
> Will