ACK: [SRU][UNSTABLE/BIONIC/XENIAL][PATCH] UBUNTU: SAUCE: shiftfs: remove custom tmpfile method

Kamal Mostafa kamal at canonical.com
Thu Apr 23 18:09:46 UTC 2020


Acked-by: Kamal Mostafa <kamal at canonical.com>

 -Kamal

On Tue, Apr 14, 2020 at 10:17:37PM +0200, Christian Brauner wrote:
> BugLink: https://bugs.launchpad.net/bugs/1872757
> 
> Our revalidate methods were very opinionated about whether or not a
> dentry was valid when we really should've just let the underlay tell us
> what's what. This has led to bugs where a ESTALE was returned for e.g.
> temporary files that were created and directly re-opened afterwards
> through /proc/<pid>/fd/<nr-of-deleted-file>. When a file is re-opened
> through /proc/<pid>/fd/<nr> LOOKUP_JUMP is set and the vfs will
> revalidate via d_weak_revalidate(). Since the file has been unhashed or
> even already gone negative we'd fail the open when we should've
> succeeded.
> 
> I had also foolishly provided a .tmpfile method which so far only has
> caused us trouble and which was involved in the ESTALE problem. If we
> really need this then we can reimplement it properly but I doubt it.
> Remove it for now.
> 
> Reported-by: Christian Kellner <ckellner at redhat.com>
> Reported-by: Evgeny Vereshchagin >evvers at ya.ru>
> Cc: Seth Forshee <seth.forshee at canonical.com>
> Link: https://github.com/systemd/systemd/issues/14861
> Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
> ---
>  fs/shiftfs.c | 45 +++++++++++++--------------------------------
>  1 file changed, 13 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/shiftfs.c b/fs/shiftfs.c
> index 5c39529d0a17..39b878a6f820 100644
> --- a/fs/shiftfs.c
> +++ b/fs/shiftfs.c
> @@ -240,19 +240,17 @@ static int shiftfs_d_weak_revalidate(struct dentry *dentry, unsigned int flags)
>  	int err = 1;
>  	struct dentry *lowerd = dentry->d_fsdata;
>  
> -	if (d_is_negative(lowerd) != d_is_negative(dentry))
> -		return 0;
> -
> -	if ((lowerd->d_flags & DCACHE_OP_WEAK_REVALIDATE))
> +	if (lowerd->d_flags & DCACHE_OP_WEAK_REVALIDATE) {
>  		err = lowerd->d_op->d_weak_revalidate(lowerd, flags);
> +		if (err < 0)
> +			return err;
> +	}
>  
>  	if (d_really_is_positive(dentry)) {
>  		struct inode *inode = d_inode(dentry);
>  		struct inode *loweri = d_inode(lowerd);
>  
>  		shiftfs_copyattr(loweri, inode);
> -		if (!inode->i_nlink)
> -			err = 0;
>  	}
>  
>  	return err;
> @@ -263,23 +261,25 @@ static int shiftfs_d_revalidate(struct dentry *dentry, unsigned int flags)
>  	int err = 1;
>  	struct dentry *lowerd = dentry->d_fsdata;
>  
> -	if (d_unhashed(lowerd) ||
> -	    ((d_is_negative(lowerd) != d_is_negative(dentry))))
> -		return 0;
> -
>  	if (flags & LOOKUP_RCU)
>  		return -ECHILD;
>  
> -	if ((lowerd->d_flags & DCACHE_OP_REVALIDATE))
> +	if (lowerd->d_flags & DCACHE_OP_REVALIDATE) {
>  		err = lowerd->d_op->d_revalidate(lowerd, flags);
> +		if (err < 0)
> +			return err;
> +		if (!err) {
> +			if (!(flags & LOOKUP_RCU))
> +				d_invalidate(lowerd);
> +			return -ESTALE;
> +		}
> +	}
>  
>  	if (d_really_is_positive(dentry)) {
>  		struct inode *inode = d_inode(dentry);
>  		struct inode *loweri = d_inode(lowerd);
>  
>  		shiftfs_copyattr(loweri, inode);
> -		if (!inode->i_nlink)
> -			err = 0;
>  	}
>  
>  	return err;
> @@ -736,24 +736,6 @@ static int shiftfs_fiemap(struct inode *inode,
>  	return err;
>  }
>  
> -static int shiftfs_tmpfile(struct inode *dir, struct dentry *dentry,
> -			   umode_t mode)
> -{
> -	int err;
> -	const struct cred *oldcred;
> -	struct dentry *lowerd = dentry->d_fsdata;
> -	struct inode *loweri = dir->i_private;
> -
> -	if (!loweri->i_op->tmpfile)
> -		return -EOPNOTSUPP;
> -
> -	oldcred = shiftfs_override_creds(dir->i_sb);
> -	err = loweri->i_op->tmpfile(loweri, lowerd, mode);
> -	revert_creds(oldcred);
> -
> -	return err;
> -}
> -
>  static int shiftfs_setattr(struct dentry *dentry, struct iattr *attr)
>  {
>  	struct dentry *lowerd = dentry->d_fsdata;
> @@ -1020,7 +1002,6 @@ static const struct inode_operations shiftfs_file_inode_operations = {
>  	.listxattr	= shiftfs_listxattr,
>  	.permission	= shiftfs_permission,
>  	.setattr	= shiftfs_setattr,
> -	.tmpfile	= shiftfs_tmpfile,
>  };
>  
>  static const struct inode_operations shiftfs_special_inode_operations = {
> 
> base-commit: 1ff0ece4a5b555e1a72ec8d030b9410761174598
> -- 
> 2.26.0
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team



More information about the kernel-team mailing list