linux-kernel - Re: [PATCH 21/31] Add debugger entry points for MIPS

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: <CAO6TR8U1m+7ALr8kbrWp8qkH0NsAVnaj4edyYTfOa+BJ22gDtA@mail.gmail.com>
Date:	Mon, 1 Feb 2016 09:03:54 -0700
From:	Jeff Merkey <linux.mdb@...il.com>
To:	"Maciej W. Rozycki" <macro@...tec.com>
Cc:	Jeffrey Merkey <jeffmerkey@...il.com>,
	linux-kernel@...r.kernel.org, Ralf Baechle <ralf@...ux-mips.org>,
	James Hogan <james.hogan@...tec.com>, linux-mips@...ux-mips.org
Subject: Re: [PATCH 21/31] Add debugger entry points for MIPS

On 2/1/16, Maciej W. Rozycki <macro@...tec.com> wrote:
> On Thu, 28 Jan 2016, 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.
>
>  What's the intended use case for this hook and what do you call a console
> debugger?
>
>  I'm asking because from the debugging perspective the Linux kernel is a
> bare metal application and for such the BREAK instruction is not the usual
> choice on the MIPS target.  This instruction is normally used for userland
> debugging, it traps to the kernel and is handled there.  Furthermore there
> are a few other applications of this instruction defined as a part of the
> MIPS ABI, also handled by the kernel, determined by the breakpoint code
> embedded with the instruction word, e.g. to trap integer division errors.
> See arch/mips/include/uapi/asm/break.h for the codes defined so far.
>
>  All this means the trap may not be appropriate for debugging the kernel
> itself as you don't want to intercept it or the system won't run
> correctly.  You'd have to hook into the kernel itself to intercept it too.
>
>  But if you do have a working debug environment already set up around this
> arrangement, then it might be fine after all.  In that case I think using
> a non-zero breakpoint code would make sense though, as 0 is often treated
> as a random (spurious) trap (it was used by IRIX though).  Or do you
> detect it by the symbol name somehow?
>
>  I've got a couple of further notes on the patch itself below.
>
>> diff --git a/arch/mips/include/asm/kdebug.h
>> b/arch/mips/include/asm/kdebug.h
>> index 8e3d08e..af5999e 100644
>> --- a/arch/mips/include/asm/kdebug.h
>> +++ b/arch/mips/include/asm/kdebug.h
>> @@ -16,4 +16,16 @@ enum die_val {
>>  	DIE_UPROBE_XOL,
>>  };
>>
>> +
>> +void arch_breakpoint(void)
>> +{
>> +	__asm__ __volatile__(
>> +		".globl breakinst\n\t"
>
>  Please keep formatting consistent -- the rest of code below uses `\t' as
> the separator, so use it here as well.  We don't have an established
> inline assembly formatting style, so please just keep your chosen one
> consistent.
>
>> +		".set\tnoreorder\n\t"
>> +		"nop\n"
>> +		"breakinst:\tbreak\n\t"
>> +		"nop\n\t"
>> +		".set\treorder");
>> +}
>> +
>>  #endif /* _ASM_MIPS_KDEBUG_H */
>
>  Why do you need these NOPs around the breakpoint?  You also need to mark
> `breakinst' as a function symbol (`.aent' might do) or otherwise you'll
> get garbled disassembly if this is built as microMIPS code.
>
>   Maciej
>

You can drop this patch series, I am rethinking a better way to do this.

Jeff