[PATCH v6 10/10] xfs: restart faults racing with realtime flag changes

[Chris Mason <mason@kernel.org>]

On a zoned XFS, a store through a shared mapping can dereference a NULL
pointer, or corrupt the free block counter, if the realtime flag of the
file changes during the write fault.

xfs_write_fault() reserves zoned space only for a zoned inode, and tests
for that with no lock held. xfs_buffered_write_iomap_begin() tests again
before it takes the ILOCK. FS_IOC_FSSETXATTR can change the flag of a
file with no blocks between the two tests, since a fault never takes the
IOLOCK:

    write fault                           FS_IOC_FSSETXATTR
    ===========                           =================
    xfs_write_fault()
      sees that the inode is not zoned
                                          sets the realtime flag
    xfs_buffered_write_iomap_begin()
      sees that the inode is zoned
      /* ac is NULL */

If the flag is cleared instead, xfs_zoned_buffered_write_iomap_begin()
can be waiting for the ILOCK at that time. It then reserves blocks the
zoned way on a regular inode, and the free block counter ends up too
high.

The fix used here is to test again under the ILOCK, before blocks are
reserved or allocated, and to return -EAGAIN if the flag has changed, so
that the fault starts over.

Fixes: 058dd70c65ab ("xfs: implement buffered writes to zoned RT devices")
Signed-off-by: Chris Mason <mason@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_iomap.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index c3465204b8d5..3e71d619c657 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -264,6 +264,23 @@ xfs_iomap_eof_align_last_fsb(
 	return end_fsb;
 }

+/*
+ * A write fault picks the zoned or the regular path before it takes a lock, and
+ * it never takes the IOLOCK, which keeps the realtime flag stable for all other
+ * callers.  The flag cannot change under the ILOCK, or once the inode has
+ * blocks.  So look at it again under the ILOCK, before reserving or allocating
+ * blocks, and make the fault start over if it has changed.
+ */
+static int
+xfs_iomap_rtflag_changed(
+	struct xfs_inode	*ip)
+{
+	xfs_warn_ratelimited(ip->i_mount,
+		"RT flag modifying during page fault for inode 0x%llx.",
+		VFS_I(ip)->i_ino);
+	return -EAGAIN;
+}
+
 int
 xfs_iomap_write_direct(
 	struct xfs_inode	*ip,
@@ -334,6 +351,11 @@ xfs_iomap_write_direct(
 	if (error)
 		return error;

+	if (xfs_is_zoned_inode(ip)) {
+		error = xfs_iomap_rtflag_changed(ip);
+		goto out_trans_cancel;
+	}
+
 	error = xfs_iext_count_extend(tp, ip, XFS_DATA_FORK, nr_exts);
 	if (error)
 		goto out_trans_cancel;
@@ -1668,7 +1690,6 @@ xfs_zoned_buffered_write_iomap_begin(

 	ASSERT(!xfs_get_extsz_hint(ip));
 	ASSERT(!(flags & IOMAP_UNSHARE));
-	ASSERT(ac);

 	if (xfs_is_shutdown(mp))
 		return -EIO;
@@ -1682,6 +1703,11 @@ xfs_zoned_buffered_write_iomap_begin(
 	if (error)
 		return error;

+	if (!xfs_is_zoned_inode(ip) || !ac) {
+		error = xfs_iomap_rtflag_changed(ip);
+		goto out_unlock;
+	}
+
 	if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(&ip->i_df)) ||
 	    XFS_TEST_ERROR(mp, XFS_ERRTAG_BMAPIFORMAT)) {
 		xfs_bmap_mark_sick(ip, XFS_DATA_FORK);
@@ -1865,6 +1891,11 @@ xfs_buffered_write_iomap_begin(
 	if (error)
 		return error;

+	if (xfs_is_zoned_inode(ip)) {
+		error = xfs_iomap_rtflag_changed(ip);
+		goto out_unlock;
+	}
+
 	if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(&ip->i_df)) ||
 	    XFS_TEST_ERROR(mp, XFS_ERRTAG_BMAPIFORMAT)) {
 		xfs_bmap_mark_sick(ip, XFS_DATA_FORK);
--
2.55.0