mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Alexander Z Lam <azl@google.com>,
	Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	David Sharp <dhsharp@google.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	Vaibhav Nagarnaik <vnagarnaik@google.com>,
	"zhangwei(Jovi)" <jovi.zhangwei@huawei.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] debugfs: debugfs_remove_recursive() must not rely on list_empty(d_subdirs)
Date: Fri, 26 Jul 2013 10:38:57 -0700	[thread overview]
Message-ID: <20130726173857.GB15013@kroah.com> (raw)
In-Reply-To: <20130726151256.GC19472@redhat.com>

On Fri, Jul 26, 2013 at 05:12:56PM +0200, Oleg Nesterov wrote:
> debugfs_remove_recursive() is wrong,
> 
> 1. it wrongly assumes that !list_empty(d_subdirs) means that this
>    dir should be removed.
> 
>    This is not that bad by itself, but:
> 
> 2. if d_subdirs does not becomes empty after __debugfs_remove()
>    it gives up and silently fails, it doesn't even try to remove
>    other entries.
> 
>    However ->d_subdirs can be non-empty because it still has the
>    already deleted !debugfs_positive() entries.
> 
> 3. simple_release_fs() is called even if __debugfs_remove() fails.
> 
> Suppose we have
> 
> 	dir1/
> 		dir2/
> 			file2
> 		file1
> 
> and someone opens dir1/dir2/file2.
> 
> Now, debugfs_remove_recursive(dir1/dir2) succeeds, and dir1/di2 goes
> away.
> 
> But debugfs_remove_recursive(dir1) silently fails and doesn't remove
> this directory. Because it tries to delete (the already deleted)
> dir1/dir2/file2 again and then fails due to "Avoid infinite loop"
> logic.
> 
> Test-case:
> 
> 	#!/bin/sh
> 
> 	cd /sys/kernel/debug/tracing
> 	echo 'p:probe/sigprocmask sigprocmask' >> kprobe_events
> 	sleep 1000 < events/probe/sigprocmask/id &
> 	echo -n >| kprobe_events
> 
> 	[ -d events/probe ] && echo "ERR!! failed to rm probe"
> 
> And after that it is not possible to create another probe entry.
> 
> With this patch debugfs_remove_recursive() skips !debugfs_positive()
> files although this is not strictly needed. The most important change
> is that it does not try to make ->d_subdirs empty, it simply scans
> the whole list(s) recursively and removes as much as possible.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


  reply	other threads:[~2013-07-26 17:39 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-23 20:58 [PATCH 0/6] tracing: open/delete fixes Oleg Nesterov
2013-07-23 20:59 ` [PATCH 1/6] tracing: Turn event/id->i_private into call->event.type Oleg Nesterov
2013-07-24 20:13   ` Steven Rostedt
2013-07-25 14:26     ` Oleg Nesterov
2013-07-23 20:59 ` [PATCH 2/6] tracing: Change event_enable/disable_read() to verify i_private != NULL Oleg Nesterov
2013-07-24 19:37   ` Steven Rostedt
2013-07-23 20:59 ` [PATCH 3/6] tracing: Change event_filter_read/write " Oleg Nesterov
2013-07-24 19:52   ` Steven Rostedt
2013-07-23 20:59 ` [PATCH 4/6] tracing: Change f_start() to take event_mutex and " Oleg Nesterov
2013-07-23 20:59 ` [PATCH 5/6] tracing: Introduce remove_event_file_dir() Oleg Nesterov
2013-07-24 20:01   ` Steven Rostedt
2013-07-23 20:59 ` [PATCH 6/6] tracing: Change remove_event_file_dir() to clear "d_subdirs"->i_private Oleg Nesterov
2013-07-24 20:08   ` Steven Rostedt
2013-07-25 14:18     ` Oleg Nesterov
2013-07-24 18:46 ` [PATCH 0/6] tracing: open/delete fixes Oleg Nesterov
2013-07-25 16:56   ` Oleg Nesterov
2013-07-25 19:27   ` PATCH? debugfs_remove_recursive() must not rely on list_empty(d_subdirs) Oleg Nesterov
2013-07-25 20:04     ` Oleg Nesterov
2013-07-25 23:43       ` Greg Kroah-Hartman
2013-07-26 15:11         ` [PATCH 0/1] debugfs: " Oleg Nesterov
2013-07-26 15:12           ` Oleg Nesterov
2013-07-26 15:14             ` Oleg Nesterov
2013-07-26 15:12           ` [PATCH 1/1] " Oleg Nesterov
2013-07-26 17:38             ` Greg Kroah-Hartman [this message]
2013-07-26 18:40               ` Steven Rostedt
2013-07-26 15:30           ` [PATCH 0/1] " Steven Rostedt
2013-07-26 16:28             ` Greg Kroah-Hartman
2013-07-26 17:38             ` Greg Kroah-Hartman
2013-07-26 10:24       ` Re: PATCH? " Masami Hiramatsu
2013-07-26 14:49         ` Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130726173857.GB15013@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@ghostprotocols.net \
    --cc=azl@google.com \
    --cc=dhsharp@google.com \
    --cc=fweisbec@gmail.com \
    --cc=jovi.zhangwei@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=vnagarnaik@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®