linux-kernel - Re: Q: perf_event && task->ptrace

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: <20110128174105.GA1761@nowhere>
Date:	Fri, 28 Jan 2011 18:41:10 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Alan Stern <stern@...land.harvard.edu>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Ingo Molnar <mingo@...e.hu>, Paul Mackerras <paulus@...ba.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Prasad <prasad@...ux.vnet.ibm.com>,
	Roland McGrath <roland@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: Q: perf_event && task->ptrace_bps[]

On Thu, Jan 20, 2011 at 06:28:10PM +0100, Oleg Nesterov wrote:
> On 01/19, Frederic Weisbecker wrote:
> > OTOH I can drop
> > more of them for the no-running-breakpoint case from thread_struct
> > in a subsequent task.
> 
> Hmm. Can't understand what do you mean. Just curious, could you explain?

Indeed now that I read that, it was completely not understandable :)

So I meant that currently we have this:

task->thread->ptrace_bps[BP_NUM]

Where ptrace_bps is:

struct perf_event *ptrace_bps[BP_NUM];

And we populate that with pointers when needed. Now this is a waste
of space, I should better make it:

struct perf_event **ptrace_bps;

And only allocate the pointer space when needed.

 
> > Note the problem touches more archs than x86. Basically every
> > arch that use breakpoint use a similar scheme that must be fixed.
> 
> Yes. Perhaps we should try to unify some code... Say, can't we move
> ->ptrace_bps[] to task_struct?

It seems that every archs that currently implement breakpoints have
this linear mapping of registers, even when physically they are not
linear: ARM has a seperate register space for instruction and data
breakpoints for example.

So yeah it seems we can store that in task_struct. I may try that
in a subsequent patch.

> 
> > +void ptrace_put_breakpoints(struct task_struct *tsk)
> > +{
> > +	if (!atomic_dec_return(&tsk->ptrace_bp_refcnt))
> > +		flush_ptrace_hw_breakpoint(tsk);
> 
> (minor nit, atomic_dec_and_test() looks more natural)

Indeed, will change that.

Thanks!
--
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/