]> git.ipfire.org Git - thirdparty/xfsprogs-dev.git/commitdiff
xfs: validate extsz hints against rt extent size when rtinherit is set
authorDarrick J. Wong <djwong@kernel.org>
Wed, 30 Jun 2021 22:38:58 +0000 (18:38 -0400)
committerEric Sandeen <sandeen@sandeen.net>
Wed, 30 Jun 2021 22:38:58 +0000 (18:38 -0400)
Source kernel commit: 603f000b15f21ce8932f76689c7aa9fe58261cf5

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, turn off the hint to correct the error.  Second, if
someone tries to misconfigure a directory via the fssetxattr ioctl, fail
the ioctl.  Third, reverify both extent size hint values when we
propagate heritable inode attributes from parent to child, to prevent
misconfigurations from spreading.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
libxfs/libxfs_priv.h
libxfs/xfs_inode_buf.c
libxfs/xfs_trans_inode.c

index 34980c9d1739a3bad738973c1238277341c81b23..7181a858931224f6af4374e4fd85c612450139aa 100644 (file)
@@ -119,6 +119,7 @@ extern char    *progname;
 extern void cmn_err(int, char *, ...);
 enum ce { CE_DEBUG, CE_CONT, CE_NOTE, CE_WARN, CE_ALERT, CE_PANIC };
 
+#define xfs_info(mp,fmt,args...)       cmn_err(CE_CONT, _(fmt), ## args)
 #define xfs_notice(mp,fmt,args...)     cmn_err(CE_NOTE, _(fmt), ## args)
 #define xfs_warn(mp,fmt,args...)       cmn_err(CE_WARN, _(fmt), ## args)
 #define xfs_err(mp,fmt,args...)                cmn_err(CE_ALERT, _(fmt), ## args)
@@ -166,6 +167,23 @@ enum ce { CE_DEBUG, CE_CONT, CE_NOTE, CE_WARN, CE_ALERT, CE_PANIC };
 #define XFS_STATS_ADD(mp, count, x)    do { (mp) = (mp); } while (0)
 #define XFS_TEST_ERROR(expr,a,b)       ( expr )
 
+#define __section(section)     __attribute__((__section__(section)))
+
+#define xfs_printk_once(func, dev, fmt, ...)                   \
+({                                                             \
+       static bool __section(".data.once") __print_once;       \
+       bool __ret_print_once = !__print_once;                  \
+                                                               \
+       if (!__print_once) {                                    \
+               __print_once = true;                            \
+               func(dev, fmt, ##__VA_ARGS__);                  \
+       }                                                       \
+       unlikely(__ret_print_once);                             \
+})
+
+#define xfs_info_once(dev, fmt, ...)                           \
+       xfs_printk_once(xfs_info, dev, fmt, ##__VA_ARGS__)
+
 #ifdef __GNUC__
 #define __return_address       __builtin_return_address(0)
 
index e574bf9a65435a4e513c22caa73fe14bcccb629a..a9ed7f246e0e70da0a54d512b47f8ac103f77b5e 100644 (file)
@@ -586,6 +586,28 @@ xfs_inode_validate_extsize(
        inherit_flag = (flags & XFS_DIFLAG_EXTSZINHERIT);
        extsize_bytes = XFS_FSB_TO_B(mp, extsize);
 
+       /*
+        * This comment describes a historic gap in this verifier function.
+        *
+        * On older kernels, the extent size hint verifier doesn't check that
+        * the extent size hint is an integer multiple of the realtime extent
+        * size on a directory with both RTINHERIT and EXTSZINHERIT flags set.
+        * The verifier has always enforced the alignment rule for regular
+        * files with the REALTIME flag set.
+        *
+        * If a directory with a misaligned extent size hint is allowed to
+        * propagate that hint into a new regular realtime file, the result
+        * is that the inode cluster buffer verifier will trigger a corruption
+        * shutdown the next time it is run.
+        *
+        * Unfortunately, there could be filesystems with these misconfigured
+        * directories in the wild, so we cannot add a check to this verifier
+        * at this time because that will result a new source of directory
+        * corruption errors when reading an existing filesystem.  Instead, we
+        * permit the misconfiguration to pass through the verifiers so that
+        * callers of this function can correct and mitigate externally.
+        */
+
        if (rt_flag)
                blocksize_bytes = mp->m_sb.sb_rextsize << mp->m_sb.sb_blocklog;
        else
index 854446a890f3999585e937731a6569ab682e2b46..c2e9820081d225fd8eb60961e8aeb875f3ff568c 100644 (file)
@@ -139,6 +139,23 @@ xfs_trans_log_inode(
                flags |= XFS_ILOG_CORE;
        }
 
+       /*
+        * Inode verifiers on older kernels don't check that the extent size
+        * hint is an integer multiple of the rt extent size on a directory
+        * with both rtinherit and extszinherit flags set.  If we're logging a
+        * directory that is misconfigured in this way, clear the hint.
+        */
+       if ((ip->i_diflags & XFS_DIFLAG_RTINHERIT) &&
+           (ip->i_diflags & XFS_DIFLAG_EXTSZINHERIT) &&
+           (ip->i_extsize % ip->i_mount->m_sb.sb_rextsize) > 0) {
+               xfs_info_once(ip->i_mount,
+       "Correcting misaligned extent size hint in inode 0x%llx.", ip->i_ino);
+               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