2017-04-13 13:49:33

by Hou Tao

[permalink] [raw]
Subject: [RFC][PATCH] proc: invalidate the deleting or deleted proc dentry

After the invocation of remove_proc_entry() for a proc fs directory,
if the related dentry had been held by some processes (eg., by chdir),
the lookup afterwards will still return the old proc_dir_entry. The
new created proc fs files under the proc fs directory will not be
visible until the old dentry is released, and this makes our hotplug
process to fail which needs to access the new proc fs files.

To fix it, we need to mark the deleting or deleted proc_dir_entry
as invalid and the lookup afterwards will use the new proc_dir_entry
regardless of the status of the old dentry.

Signed-off-by: Hou Tao <[email protected]>
---
fs/proc/generic.c | 21 ++++++++++++++++++++-
fs/proc/inode.c | 5 +++++
fs/proc/internal.h | 1 +
3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index ee27feb..92c9dd4 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -23,11 +23,19 @@
#include <linux/spinlock.h>
#include <linux/completion.h>
#include <linux/uaccess.h>
+#include <linux/namei.h>

#include "internal.h"

+static int proc_dentry_revalidate(struct dentry *dentry, unsigned int flags);
+
static DEFINE_RWLOCK(proc_subdir_lock);

+static const struct dentry_operations proc_dentry_operations = {
+ .d_revalidate = proc_dentry_revalidate,
+ .d_delete = always_delete_dentry,
+};
+
static int proc_match(unsigned int len, const char *name, struct proc_dir_entry *de)
{
if (len < de->namelen)
@@ -223,6 +231,17 @@ void proc_free_inum(unsigned int inum)
spin_unlock_irqrestore(&proc_inum_lock, flags);
}

+static int proc_dentry_revalidate(struct dentry *dentry, unsigned int flags)
+{
+ struct proc_dir_entry *de;
+
+ if (flags & LOOKUP_RCU)
+ return -ECHILD;
+
+ de = PDE(d_inode(dentry));
+ return !proc_entry_is_removing(de);
+}
+
/*
* Don't create negative dentries here, return -ENOENT by hand
* instead.
@@ -240,7 +259,7 @@ struct dentry *proc_lookup_de(struct proc_dir_entry *de, struct inode *dir,
inode = proc_get_inode(dir->i_sb, de);
if (!inode)
return ERR_PTR(-ENOMEM);
- d_set_d_op(dentry, &simple_dentry_operations);
+ d_set_d_op(dentry, &proc_dentry_operations);
d_add(dentry, inode);
return NULL;
}
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 2cc7a80..84232d0 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -171,6 +171,11 @@ static void close_pdeo(struct proc_dir_entry *pde, struct pde_opener *pdeo)
}
}

+bool proc_entry_is_removing(struct proc_dir_entry *de)
+{
+ return (atomic_read(&de->in_use) == BIAS);
+}
+
void proc_entry_rundown(struct proc_dir_entry *de)
{
DECLARE_COMPLETION_ONSTACK(c);
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index c5ae09b..646b3f6 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -205,6 +205,7 @@ void set_proc_pid_nlink(void);
extern struct inode *proc_get_inode(struct super_block *, struct proc_dir_entry *);
extern int proc_fill_super(struct super_block *, void *data, int flags);
extern void proc_entry_rundown(struct proc_dir_entry *);
+extern bool proc_entry_is_removing(struct proc_dir_entry *de);

/*
* proc_namespaces.c
--
2.5.0


2017-04-23 09:53:17

by Hou Tao

[permalink] [raw]
Subject: Re: [RFC][PATCH] proc: invalidate the deleting or deleted proc dentry

Hi, any comment ?

On 2017/4/13 21:49, Hou Tao wrote:
> After the invocation of remove_proc_entry() for a proc fs directory,
> if the related dentry had been held by some processes (eg., by chdir),
> the lookup afterwards will still return the old proc_dir_entry. The
> new created proc fs files under the proc fs directory will not be
> visible until the old dentry is released, and this makes our hotplug
> process to fail which needs to access the new proc fs files.
>
> To fix it, we need to mark the deleting or deleted proc_dir_entry
> as invalid and the lookup afterwards will use the new proc_dir_entry
> regardless of the status of the old dentry.
>
> Signed-off-by: Hou Tao <[email protected]>
> ---
> fs/proc/generic.c | 21 ++++++++++++++++++++-
> fs/proc/inode.c | 5 +++++
> fs/proc/internal.h | 1 +
> 3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/fs/proc/generic.c b/fs/proc/generic.c
> index ee27feb..92c9dd4 100644
> --- a/fs/proc/generic.c
> +++ b/fs/proc/generic.c
> @@ -23,11 +23,19 @@
> #include <linux/spinlock.h>
> #include <linux/completion.h>
> #include <linux/uaccess.h>
> +#include <linux/namei.h>
>
> #include "internal.h"
>
> +static int proc_dentry_revalidate(struct dentry *dentry, unsigned int flags);
> +
> static DEFINE_RWLOCK(proc_subdir_lock);
>
> +static const struct dentry_operations proc_dentry_operations = {
> + .d_revalidate = proc_dentry_revalidate,
> + .d_delete = always_delete_dentry,
> +};
> +
> static int proc_match(unsigned int len, const char *name, struct proc_dir_entry *de)
> {
> if (len < de->namelen)
> @@ -223,6 +231,17 @@ void proc_free_inum(unsigned int inum)
> spin_unlock_irqrestore(&proc_inum_lock, flags);
> }
>
> +static int proc_dentry_revalidate(struct dentry *dentry, unsigned int flags)
> +{
> + struct proc_dir_entry *de;
> +
> + if (flags & LOOKUP_RCU)
> + return -ECHILD;
> +
> + de = PDE(d_inode(dentry));
> + return !proc_entry_is_removing(de);
> +}
> +
> /*
> * Don't create negative dentries here, return -ENOENT by hand
> * instead.
> @@ -240,7 +259,7 @@ struct dentry *proc_lookup_de(struct proc_dir_entry *de, struct inode *dir,
> inode = proc_get_inode(dir->i_sb, de);
> if (!inode)
> return ERR_PTR(-ENOMEM);
> - d_set_d_op(dentry, &simple_dentry_operations);
> + d_set_d_op(dentry, &proc_dentry_operations);
> d_add(dentry, inode);
> return NULL;
> }
> diff --git a/fs/proc/inode.c b/fs/proc/inode.c
> index 2cc7a80..84232d0 100644
> --- a/fs/proc/inode.c
> +++ b/fs/proc/inode.c
> @@ -171,6 +171,11 @@ static void close_pdeo(struct proc_dir_entry *pde, struct pde_opener *pdeo)
> }
> }
>
> +bool proc_entry_is_removing(struct proc_dir_entry *de)
> +{
> + return (atomic_read(&de->in_use) == BIAS);
> +}
> +
> void proc_entry_rundown(struct proc_dir_entry *de)
> {
> DECLARE_COMPLETION_ONSTACK(c);
> diff --git a/fs/proc/internal.h b/fs/proc/internal.h
> index c5ae09b..646b3f6 100644
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -205,6 +205,7 @@ void set_proc_pid_nlink(void);
> extern struct inode *proc_get_inode(struct super_block *, struct proc_dir_entry *);
> extern int proc_fill_super(struct super_block *, void *data, int flags);
> extern void proc_entry_rundown(struct proc_dir_entry *);
> +extern bool proc_entry_is_removing(struct proc_dir_entry *de);
>
> /*
> * proc_namespaces.c
>