linux-kernel - Re: [RFC Patch 1/9] Introducing generic hardware breakpoint handler interfaces

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: <20081007164932.GB4130@in.ibm.com>
Date:	Tue, 7 Oct 2008 22:19:32 +0530
From:	"K.Prasad" <prasad@...ux.vnet.ibm.com>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	linux-kernel@...r.kernel.org, Roland McGrath <roland@...hat.com>,
	akpm@...ux-foundation.org, mingo@...e.hu,
	jason.wessel@...driver.com, avi@...ranet.com,
	richardj_moore@...ibm.com
Subject: Re: [RFC Patch 1/9] Introducing generic hardware breakpoint
	handler interfaces

On Tue, Oct 07, 2008 at 11:21:53AM -0400, Alan Stern wrote:
> On Tue, 7 Oct 2008, K.Prasad wrote:
> 
> > This patch introduces two new files hw_breakpoint.[ch] which defines the 
> > generic interfaces to use hardware breakpoint infrastructure of the system. 
> 
> ...
> > --- /dev/null
> > +++ linux-bkpt-lkml-27-rc9/include/asm-generic/hw_breakpoint.h
> > @@ -0,0 +1,236 @@
> > +#ifndef	_ASM_GENERIC_HW_BREAKPOINT_H
> > +#define	_ASM_GENERIC_HW_BREAKPOINT_H
> > +
> > +#ifndef __ARCH_HW_BREAKPOINT_H
> > +#error "Please don't include this file directly"
> > +#endif
> > +
> > +#ifdef	__KERNEL__
> > +#include <linux/list.h>
> > +#include <linux/types.h>
> > +
> > +/**
> > + * struct hw_breakpoint - unified kernel/user-space hardware breakpoint
> > + * @node: internal linked-list management
> > + * @triggered: callback invoked when the breakpoint is hit
> 
> The kerneldoc needs to be updated to match the structure.  @triggered
> doesn't exist any more; instead there are @pre_handler and
> @post_handler.
> 
> ...
> > + * When a breakpoint gets hit, the @triggered callback is invoked
> > + * in_interrupt with a pointer to the %hw_breakpoint structure and the
> > + * processor registers.  Execute-breakpoint traps occur before the
> > + * breakpointed instruction runs; when the callback returns the
> > + * instruction is restarted (this time without a debug exception).  All
> > + * other types of trap occur after the memory access has taken place.
> > + * Breakpoints are disabled while @triggered runs, to avoid recursive
> > + * traps and allow unhindered access to breakpointed memory.
> 
> This paragraph needs to be fixed as well.
> 
> ...
> > +/*
> > + * The following routines help the user determine if the architecture supports
> > + * a trigger-before access/execution mechanism or a trigger-after
> > + * access/execution. This is dependent on the type of breakpoint and the
> > + * architecture
> > + */
> > +int trigger_before_arch(unsigned type);
> > +int trigger_after_arch(unsigned type);
> 
> These routines should have hwbp_ in their name (or something similar).  
> And they don't need to have _arch.
> 
> Alan Stern
>
I agree this part of the patch needs cleanup. Will do so during the next
iteration of the patchset. trigger_<before/after>_arch() routines are
rendered redundant through the pre_ and post_handler_allowed() routines
and I will remove the above two lines too.

Thanks,
K.Prasad
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/