Re: [PATCH 3/4] xfs: validate extsz hints against rt extent size when rtinherit is set

[Brian Foster <bfoster@redhat.com>]

On Wed, May 12, 2021 at 06:01:58PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> The RTINHERIT bit can be set on a directory so that newly created
> regular files will have the REALTIME bit set to store their data on the
> realtime volume.  If an extent size hint (and EXTSZINHERIT) are set on
> the directory, the hint will also be copied into the new file.
> 
> As pointed out in previous patches, for realtime files we require the
> extent size hint be an integer multiple of the realtime extent, but we
> don't perform the same validation on a directory with both RTINHERIT and
> EXTSZINHERIT set, even though the only use-case of that combination is
> to propagate extent size hints into new realtime files.  This leads to
> inode corruption errors when the bad values are propagated.
> 
> Strengthen the validation routine to avoid this situation and fix the
> open-coded unit conversion while we're at it.  Note that this is
> technically a breaking change to the ondisk format, but the risk should
> be minimal because (a) most vendors disable realtime, (b) letting
> unaligned hints propagate to new files would immediately crash the
> filesystem, and (c) xfs_repair flags such filesystems as corrupt, so
> anyone with such a configuration is broken already anyway.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Ok, so this looks more like a proper fix, but does this turn an existing
directory with (rtinherit && extszinherit) and a badly aligned extsz
hint into a read validation error?

Brian

>  fs/xfs/libxfs/xfs_inode_buf.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 5c9a7440d9e4..25261dd73290 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -569,19 +569,20 @@ xfs_inode_validate_extsize(
>  	uint16_t			mode,
>  	uint16_t			flags)
>  {
> -	bool				rt_flag;
> +	bool				rt_flag, rtinherit_flag;
>  	bool				hint_flag;
>  	bool				inherit_flag;
>  	uint32_t			extsize_bytes;
>  	uint32_t			blocksize_bytes;
>  
>  	rt_flag = (flags & XFS_DIFLAG_REALTIME);
> +	rtinherit_flag = (flags & XFS_DIFLAG_RTINHERIT);
>  	hint_flag = (flags & XFS_DIFLAG_EXTSIZE);
>  	inherit_flag = (flags & XFS_DIFLAG_EXTSZINHERIT);
>  	extsize_bytes = XFS_FSB_TO_B(mp, extsize);
>  
> -	if (rt_flag)
> -		blocksize_bytes = mp->m_sb.sb_rextsize << mp->m_sb.sb_blocklog;
> +	if (rt_flag || (rtinherit_flag && inherit_flag))
> +		blocksize_bytes = XFS_FSB_TO_B(mp, mp->m_sb.sb_rextsize);
>  	else
>  		blocksize_bytes = mp->m_sb.sb_blocksize;
>  
>