From: Al Viro <viro@zeniv.linux.org.uk>
Date: Mon, 18 Nov 2019 09:43:10 -0500
Subject: simple_recursive_removal(): kernel-side rm -rf for ramfs-style
filesystems
Patch-mainline: v5.6-rc1
Git-commit: a3d1e7eb5abe3aa1095bc75d1a6760d3809bd672
References: bsc#1159198 CVE-2019-19770
two requirements: no file creations in IS_DEADDIR and no cross-directory
renames whatsoever.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
[nstange@suse.de:
- fix diff context
- skip calls to non-existent fsnotify hooks
]
Acked-by: Nicolai Stange <nstange@suse.de>
---
fs/debugfs/inode.c | 118 +++++---------------------------------------
fs/libfs.c | 65 ++++++++++++++++++++++++
fs/tracefs/inode.c | 111 ++++-------------------------------------
include/linux/debugfs.h | 2
include/linux/fs.h | 2
include/linux/tracefs.h | 1
kernel/trace/trace.c | 4 -
kernel/trace/trace_events.c | 6 +-
kernel/trace/trace_hwlat.c | 2
9 files changed, 99 insertions(+), 212 deletions(-)
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -315,7 +315,10 @@ static struct dentry *start_creating(con
parent = debugfs_mount->mnt_root;
inode_lock(d_inode(parent));
- dentry = lookup_one_len(name, parent, strlen(name));
+ if (unlikely(IS_DEADDIR(d_inode(parent))))
+ dentry = ERR_PTR(-ENOENT);
+ else
+ dentry = lookup_one_len(name, parent, strlen(name));
if (!IS_ERR(dentry) && d_really_is_positive(dentry)) {
dput(dentry);
dentry = ERR_PTR(-EEXIST);
@@ -643,59 +646,15 @@ static void __debugfs_file_removed(struc
wait_for_completion(&fsd->active_users_drained);
}
-static int __debugfs_remove(struct dentry *dentry, struct dentry *parent)
+static void remove_one(struct dentry *victim)
{
- int ret = 0;
-
- if (simple_positive(dentry)) {
- dget(dentry);
- if (d_is_dir(dentry)) {
- ret = simple_rmdir(d_inode(parent), dentry);
- } else {
- simple_unlink(d_inode(parent), dentry);
- }
- if (!ret)
- d_delete(dentry);
- if (d_is_reg(dentry))
- __debugfs_file_removed(dentry);
- dput(dentry);
- }
- return ret;
-}
-
-/**
- * debugfs_remove - removes a file or directory from the debugfs filesystem
- * @dentry: a pointer to a the dentry of the file or directory to be
- * removed. If this parameter is NULL or an error value, nothing
- * will be done.
- *
- * This function removes a file or directory in debugfs that was previously
- * created with a call to another debugfs function (like
- * debugfs_create_file() or variants thereof.)
- *
- * This function is required to be called in order for the file to be
- * removed, no automatic cleanup of files will happen when a module is
- * removed, you are responsible here.
- */
-void debugfs_remove(struct dentry *dentry)
-{
- struct dentry *parent;
- int ret;
-
- if (IS_ERR_OR_NULL(dentry))
- return;
-
- parent = dentry->d_parent;
- inode_lock(d_inode(parent));
- ret = __debugfs_remove(dentry, parent);
- inode_unlock(d_inode(parent));
- if (!ret)
- simple_release_fs(&debugfs_mount, &debugfs_mount_count);
+ if (d_is_reg(victim))
+ __debugfs_file_removed(victim);
+ simple_release_fs(&debugfs_mount, &debugfs_mount_count);
}
-EXPORT_SYMBOL_GPL(debugfs_remove);
/**
- * debugfs_remove_recursive - recursively removes a directory
+ * debugfs_remove - recursively removes a directory
* @dentry: a pointer to a the dentry of the directory to be removed. If this
* parameter is NULL or an error value, nothing will be done.
*
@@ -707,65 +666,16 @@ EXPORT_SYMBOL_GPL(debugfs_remove);
* removed, no automatic cleanup of files will happen when a module is
* removed, you are responsible here.
*/
-void debugfs_remove_recursive(struct dentry *dentry)
+void debugfs_remove(struct dentry *dentry)
{
- struct dentry *child, *parent;
-
if (IS_ERR_OR_NULL(dentry))
return;
- parent = dentry;
- down:
- inode_lock(d_inode(parent));
- loop:
- /*
- * The parent->d_subdirs is protected by the d_lock. Outside that
- * lock, the child can be unlinked and set to be freed which can
- * use the d_u.d_child as the rcu head and corrupt this list.
- */
- spin_lock(&parent->d_lock);
- list_for_each_entry(child, &parent->d_subdirs, d_child) {
- if (!simple_positive(child))
- continue;
-
- /* perhaps simple_empty(child) makes more sense */
- if (!list_empty(&child->d_subdirs)) {
- spin_unlock(&parent->d_lock);
- inode_unlock(d_inode(parent));
- parent = child;
- goto down;
- }
-
- spin_unlock(&parent->d_lock);
-
- if (!__debugfs_remove(child, parent))
- simple_release_fs(&debugfs_mount, &debugfs_mount_count);
-
- /*
- * The parent->d_lock protects agaist child from unlinking
- * from d_subdirs. When releasing the parent->d_lock we can
- * no longer trust that the next pointer is valid.
- * Restart the loop. We'll skip this one with the
- * simple_positive() check.
- */
- goto loop;
- }
- spin_unlock(&parent->d_lock);
-
- inode_unlock(d_inode(parent));
- child = parent;
- parent = parent->d_parent;
- inode_lock(d_inode(parent));
-
- if (child != dentry)
- /* go up */
- goto loop;
-
- if (!__debugfs_remove(child, parent))
- simple_release_fs(&debugfs_mount, &debugfs_mount_count);
- inode_unlock(d_inode(parent));
+ simple_pin_fs(&debug_fs_type, &debugfs_mount, &debugfs_mount_count);
+ simple_recursive_removal(dentry, remove_one);
+ simple_release_fs(&debugfs_mount, &debugfs_mount_count);
}
-EXPORT_SYMBOL_GPL(debugfs_remove_recursive);
+EXPORT_SYMBOL_GPL(debugfs_remove);
/**
* debugfs_rename - rename a file/directory in the debugfs filesystem
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -229,6 +229,71 @@ const struct inode_operations simple_dir
};
EXPORT_SYMBOL(simple_dir_inode_operations);
+static struct dentry *find_next_child(struct dentry *parent, struct dentry *prev)
+{
+ struct dentry *child = NULL;
+ struct list_head *p = prev ? &prev->d_child : &parent->d_subdirs;
+
+ spin_lock(&parent->d_lock);
+ while ((p = p->next) != &parent->d_subdirs) {
+ struct dentry *d = container_of(p, struct dentry, d_child);
+ if (simple_positive(d)) {
+ spin_lock_nested(&d->d_lock, DENTRY_D_LOCK_NESTED);
+ if (simple_positive(d))
+ child = dget_dlock(d);
+ spin_unlock(&d->d_lock);
+ if (likely(child))
+ break;
+ }
+ }
+ spin_unlock(&parent->d_lock);
+ dput(prev);
+ return child;
+}
+
+void simple_recursive_removal(struct dentry *dentry,
+ void (*callback)(struct dentry *))
+{
+ struct dentry *this = dget(dentry);
+ while (true) {
+ struct dentry *victim = NULL, *child;
+ struct inode *inode = this->d_inode;
+
+ inode_lock(inode);
+ if (d_is_dir(this))
+ inode->i_flags |= S_DEAD;
+ while ((child = find_next_child(this, victim)) == NULL) {
+ // kill and ascend
+ // update metadata while it's still locked
+ inode->i_ctime = current_time(inode);
+ clear_nlink(inode);
+ inode_unlock(inode);
+ victim = this;
+ this = this->d_parent;
+ inode = this->d_inode;
+ inode_lock(inode);
+ if (simple_positive(victim)) {
+ d_invalidate(victim); // avoid lost mounts
+ if (callback)
+ callback(victim);
+ dput(victim); // unpin it
+ }
+ if (victim == dentry) {
+ inode->i_ctime = inode->i_mtime =
+ current_time(inode);
+ if (d_is_dir(dentry))
+ drop_nlink(inode);
+ inode_unlock(inode);
+ dput(dentry);
+ return;
+ }
+ }
+ inode_unlock(inode);
+ this = child;
+ }
+}
+EXPORT_SYMBOL(simple_recursive_removal);
+
static const struct super_operations simple_super_operations = {
.statfs = simple_statfs,
};
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -335,7 +335,10 @@ static struct dentry *start_creating(con
parent = tracefs_mount->mnt_root;
inode_lock(parent->d_inode);
- dentry = lookup_one_len(name, parent, strlen(name));
+ if (unlikely(IS_DEADDIR(parent->d_inode)))
+ dentry = ERR_PTR(-ENOENT);
+ else
+ dentry = lookup_one_len(name, parent, strlen(name));
if (!IS_ERR(dentry) && dentry->d_inode) {
dput(dentry);
dentry = ERR_PTR(-EEXIST);
@@ -500,119 +503,27 @@ struct dentry *tracefs_create_instance_d
return dentry;
}
-static int __tracefs_remove(struct dentry *dentry, struct dentry *parent)
+static void remove_one(struct dentry *victim)
{
- int ret = 0;
-
- if (simple_positive(dentry)) {
- if (dentry->d_inode) {
- dget(dentry);
- switch (dentry->d_inode->i_mode & S_IFMT) {
- case S_IFDIR:
- ret = simple_rmdir(parent->d_inode, dentry);
- break;
- default:
- simple_unlink(parent->d_inode, dentry);
- break;
- }
- if (!ret)
- d_delete(dentry);
- dput(dentry);
- }
- }
- return ret;
-}
-
-/**
- * tracefs_remove - removes a file or directory from the tracefs filesystem
- * @dentry: a pointer to a the dentry of the file or directory to be
- * removed.
- *
- * This function removes a file or directory in tracefs that was previously
- * created with a call to another tracefs function (like
- * tracefs_create_file() or variants thereof.)
- */
-void tracefs_remove(struct dentry *dentry)
-{
- struct dentry *parent;
- int ret;
-
- if (IS_ERR_OR_NULL(dentry))
- return;
-
- parent = dentry->d_parent;
- inode_lock(parent->d_inode);
- ret = __tracefs_remove(dentry, parent);
- inode_unlock(parent->d_inode);
- if (!ret)
- simple_release_fs(&tracefs_mount, &tracefs_mount_count);
+ simple_release_fs(&tracefs_mount, &tracefs_mount_count);
}
/**
- * tracefs_remove_recursive - recursively removes a directory
+ * tracefs_remove - recursively removes a directory
* @dentry: a pointer to a the dentry of the directory to be removed.
*
* This function recursively removes a directory tree in tracefs that
* was previously created with a call to another tracefs function
* (like tracefs_create_file() or variants thereof.)
*/
-void tracefs_remove_recursive(struct dentry *dentry)
+void tracefs_remove(struct dentry *dentry)
{
- struct dentry *child, *parent;
-
if (IS_ERR_OR_NULL(dentry))
return;
- parent = dentry;
- down:
- inode_lock(parent->d_inode);
- loop:
- /*
- * The parent->d_subdirs is protected by the d_lock. Outside that
- * lock, the child can be unlinked and set to be freed which can
- * use the d_u.d_child as the rcu head and corrupt this list.
- */
- spin_lock(&parent->d_lock);
- list_for_each_entry(child, &parent->d_subdirs, d_child) {
- if (!simple_positive(child))
- continue;
-
- /* perhaps simple_empty(child) makes more sense */
- if (!list_empty(&child->d_subdirs)) {
- spin_unlock(&parent->d_lock);
- inode_unlock(parent->d_inode);
- parent = child;
- goto down;
- }
-
- spin_unlock(&parent->d_lock);
-
- if (!__tracefs_remove(child, parent))
- simple_release_fs(&tracefs_mount, &tracefs_mount_count);
-
- /*
- * The parent->d_lock protects agaist child from unlinking
- * from d_subdirs. When releasing the parent->d_lock we can
- * no longer trust that the next pointer is valid.
- * Restart the loop. We'll skip this one with the
- * simple_positive() check.
- */
- goto loop;
- }
- spin_unlock(&parent->d_lock);
-
- inode_unlock(parent->d_inode);
- child = parent;
- parent = parent->d_parent;
- inode_lock(parent->d_inode);
-
- if (child != dentry)
- /* go up */
- goto loop;
-
- if (!__tracefs_remove(child, parent))
- simple_release_fs(&tracefs_mount, &tracefs_mount_count);
- inode_unlock(parent->d_inode);
+ simple_pin_fs(&trace_fs_type, &tracefs_mount, &tracefs_mount_count);
+ simple_recursive_removal(dentry, remove_one);
+ simple_release_fs(&tracefs_mount, &tracefs_mount_count);
}
/**
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -85,7 +85,7 @@ struct dentry *debugfs_create_automount(
void *data);
void debugfs_remove(struct dentry *dentry);
-void debugfs_remove_recursive(struct dentry *dentry);
+#define debugfs_remove_recursive debugfs_remove
const struct file_operations *debugfs_real_fops(const struct file *filp);
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3068,6 +3068,8 @@ extern int simple_unlink(struct inode *,
extern int simple_rmdir(struct inode *, struct dentry *);
extern int simple_rename(struct inode *, struct dentry *,
struct inode *, struct dentry *, unsigned int);
+extern void simple_recursive_removal(struct dentry *,
+ void (*callback)(struct dentry *));
extern int noop_fsync(struct file *, loff_t, loff_t, int);
extern int noop_set_page_dirty(struct page *page);
extern void noop_invalidatepage(struct page *page, unsigned int offset,
--- a/include/linux/tracefs.h
+++ b/include/linux/tracefs.h
@@ -32,7 +32,6 @@ struct dentry *tracefs_create_file(const
struct dentry *tracefs_create_dir(const char *name, struct dentry *parent);
void tracefs_remove(struct dentry *dentry);
-void tracefs_remove_recursive(struct dentry *dentry);
struct dentry *tracefs_create_instance_dir(const char *name, struct dentry *parent,
int (*mkdir)(const char *name),
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7570,7 +7570,7 @@ static int instance_mkdir(const char *na
ret = event_trace_add_tracer(tr->dir, tr);
if (ret) {
- tracefs_remove_recursive(tr->dir);
+ tracefs_remove(tr->dir);
goto out_free_tr;
}
@@ -7635,7 +7635,7 @@ static int instance_rmdir(const char *na
event_trace_del_tracer(tr);
ftrace_clear_pids(tr);
ftrace_destroy_function_files(tr);
- tracefs_remove_recursive(tr->dir);
+ tracefs_remove(tr->dir);
free_trace_buffers(tr);
for (i = 0; i < tr->nr_topts; i++) {
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -663,7 +663,7 @@ static void remove_subsystem(struct trac
return;
if (!--dir->nr_events) {
- tracefs_remove_recursive(dir->entry);
+ tracefs_remove(dir->entry);
list_del(&dir->list);
__put_system_dir(dir);
}
@@ -682,7 +682,7 @@ static void remove_event_file_dir(struct
}
spin_unlock(&dir->d_lock);
- tracefs_remove_recursive(dir);
+ tracefs_remove(dir);
}
list_del(&file->list);
@@ -2986,7 +2986,7 @@ int event_trace_del_tracer(struct trace_
down_write(&trace_event_sem);
__trace_remove_event_dirs(tr);
- tracefs_remove_recursive(tr->event_dir);
+ tracefs_remove(tr->event_dir);
up_write(&trace_event_sem);
tr->event_dir = NULL;
--- a/kernel/trace/trace_hwlat.c
+++ b/kernel/trace/trace_hwlat.c
@@ -550,7 +550,7 @@ static int init_tracefs(void)
return 0;
err:
- tracefs_remove_recursive(top_dir);
+ tracefs_remove(top_dir);
return -ENOMEM;
}