linux-kernel - Re: [PATCH 2/5] perf core: Set event's default overflow

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: <20160323181348.GA2149@arm.com>
Date:	Wed, 23 Mar 2016 18:13:49 +0000
From:	Will Deacon <will.deacon@....com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Wang Nan <wangnan0@...wei.com>, mingo@...hat.com,
	linux-kernel@...r.kernel.org, He Kuang <hekuang@...wei.com>,
	Alexei Starovoitov <ast@...nel.org>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Brendan Gregg <brendan.d.gregg@...il.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Namhyung Kim <namhyung@...nel.org>,
	Zefan Li <lizefan@...wei.com>, pi3orama@....com
Subject: Re: [PATCH 2/5] perf core: Set event's default overflow_handler

On Wed, Mar 23, 2016 at 06:50:21PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 14, 2016 at 09:59:42AM +0000, Wang Nan wrote:
> > +++ b/arch/arm/kernel/hw_breakpoint.c
> > @@ -631,7 +631,7 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp)
> >  	info->address &= ~alignment_mask;
> >  	info->ctrl.len <<= offset;
> >  
> > -	if (!bp->overflow_handler) {
> > +	if (is_default_overflow_handler(bp)) {
> >  		/*
> >  		 * Mismatch breakpoints are required for single-stepping
> >  		 * breakpoints.
> > @@ -754,7 +754,7 @@ static void watchpoint_handler(unsigned long addr, unsigned int fsr,
> >  		 * mismatch breakpoint so we can single-step over the
> >  		 * watchpoint trigger.
> >  		 */
> > -		if (!wp->overflow_handler)
> > +		if (is_default_overflow_handler(wp))
> >  			enable_single_step(wp, instruction_pointer(regs));
> >  
> >  unlock:
> > diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
> > index b45c95d..4ef5373 100644
> > --- a/arch/arm64/kernel/hw_breakpoint.c
> > +++ b/arch/arm64/kernel/hw_breakpoint.c
> > @@ -616,7 +616,7 @@ static int breakpoint_handler(unsigned long unused, unsigned int esr,
> >  		perf_bp_event(bp, regs);
> >  
> >  		/* Do we need to handle the stepping? */
> > -		if (!bp->overflow_handler)
> > +		if (is_default_overflow_handler(bp))
> >  			step = 1;
> >  unlock:
> >  		rcu_read_unlock();
> > @@ -712,7 +712,7 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr,
> >  		perf_bp_event(wp, regs);
> >  
> >  		/* Do we need to handle the stepping? */
> > -		if (!wp->overflow_handler)
> > +		if (is_default_overflow_handler(wp))
> >  			step = 1;
> >  
> >  unlock:
> 
> Will, why does it matter what the overflow handler is for this stuff?

Because ptrace registers an overflow handler for raising a SIGTRAP and
ptrace users (e.g. GDB) expect to handle the single-stepping themselves.
Perf, on the other hand, will livelock if the kernel doesn't do the
stepping.

FWIW, I hate this whole thing. The only users of the perf side just seem
to be people running whacky test cases and then pointing out the places
where we're not identical to x86 :(

Will