linux-kernel - Re: [PATCH] arm64: Call debug_traps_init() from trap

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: <20200515162316.GB23334@willie-the-truck>
Date:   Fri, 15 May 2020 17:23:17 +0100
From:   Will Deacon <will@...nel.org>
To:     Douglas Anderson <dianders@...omium.org>
Cc:     Jason Wessel <jason.wessel@...driver.com>,
        Daniel Thompson <daniel.thompson@...aro.org>,
        kgdb-bugreport@...ts.sourceforge.net, liwei391@...wei.com,
        Catalin Marinas <catalin.marinas@....com>,
        sumit.garg@...aro.org, Alexios Zavras <alexios.zavras@...el.com>,
        Allison Randal <allison@...utok.net>,
        Dave Martin <Dave.Martin@....com>,
        Enrico Weigelt <info@...ux.net>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        James Morse <james.morse@....com>,
        Mark Rutland <mark.rutland@....com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Zenghui Yu <yuzenghui@...wei.com>,
        jinho lim <jordan.lim@...sung.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64: Call debug_traps_init() from trap_init() to help
 early kgdb

On Wed, May 13, 2020 at 04:06:37PM -0700, Douglas Anderson wrote:
> A new kgdb feature will soon land (kgdb_earlycon) that lets us run
> kgdb much earlier.  In order for everything to work properly it's
> important that the break hook is setup by the time we process
> "kgdbwait".
> 
> Right now the break hook is setup in debug_traps_init() and that's
> called from arch_initcall().  That's a bit too late since
> kgdb_earlycon really needs things to be setup by the time the system
> calls dbg_late_init().
> 
> We could fix this by adding call_break_hook() into early_brk64() and
> that works fine.  However, it's a little ugly.  Instead, let's just
> add a call to debug_traps_init() straight from trap_init().  There's
> already a documented dependency between trap_init() and
> debug_traps_init() and this makes the dependency more obvious rather
> than just relying on a comment.
> 
> NOTE: this solution isn't early enough to let us select the
> "ARCH_HAS_EARLY_DEBUG" KConfig option that is introduced by the
> kgdb_earlycon patch series.  That would only be set if we could do
> breakpoints when early params are parsed.  This patch only enables
> "late early" breakpoints, AKA breakpoints when dbg_late_init() is
> called.  It's expected that this should be fine for most people.
> 
> It should also be noted that if you crash you can still end up in kgdb
> earlier than debug_traps_init().  Since you don't need breakpoints to
> debug a crash that's fine.
> 
> Suggested-by: Will Deacon <will@...nel.org>
> Signed-off-by: Douglas Anderson <dianders@...omium.org>
> Cc: Catalin Marinas <catalin.marinas@....com>
> Cc: Will Deacon <will@...nel.org>
> ---
> This replaces the patch ("arm64: Add call_break_hook() to
> early_brk64() for early kgdb") in my recent kgdb series [1].  If I end
> up re-posting that series again I'll include this patch as a
> replacement, but I'm sending it separately to avoid spamming a pile of
> people another time with a 12-patch series.
> 
> Note that, because it doesn't select the "ARCH_HAS_EARLY_DEBUG"
> KConfig option it could be landed standalone.  However, it's still
> probably better to land together with that patch series.
> 
> If the kgdb_earlycon patch series lands without this patch then
> kgdbwait + kgdb_earlycon won't work well on arm64, but there would be
> no other bad side effects.
> 
> If this patch lands without the kgdb_earlycon patch series then there
> will be no known problems.
> 
> [1] https://lore.kernel.org/r/20200507130644.v4.5.I22067ad43e77ddfd4b64c2d49030628480f9e8d9@changeid
> 
>  arch/arm64/include/asm/debug-monitors.h | 2 ++
>  arch/arm64/kernel/debug-monitors.c      | 4 +---
>  arch/arm64/kernel/traps.c               | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)

[...]

Acked-by: Will Deacon <will@...nel.org>

I would prefer to take this via arm64, if possible, since we have quite lot
going in for 5.8, although I don't think this conflicts at the moment.

Daniel -- what do you want to do?

Will