Subject: Re: [RFC][PATCH] proc: invalidate the deleting or deleted proc dentry
To: <linux-fsdevel@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <1492091375-18642-1-git-send-email-houtao1@huawei.com>
CC: <viro@zeniv.linux.org.uk>, <ebiederm@xmission.com>,
        <dhowells@redhat.com>, <jgross@suse.com>, <geliangtang@gmail.com>,
        <akpm@linux-foundation.org>, <dmitry.torokhov@gmail.com>,
        Jan Kara <jack@suse.cz>
From: Hou Tao <houtao1@huawei.com>
Message-ID: <54f9d878-d3f8-8d46-37b3-89583a4599f9@huawei.com>
Date: Sun, 23 Apr 2017 17:50:18 +0800
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.4.0
MIME-Version: 1.0
In-Reply-To: <1492091375-18642-1-git-send-email-houtao1@huawei.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3387
Lines: 101

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 <houtao1@huawei.com>
> ---
>  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
>