Commit 776164c1 authored by Oleg Nesterov's avatar Oleg Nesterov Committed by Steven Rostedt

debugfs: debugfs_remove_recursive() must not rely on list_empty(d_subdirs)

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

Link: http://lkml.kernel.org/r/20130726151256.GC19472@redhat.comAcked-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
Signed-off-by: default avatarSteven Rostedt <rostedt@goodmis.org>
parent 8c4f3c3f
...@@ -533,8 +533,7 @@ EXPORT_SYMBOL_GPL(debugfs_remove); ...@@ -533,8 +533,7 @@ EXPORT_SYMBOL_GPL(debugfs_remove);
*/ */
void debugfs_remove_recursive(struct dentry *dentry) void debugfs_remove_recursive(struct dentry *dentry)
{ {
struct dentry *child; struct dentry *child, *next, *parent;
struct dentry *parent;
if (IS_ERR_OR_NULL(dentry)) if (IS_ERR_OR_NULL(dentry))
return; return;
...@@ -544,61 +543,37 @@ void debugfs_remove_recursive(struct dentry *dentry) ...@@ -544,61 +543,37 @@ void debugfs_remove_recursive(struct dentry *dentry)
return; return;
parent = dentry; parent = dentry;
down:
mutex_lock(&parent->d_inode->i_mutex); mutex_lock(&parent->d_inode->i_mutex);
list_for_each_entry_safe(child, next, &parent->d_subdirs, d_u.d_child) {
if (!debugfs_positive(child))
continue;
while (1) { /* perhaps simple_empty(child) makes more sense */
/*
* When all dentries under "parent" has been removed,
* walk up the tree until we reach our starting point.
*/
if (list_empty(&parent->d_subdirs)) {
mutex_unlock(&parent->d_inode->i_mutex);
if (parent == dentry)
break;
parent = parent->d_parent;
mutex_lock(&parent->d_inode->i_mutex);
}
child = list_entry(parent->d_subdirs.next, struct dentry,
d_u.d_child);
next_sibling:
/*
* If "child" isn't empty, walk down the tree and
* remove all its descendants first.
*/
if (!list_empty(&child->d_subdirs)) { if (!list_empty(&child->d_subdirs)) {
mutex_unlock(&parent->d_inode->i_mutex); mutex_unlock(&parent->d_inode->i_mutex);
parent = child; parent = child;
mutex_lock(&parent->d_inode->i_mutex); goto down;
continue;
} }
__debugfs_remove(child, parent); up:
if (parent->d_subdirs.next == &child->d_u.d_child) { if (!__debugfs_remove(child, parent))
/* simple_release_fs(&debugfs_mount, &debugfs_mount_count);
* Try the next sibling.
*/
if (child->d_u.d_child.next != &parent->d_subdirs) {
child = list_entry(child->d_u.d_child.next,
struct dentry,
d_u.d_child);
goto next_sibling;
} }
/*
* Avoid infinite loop if we fail to remove
* one dentry.
*/
mutex_unlock(&parent->d_inode->i_mutex); mutex_unlock(&parent->d_inode->i_mutex);
break; child = parent;
} parent = parent->d_parent;
simple_release_fs(&debugfs_mount, &debugfs_mount_count); mutex_lock(&parent->d_inode->i_mutex);
if (child != dentry) {
next = list_entry(child->d_u.d_child.next, struct dentry,
d_u.d_child);
goto up;
} }
parent = dentry->d_parent; if (!__debugfs_remove(child, parent))
mutex_lock(&parent->d_inode->i_mutex);
__debugfs_remove(dentry, parent);
mutex_unlock(&parent->d_inode->i_mutex);
simple_release_fs(&debugfs_mount, &debugfs_mount_count); simple_release_fs(&debugfs_mount, &debugfs_mount_count);
mutex_unlock(&parent->d_inode->i_mutex);
} }
EXPORT_SYMBOL_GPL(debugfs_remove_recursive); EXPORT_SYMBOL_GPL(debugfs_remove_recursive);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment