Re: [PATCH 2/4] xfs: don't propagate invalid extent size hints to new files

[Brian Foster <bfoster@redhat.com>]

On Wed, May 12, 2021 at 06:01:53PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Under the current inode extent size hint validation rules, it's possible
> to set extent size hints on directories along with an 'inherit' flag so
> that the values will be propagated to newly created regular files.  (The
> directories themselves do not care about the hint values.)
> 
> For these directories, the alignment of the hint is checked against the
> data device even if the directory also has the rtinherit hint set, which
> means that one can set a directory's hint value to something that isn't
> an integer multiple of the realtime extent size.  This isn't a problem
> for the directory itself, but the validation routines require rt extent
> alignment for realtime files.
> 
> If the unaligned hint value and the realtime bit are both propagated
> into a newly created regular realtime file, we end up writing out an
> incorrect hint that trips the verifiers the next time we try to read the
> inode buffer, and the fs shuts down.  Fix this by cancelling the hint
> propagation if it would cause problems.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Hmm.. this seems a bit unfortunate. Is the purpose of this flag
cancellation behavior basically to accommodate existing filesystems that
might have this incompatible combination in place?

Brian

>  fs/xfs/xfs_inode.c |   19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 0369eb22c1bb..db81e8c22708 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -689,6 +689,7 @@ xfs_inode_inherit_flags(
>  	struct xfs_inode	*ip,
>  	const struct xfs_inode	*pip)
>  {
> +	xfs_failaddr_t		failaddr;
>  	unsigned int		di_flags = 0;
>  	umode_t			mode = VFS_I(ip)->i_mode;
>  
> @@ -728,6 +729,14 @@ xfs_inode_inherit_flags(
>  	if (pip->i_diflags & XFS_DIFLAG_FILESTREAM)
>  		di_flags |= XFS_DIFLAG_FILESTREAM;
>  
> +	/* Make sure the extsize actually validates properly. */
> +	failaddr = xfs_inode_validate_extsize(ip->i_mount, ip->i_extsize,
> +			VFS_I(ip)->i_mode, ip->i_diflags);
> +	if (failaddr) {
> +		di_flags &= ~(XFS_DIFLAG_EXTSIZE | XFS_DIFLAG_EXTSZINHERIT);
> +		ip->i_extsize = 0;
> +	}
> +
>  	ip->i_diflags |= di_flags;
>  }
>  
> @@ -737,12 +746,22 @@ xfs_inode_inherit_flags2(
>  	struct xfs_inode	*ip,
>  	const struct xfs_inode	*pip)
>  {
> +	xfs_failaddr_t		failaddr;
> +
>  	if (pip->i_diflags2 & XFS_DIFLAG2_COWEXTSIZE) {
>  		ip->i_diflags2 |= XFS_DIFLAG2_COWEXTSIZE;
>  		ip->i_cowextsize = pip->i_cowextsize;
>  	}
>  	if (pip->i_diflags2 & XFS_DIFLAG2_DAX)
>  		ip->i_diflags2 |= XFS_DIFLAG2_DAX;
> +
> +	/* Make sure the cowextsize actually validates properly. */
> +	failaddr = xfs_inode_validate_cowextsize(ip->i_mount, ip->i_cowextsize,
> +			VFS_I(ip)->i_mode, ip->i_diflags, ip->i_diflags2);
> +	if (failaddr) {
> +		ip->i_diflags2 &= ~XFS_DIFLAG2_COWEXTSIZE;
> +		ip->i_cowextsize = 0;
> +	}
>  }
>  
>  /*
>