From: Darrick J. Wong <djwong@xxxxxxxxxx>
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.
Because there may be existing filesystems with such a configuration, we
cannot simply amend the inode verifier to trip on these directories and
call it a day because that will cause previously "working" filesystems
to start throwing errors abruptly. Note that it's valid to have
directories with rtinherit set even if there is no realtime volume, in
which case the problem does not manifest because rtinherit is ignored if
there's no realtime device; and it's possible that someone set the flag,
crashed, repaired the filesystem (which clears the hint on the realtime
file) and continued.
Therefore, mitigate this issue in several ways: First, if we try to
write out an inode with both rtinherit/extszinherit set and an unaligned
extent size hint, we'll simply turn off the hint to correct the error.
Second, if someone tries to misconfigure a file via the fssetxattr
ioctl, we'll fail the ioctl. Third, we reverify both extent size hint
values when we propagate heritable inode attributes from parent to
child, so that we prevent misconfigurations from spreading.
Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
---
fs/xfs/libxfs/xfs_inode_buf.c | 15 ++++++++++++-
fs/xfs/libxfs/xfs_trans_inode.c | 13 +++++++++++
fs/xfs/xfs_inode.c | 46 ++++++++++++++++++++++++++++-----------
fs/xfs/xfs_ioctl.c | 14 ++++++++++++
4 files changed, 74 insertions(+), 14 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 045118c7bf78..dce267dbea5f 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -589,8 +589,21 @@ xfs_inode_validate_extsize(
inherit_flag = (flags & XFS_DIFLAG_EXTSZINHERIT);
extsize_bytes = XFS_FSB_TO_B(mp, extsize);
+ /*
+ * Historically, XFS didn't check that the extent size hint was an
+ * integer multiple of the rt extent size on a directory with both
+ * rtinherit and extszinherit flags set. This results in math errors
+ * in the rt allocator and inode verifier errors when the unaligned
+ * hint value propagates into new realtime files. Since there might
+ * be filesystems in the wild, the best we can do for now is to
+ * mitigate the harms by stopping the propagation.
+ *
+ * The next time we add a new inode feature, the if test below should
+ * also trigger if that new feature is enabled and (rtinherit_flag &&
+ * inherit_flag).
+ */
if (rt_flag)
- blocksize_bytes = mp->m_sb.sb_rextsize << mp->m_sb.sb_blocklog;
+ blocksize_bytes = XFS_FSB_TO_B(mp, mp->m_sb.sb_rextsize);
else
blocksize_bytes = mp->m_sb.sb_blocksize;
diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c
index 78324e043e25..cb5e04ed2654 100644
--- a/fs/xfs/libxfs/xfs_trans_inode.c
+++ b/fs/xfs/libxfs/xfs_trans_inode.c
@@ -142,6 +142,19 @@ xfs_trans_log_inode(
flags |= XFS_ILOG_CORE;
}
+ /*
+ * Clear invalid extent size hints set on files with rtinherit and
+ * extszinherit set. See the comments in xfs_inode_validate_extsize
+ * for details.
+ */
+ if ((ip->i_diflags & XFS_DIFLAG_RTINHERIT) &&
+ (ip->i_diflags & XFS_DIFLAG_EXTSZINHERIT) &&
+ (ip->i_extsize % ip->i_mount->m_sb.sb_rextsize) > 0) {
+ ip->i_diflags &= ~(XFS_DIFLAG_EXTSIZE | XFS_DIFLAG_EXTSZINHERIT);
+ ip->i_extsize = 0;
+ flags |= XFS_ILOG_CORE;
+ }
+
/*
* Record the specific change for fdatasync optimisation. This allows
* fdatasync to skip log forces for inodes that are only timestamp
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 0369eb22c1bb..32458f3f686c 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -689,46 +689,56 @@ xfs_inode_inherit_flags(
struct xfs_inode *ip,
const struct xfs_inode *pip)
{
- unsigned int di_flags = 0;
+ xfs_failaddr_t failaddr;
umode_t mode = VFS_I(ip)->i_mode;
if (S_ISDIR(mode)) {
if (pip->i_diflags & XFS_DIFLAG_RTINHERIT)
- di_flags |= XFS_DIFLAG_RTINHERIT;
+ ip->i_diflags |= XFS_DIFLAG_RTINHERIT;
if (pip->i_diflags & XFS_DIFLAG_EXTSZINHERIT) {
- di_flags |= XFS_DIFLAG_EXTSZINHERIT;
+ ip->i_diflags |= XFS_DIFLAG_EXTSZINHERIT;
ip->i_extsize = pip->i_extsize;
}
if (pip->i_diflags & XFS_DIFLAG_PROJINHERIT)
- di_flags |= XFS_DIFLAG_PROJINHERIT;
+ ip->i_diflags |= XFS_DIFLAG_PROJINHERIT;
} else if (S_ISREG(mode)) {
if ((pip->i_diflags & XFS_DIFLAG_RTINHERIT) &&
xfs_sb_version_hasrealtime(&ip->i_mount->m_sb))
- di_flags |= XFS_DIFLAG_REALTIME;
+ ip->i_diflags |= XFS_DIFLAG_REALTIME;
if (pip->i_diflags & XFS_DIFLAG_EXTSZINHERIT) {
- di_flags |= XFS_DIFLAG_EXTSIZE;
+ ip->i_diflags |= XFS_DIFLAG_EXTSIZE;
ip->i_extsize = pip->i_extsize;
}
}
if ((pip->i_diflags & XFS_DIFLAG_NOATIME) &&
xfs_inherit_noatime)
- di_flags |= XFS_DIFLAG_NOATIME;
+ ip->i_diflags |= XFS_DIFLAG_NOATIME;
if ((pip->i_diflags & XFS_DIFLAG_NODUMP) &&
xfs_inherit_nodump)
- di_flags |= XFS_DIFLAG_NODUMP;
+ ip->i_diflags |= XFS_DIFLAG_NODUMP;
if ((pip->i_diflags & XFS_DIFLAG_SYNC) &&
xfs_inherit_sync)
- di_flags |= XFS_DIFLAG_SYNC;
+ ip->i_diflags |= XFS_DIFLAG_SYNC;
if ((pip->i_diflags & XFS_DIFLAG_NOSYMLINKS) &&
xfs_inherit_nosymlinks)
- di_flags |= XFS_DIFLAG_NOSYMLINKS;
+ ip->i_diflags |= XFS_DIFLAG_NOSYMLINKS;
if ((pip->i_diflags & XFS_DIFLAG_NODEFRAG) &&
xfs_inherit_nodefrag)
- di_flags |= XFS_DIFLAG_NODEFRAG;
+ ip->i_diflags |= XFS_DIFLAG_NODEFRAG;
if (pip->i_diflags & XFS_DIFLAG_FILESTREAM)
- di_flags |= XFS_DIFLAG_FILESTREAM;
+ ip->i_diflags |= XFS_DIFLAG_FILESTREAM;
- ip->i_diflags |= di_flags;
+ /*
+ * Make sure the extsize actually validates properly. See the
+ * comments in xfs_inode_validate_extsize for details.
+ */
+ failaddr = xfs_inode_validate_extsize(ip->i_mount, ip->i_extsize,
+ VFS_I(ip)->i_mode, ip->i_diflags);
+ if (failaddr) {
+ ip->i_diflags &= ~(XFS_DIFLAG_EXTSIZE |
+ XFS_DIFLAG_EXTSZINHERIT);
+ ip->i_extsize = 0;
+ }
}
/* Propagate di_flags2 from a parent inode to a child inode. */
@@ -737,12 +747,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;
+ }
}
/*
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 6407921aca96..0d1d5e32ab9d 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1291,6 +1291,20 @@ xfs_ioctl_setattr_check_extsize(
new_diflags = xfs_flags2diflags(ip, fa->fsx_xflags);
+ /*
+ * Prevent the sysadmin from misconfiguring files to have an invalid
+ * extent size hint set if rtinherit and extszinherit are set. See the
+ * comments in xfs_inode_validate_extsize for details.
+ */
+ if ((new_diflags & XFS_DIFLAG_RTINHERIT) &&
+ (new_diflags & XFS_DIFLAG_EXTSZINHERIT)) {
+ unsigned int rtextsize_bytes;
+
+ rtextsize_bytes = XFS_FSB_TO_B(mp, mp->m_sb.sb_rextsize);
+ if (fa->fsx_extsize % rtextsize_bytes)
+ return -EINVAL;
+ }
+
failaddr = xfs_inode_validate_extsize(ip->i_mount,
XFS_B_TO_FSB(mp, fa->fsx_extsize),
VFS_I(ip)->i_mode, new_diflags);
