Re: [PATCH v3] ceph: fail the open_by_handle_at() if the dentry is being unlinked

["Luís Henriques" <lhenriques@suse.de>]

On Wed, Aug 31, 2022 at 10:16:17AM +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> When unlinking a file the kclient will send a unlink request to MDS
> by holding the dentry reference, and then the MDS will return 2 replies,
> which are unsafe reply and a deferred safe reply.
> 
> After the unsafe reply received the kernel will return and succeed
> the unlink request to user space apps.
> 
> Only when the safe reply received the dentry's reference will be
> released. Or the dentry will only be unhashed from dcache. But when
> the open_by_handle_at() begins to open the unlinked files it will
> succeed.
> 
> The inode->i_count couldn't be used to check whether the inode is
> opened or not.
> 
> URL: https://tracker.ceph.com/issues/56524
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
> 
> V3:
> - The inode->i_count couldn't be correctly indicate that whether the
>   file is opened or not.
> 
> V2:
> - If the dentry was released and inode is evicted such as by dropping
>   the caches, it will allocate a new dentry, which is also unhashed.
> 
>  fs/ceph/export.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ceph/export.c b/fs/ceph/export.c
> index 0ebf2bd93055..8559990a59a5 100644
> --- a/fs/ceph/export.c
> +++ b/fs/ceph/export.c
> @@ -182,6 +182,7 @@ struct inode *ceph_lookup_inode(struct super_block *sb, u64 ino)
>  static struct dentry *__fh_to_dentry(struct super_block *sb, u64 ino)
>  {
>  	struct inode *inode = __lookup_inode(sb, ino);
> +	struct ceph_inode_info *ci = ceph_inode(inode);
>  	int err;
>  
>  	if (IS_ERR(inode))
> @@ -193,7 +194,7 @@ static struct dentry *__fh_to_dentry(struct super_block *sb, u64 ino)
>  		return ERR_PTR(err);
>  	}
>  	/* -ESTALE if inode as been unlinked and no file is open */
> -	if ((inode->i_nlink == 0) && (atomic_read(&inode->i_count) == 1)) {
> +	if ((inode->i_nlink == 0) && !__ceph_is_file_opened(ci)) {
>  		iput(inode);
>  		return ERR_PTR(-ESTALE);
>  	}
> -- 
> 2.36.0.rc1
> 

Thanks, this seems be correct.  I was able to reproduce this locally, and
I can confirm this patch fixes it.  (Although I had this fixed this in the
past with 878dabb64117 and at that time it looked like it was fixed too.)

Feel free to add my:

Tested-by: Luís Henriques <lhenriques@suse.de>
Reviewed-by: Luís Henriques <lhenriques@suse.de>

Cheers,
--
Luís