linux-kernel - Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

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: <CA+ekxPX7c3Dd4Pv89kOvMkVnEzo_1+4UBaERA6z9ztFdes-mDA@mail.gmail.com>
Date:	Thu, 28 Jan 2016 13:48:56 -0700
From:	Jeffrey Merkey <jeffmerkey@...il.com>
To:	Chris Metcalf <cmetcalf@...hip.com>
Cc:	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	dzickus@...hat.com, uobergfe@...hat.com, atomlin@...hat.com,
	mhocko@...e.cz, fweisbec@...il.com, tj@...nel.org,
	hidehiro.kawai.ez@...achi.com, tglx@...utronix.de
Subject: Re: [PATCH 01/31] Add hard/soft lockup debugger entry points

On 1/28/16, Chris Metcalf <cmetcalf@...hip.com> wrote:
> On 01/28/2016 02:46 PM, Jeffrey Merkey wrote:
>> This patch series adds an export which can be set by system debuggers to
>> direct the hard lockup and soft lockup detector to trigger a breakpoint
>> exception and enter a debugger if one is active.  It is assumed that if
>> someone sets this variable, then an breakpoint handler of some sort will
>> be actively loaded or registered via the notify die handler chain.
>>
>> This addition is extremely useful for debugging hard and soft lockups
>> real time and quickly from a console debugger.
>
> I'm concerned that you are duplicating the breakpoint instructions
> for all the platforms.  Could you make kgdb.h include kdebug.h and
> just move the arch_kgdb_breakpoint() implementations in kgdb.h to
> arch_breakpoint() in kdebug.h?  Then each platform can just put an
> appropriate define in kgdb.h, e.g. "#define arch_kgdb_breakpoint
> arch_breakpoint", unless (like mips) they have a more complicated
> requirement.
>
> I'm concerned that in some cases (e.g. arm64) there is a perfectly good
> breakpoint defined in kgdb.h but you are providing a no-op in kdebug.h.
> You should probably do another check across all the architectures for
> this case.
>
> You should probably add your no-op implementation of arch_breakpoint()
> to asm-generic/kdebug.h, and then add "generic-y" lines to the Kbuild
> files for the architectures that you are creating new empty files for.
> I'm a little ambivalent about the "silent no-op" implementation, but I'm
> not really sure there's a better option.
>
> For mips, I'm pretty sure you don't want to create a global "breakinst"
> symbol every time you insert a breakpoint into code.  I think this is an
> example of where you need to have a different implementation of
> arch_breakpoint() and arch_kgdb_breakpoint(), since mips makes its
> breakpoint magical by knowing what the address used for that specific
> instruction is, and you can't do that for arch_breakpoint().
>
> As a general rule, you probably want to provide header guards in new
> headers that you create, but if you just use asm-generic instead, it
> actually won't matter for this case.
>
> I should add that I didn't do a thorough review of the patch series,
> just a quick skim of a few of the architectures.
>
> --
> Chris Metcalf, EZChip Semiconductor
> http://www.ezchip.com
>
>

todo:

* Adding header guards (if file does not exist)
* reimplemeting arch_breakpoint
* move arch_breakpoint to asm-generic/kdebug.h
* implement as a macro if possible
* study MIPS breakpoint instruction.  Ask for help is not sure.
* fix syntax for INT3 instruction for x86 to use proper gas semantics
* fix kbuild test robot error for SPARC (move to asm-generic fixes)

List so far.

Jeff