linux-kernel - Re: [PATCH 1/2 v2] tracing/syscalls: core infrastructure for syscalls tracing

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]

Date:	Sun, 15 Mar 2009 17:17:36 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Ingo Molnar <mingo@...e.hu>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Steven Rostedt <rostedt@...dmis.org>, tglx@...utronix.de,
	Jason Baron <jbaron@...hat.com>,
	"Frank Ch. Eigler" <fche@...hat.com>,
	Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	Jiaying Zhang <jiayingz@...gle.com>,
	Michael Rubin <mrubin@...gle.com>,
	Martin Bligh <mbligh@...gle.com>,
	Michael Davidson <md@...gle.com>
Subject: Re: [PATCH 1/2 v2] tracing/syscalls: core infrastructure for
	syscalls tracing

On Sun, Mar 15, 2009 at 01:02:39AM -0700, Andrew Morton wrote:
> On Sun, 15 Mar 2009 05:59:04 +0100 Ingo Molnar <mingo@...e.hu> wrote:
> 
> > 
> > * Andrew Morton <akpm@...ux-foundation.org> wrote:
> > 
> > > On Fri, 13 Mar 2009 15:42:11 +0100 Frederic Weisbecker <fweisbec@...il.com> wrote:
> > > 
> > > > +void start_ftrace_syscalls(void)
> > > > +{
> > > > +	unsigned long flags;
> > > > +	struct task_struct *g, *t;
> > > > +
> > > > +	if (atomic_inc_return(&refcount) != 1)
> > > > +		goto out;
> > > > +
> > > > +	arch_init_ftrace_syscalls();
> > > > +	read_lock_irqsave(&tasklist_lock, flags);
> > > > +
> > > > +	do_each_thread(g, t) {
> > > > +		set_tsk_thread_flag(t, TIF_SYSCALL_FTRACE);
> > > > +	} while_each_thread(g, t);
> > > > +
> > > > +	read_unlock_irqrestore(&tasklist_lock, flags);
> > > > +out:
> > > > +	atomic_dec(&refcount);
> > > > +}
> > > > +
> > > > +void stop_ftrace_syscalls(void)
> > > > +{
> > > > +	unsigned long flags;
> > > > +	struct task_struct *g, *t;
> > > > +
> > > > +	if (atomic_dec_return(&refcount))
> > > > +		goto out;
> > > > +
> > > > +	read_lock_irqsave(&tasklist_lock, flags);
> > > > +
> > > > +	do_each_thread(g, t) {
> > > > +		clear_tsk_thread_flag(t, TIF_SYSCALL_FTRACE);
> > > > +	} while_each_thread(g, t);
> > > > +
> > > > +	read_unlock_irqrestore(&tasklist_lock, flags);
> > > > +out:
> > > > +	atomic_inc(&refcount);
> > > > +}
> > > 
> > > What is this `refcount' thing trying to do?  afacit it does 
> > > not prevent the two loops from running concurrently and making 
> > > a mess.
> > > 
> > > If it _is_ trying to prevent that from happening, then why not 
> > > use plain old mutex_lock()?
> > 
> > yeah - already commented about that to Frederic over IRC. A 
> > plain flag, checked inside the tasklist lock is more than 
> > enough.
> > 
> 
> That would require write_lock().


Hm, I made a mistake in this code.
What I wanted to do is to allow several tracing callsites to ask
for syscalls tracing and to only disable it when the last user releases
it (and avoid the iterate the tasklist one more time while the flag
is already set).

So I shouldn't increment it back when there is already one user,
I should only exit without doing anything.

But I still need a lock to prevent from concurrent clear/set flag
on the tasks when the last user is disabling syscall tracing and a new one
requests it.

I need a mutex here. I'm preparing it.

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/