linux-kernel - Re: [RFC][PATCH 0/4] tracing/kprobes/uprobes: Fix race between opening probe event files and deleting probe

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: <1374001825.6458.44.camel@gandalf.local.home>
Date:	Tue, 16 Jul 2013 15:10:25 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	linux-kernel@...r.kernel.org,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	"zhangwei(Jovi)" <jovi.zhangwei@...wei.com>,
	Jiri Olsa <jolsa@...hat.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Arnaldo Carvalho de Melo <acme@...stprotocols.net>,
	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [RFC][PATCH 0/4] tracing/kprobes/uprobes: Fix race between
 opening probe event files and deleting probe

On Tue, 2013-07-16 at 18:38 +0200, Oleg Nesterov wrote:
> On 07/15, Oleg Nesterov wrote:
> >
> > So. As Masami pointed out, this is not enough. Probably we can add more
> > hacks, but I'd like to discuss the alternative approach.
> >
> > Note also that this ref count has the unfortunate property, if someone
> > keeps the file opened we can't remove an event.
> 
> And please correct me, but afaics there are similar problems with
> rmdir instances/xxx.

The instances have their own refcount. And if a file is open, it wont
remove the directory. Just try it...


# cd /sys/kernel/debug/tracing/instances
# mkdir foo
# sleep 10 < foo/events/ext3/enable &
[1] 2087
# rmdir foo
rmdir: failed to remove `foo': Device or resource busy
# 
[1]+  Done                    sleep 10 < foo/events/ext3/enable
# rmdir foo
# 

> 
> > What this patch does:
> >
> > 	- add the new "topmost" rw_semaphore, file_sem.
> 
> probably unneeded...
> 
> > 	- trace_remove_event_call() takes this sem for writing and
> > 	  cleares enable/id/format/filter->i_private
> >
> > 	- removes tracing_open_generic_file/tracing_release_generic_file,
> > 	  we do not use file->f_private any longer
> >
> > 	- changes event_enable_read/event_enable_write to read
> > 	  ftrace_event_file = i_private under read_lock(file_sem) and
> > 	  abort if it is null.
> >
> > Question: why event_enable_write() needs trace_array_get() ?
> 
> probably it doesn't...

I'm confused. Which code has event_enable_write doing a
trace_array_get()?

> 
> > Steven, Masami, do you think this can make any sense?

I need to sort these patches out. They are all over the place. What
exactly do we have for a solution here. Did we figure out which patches
are needed?

I'll have to go back and re-read the entire thread. I've been all over
the place lately with different topics, and don't remember all that has
been said here.

-- Steve


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