Skip to content

Commit 83193e5

Browse files
author
Darrick J. Wong
committed
xfs: correct the narrative around misaligned rtinherit/extszinherit dirs
While auditing the realtime growfs code, I realized that the GROWFSRT ioctl (and by extension xfs_growfs) has always allowed sysadmins to change the realtime extent size when adding a realtime section to the filesystem. Since we also have always allowed sysadmins to set RTINHERIT and EXTSZINHERIT on directories even if there is no realtime device, this invalidates the premise laid out in the comments added in commit 603f000. In other words, this is not a case of inadequate metadata validation. This is a case of nearly forgotten (and apparently untested) but supported functionality. Update the comments to reflect what we've learned, and remove the log message about correcting the misalignment. Fixes: 603f000 ("xfs: validate extsz hints against rt extent size when rtinherit is set") Signed-off-by: Darrick J. Wong <[email protected]> Reviewed-by: Carlos Maiolino <[email protected]> Reviewed-by: Christoph Hellwig <[email protected]>
1 parent 5838d03 commit 83193e5

File tree

3 files changed

+24
-22
lines changed

3 files changed

+24
-22
lines changed

fs/xfs/libxfs/xfs_inode_buf.c

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -592,23 +592,27 @@ xfs_inode_validate_extsize(
592592
/*
593593
* This comment describes a historic gap in this verifier function.
594594
*
595-
* On older kernels, the extent size hint verifier doesn't check that
596-
* the extent size hint is an integer multiple of the realtime extent
597-
* size on a directory with both RTINHERIT and EXTSZINHERIT flags set.
598-
* The verifier has always enforced the alignment rule for regular
599-
* files with the REALTIME flag set.
595+
* For a directory with both RTINHERIT and EXTSZINHERIT flags set, this
596+
* function has never checked that the extent size hint is an integer
597+
* multiple of the realtime extent size. Since we allow users to set
598+
* this combination on non-rt filesystems /and/ to change the rt
599+
* extent size when adding a rt device to a filesystem, the net effect
600+
* is that users can configure a filesystem anticipating one rt
601+
* geometry and change their minds later. Directories do not use the
602+
* extent size hint, so this is harmless for them.
600603
*
601604
* If a directory with a misaligned extent size hint is allowed to
602605
* propagate that hint into a new regular realtime file, the result
603606
* is that the inode cluster buffer verifier will trigger a corruption
604-
* shutdown the next time it is run.
607+
* shutdown the next time it is run, because the verifier has always
608+
* enforced the alignment rule for regular files.
605609
*
606-
* Unfortunately, there could be filesystems with these misconfigured
607-
* directories in the wild, so we cannot add a check to this verifier
608-
* at this time because that will result a new source of directory
609-
* corruption errors when reading an existing filesystem. Instead, we
610-
* permit the misconfiguration to pass through the verifiers so that
611-
* callers of this function can correct and mitigate externally.
610+
* Because we allow administrators to set a new rt extent size when
611+
* adding a rt section, we cannot add a check to this verifier because
612+
* that will result a new source of directory corruption errors when
613+
* reading an existing filesystem. Instead, we rely on callers to
614+
* decide when alignment checks are appropriate, and fix things up as
615+
* needed.
612616
*/
613617

614618
if (rt_flag)

fs/xfs/libxfs/xfs_trans_inode.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -143,16 +143,14 @@ xfs_trans_log_inode(
143143
}
144144

145145
/*
146-
* Inode verifiers on older kernels don't check that the extent size
147-
* hint is an integer multiple of the rt extent size on a directory
148-
* with both rtinherit and extszinherit flags set. If we're logging a
149-
* directory that is misconfigured in this way, clear the hint.
146+
* Inode verifiers do not check that the extent size hint is an integer
147+
* multiple of the rt extent size on a directory with both rtinherit
148+
* and extszinherit flags set. If we're logging a directory that is
149+
* misconfigured in this way, clear the hint.
150150
*/
151151
if ((ip->i_diflags & XFS_DIFLAG_RTINHERIT) &&
152152
(ip->i_diflags & XFS_DIFLAG_EXTSZINHERIT) &&
153153
(ip->i_extsize % ip->i_mount->m_sb.sb_rextsize) > 0) {
154-
xfs_info_once(ip->i_mount,
155-
"Correcting misaligned extent size hint in inode 0x%llx.", ip->i_ino);
156154
ip->i_diflags &= ~(XFS_DIFLAG_EXTSIZE |
157155
XFS_DIFLAG_EXTSZINHERIT);
158156
ip->i_extsize = 0;

fs/xfs/xfs_ioctl.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1292,10 +1292,10 @@ xfs_ioctl_setattr_check_extsize(
12921292
new_diflags = xfs_flags2diflags(ip, fa->fsx_xflags);
12931293

12941294
/*
1295-
* Inode verifiers on older kernels don't check that the extent size
1296-
* hint is an integer multiple of the rt extent size on a directory
1297-
* with both rtinherit and extszinherit flags set. Don't let sysadmins
1298-
* misconfigure directories.
1295+
* Inode verifiers do not check that the extent size hint is an integer
1296+
* multiple of the rt extent size on a directory with both rtinherit
1297+
* and extszinherit flags set. Don't let sysadmins misconfigure
1298+
* directories.
12991299
*/
13001300
if ((new_diflags & XFS_DIFLAG_RTINHERIT) &&
13011301
(new_diflags & XFS_DIFLAG_EXTSZINHERIT)) {

0 commit comments

Comments
 (0)