On Fri, May 14, 2021 at 08:38:27AM -0400, Brian Foster wrote:
> On Wed, May 12, 2021 at 06:01:53PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
>
> 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?
Yes. The incompatible combination when set on a directory is benign,
but setting it on regular files gets us into real trouble, so the goal
here is to end the propagation of the incompatible values.
--D
> 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;
> > + }
> > }
> >
> > /*
> >
>
