proc_remove() looks the entry up by its name in its parent and removes
whatever it finds there:
remove_proc_subtree(de->name, de->parent);
If the entry was removed already, say along with its parent, that is
either nothing or a different entry that got the same name in the
meantime, and de->parent may well be gone. Two removals of the same
entry are a bug in the caller, but going by the name turns it into
removing an entry that belongs to somebody else.
Since commit 895b4c0c79b0 ("fs/proc: fix uaf in proc_readdir_de()")
an entry that was unlinked has an empty rb node. So check for that
under proc_subdir_lock and otherwise remove the entry itself, with the
loop of remove_proc_subtree() moved into a helper both of them use.
Nothing can help a caller that passes an entry that has been freed
already.
Reported-by: Chris Mason <mason@kernel.org>
Fixes: a8ca16ea7b0a ("proc: Supply a function to remove a proc entry by PDE")
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
fs/proc/generic.c | 78 ++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 51 insertions(+), 27 deletions(-)
diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index 47830d6af255..23fc026d7bbb 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -817,33 +817,14 @@ void remove_proc_entry(const char *name, struct proc_dir_entry *parent)
}
EXPORT_SYMBOL(remove_proc_entry);
-int remove_proc_subtree(const char *name, struct proc_dir_entry *parent)
+/*
+ * Remove @root, already erased from its parent, and everything below it.
+ * Called with proc_subdir_lock held for writing, returns with it dropped.
+ */
+static int __remove_proc_subtree(struct proc_dir_entry *root)
{
- struct proc_dir_entry *root = NULL, *de, *next;
- const char *fn = name;
- unsigned int len;
-
- write_lock(&proc_subdir_lock);
- if (__xlate_proc_name(name, &parent, &fn) != 0) {
- write_unlock(&proc_subdir_lock);
- return -ENOENT;
- }
- len = strlen(fn);
+ struct proc_dir_entry *de = root, *next;
- root = pde_subdir_find(parent, fn, len);
- if (!root) {
- write_unlock(&proc_subdir_lock);
- return -ENOENT;
- }
- if (unlikely(pde_is_permanent(root))) {
- write_unlock(&proc_subdir_lock);
- WARN(1, "removing permanent /proc entry '%s/%s'",
- root->parent->name, root->name);
- return -EINVAL;
- }
- pde_erase(root, parent);
-
- de = root;
while (1) {
next = pde_subdir_first(de);
if (next) {@@ -871,6 +852,34 @@ int remove_proc_subtree(const char *name, struct proc_dir_entry *parent)
pde_put(root);
return 0;
}
+
+int remove_proc_subtree(const char *name, struct proc_dir_entry *parent)
+{
+ struct proc_dir_entry *root = NULL;
+ const char *fn = name;
+ unsigned int len;
+
+ write_lock(&proc_subdir_lock);
+ if (__xlate_proc_name(name, &parent, &fn) != 0) {
+ write_unlock(&proc_subdir_lock);
+ return -ENOENT;
+ }
+ len = strlen(fn);
+
+ root = pde_subdir_find(parent, fn, len);
+ if (!root) {
+ write_unlock(&proc_subdir_lock);
+ return -ENOENT;
+ }
+ if (unlikely(pde_is_permanent(root))) {
+ write_unlock(&proc_subdir_lock);
+ WARN(1, "removing permanent /proc entry '%s/%s'",
+ root->parent->name, root->name);
+ return -EINVAL;
+ }
+ pde_erase(root, parent);
+ return __remove_proc_subtree(root);
+}
EXPORT_SYMBOL(remove_proc_subtree);
void *proc_get_parent_data(const struct inode *inode)@@ -882,8 +891,23 @@ EXPORT_SYMBOL_GPL(proc_get_parent_data);
void proc_remove(struct proc_dir_entry *de)
{
- if (de)
- remove_proc_subtree(de->name, de->parent);
+ if (!de)
+ return;
+
+ write_lock(&proc_subdir_lock);
+ /* Removed already, maybe along with its parent. */
+ if (RB_EMPTY_NODE(&de->subdir_node)) {
+ write_unlock(&proc_subdir_lock);
+ return;
+ }
+ if (unlikely(pde_is_permanent(de))) {
+ write_unlock(&proc_subdir_lock);
+ WARN(1, "removing permanent /proc entry '%s/%s'",
+ de->parent->name, de->name);
+ return;
+ }
+ pde_erase(de, de->parent);
+ __remove_proc_subtree(de);
}
EXPORT_SYMBOL(proc_remove);
--
2.53.0