[PATCH v9 12/14] 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 if the realtime flag of the file is set as the write fault
starts.

xfs_write_fault() reserves zoned space only for a zoned inode. The
reservation can wait for garbage collection, which takes the MMAPLOCK,
so it comes ahead of that lock, and so does the test for a zoned inode.
FS_IOC_FSSETXATTR can set the flag of a file with no blocks right after
the test:

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

The fix used here is to test again under the MMAPLOCK, which keeps the
ioctl out since "xfs: take the MMAPLOCK in xfs_fileattr_set()", and to
start the fault over if the flag has changed.

Fixes: 058dd70c65ab ("xfs: implement buffered writes to zoned RT devices")
Signed-off-by: Chris Mason <mason@kernel.org>
---

Notes:
    v8->v9:
    - The flag is tested once, in __xfs_write_fault() under the MMAPLOCK.
      The four tests under the ILOCK are gone, and so is the -EINVAL for
      callers that are no faults (Christoph Hellwig).
    - Was 11/15.
    
    v7->v8:
    - Only a page fault gets -EAGAIN. Any other caller that gets to one of
      the tests fails with -EINVAL, as a write and an fallocate do. pNFS can
      get there: xfs_fs_map_blocks() tests the flag before it takes its
      locks (Christoph Hellwig).
    - The warning now says "during block mapping".
    - The helper reads the inode number with I_INO(), and its comment no
      longer says that the flag is stable once the inode has blocks.
    - Was 11/11.
    
    v6->v7:
    - xfs_direct_write_iomap_begin() tests the flag too, as soon as it holds
      the ILOCK. It could allocate blocks for a COW fork extent before it
      got to the test in xfs_iomap_write_direct().
    - The assert on the extent size hint in
      xfs_zoned_buffered_write_iomap_begin() moved below the test of the
      flag.
    - Dropped the Reviewed-by of Christoph Hellwig, since the patch changed.
    - Was 10/10.

 fs/xfs/xfs_file.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 78c9668c806b..6ff544c0934d 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -2076,11 +2076,22 @@ __xfs_write_fault(
 		lock_mode = XFS_MMAPLOCK_EXCL;
 	}
 
-	if (IS_DAX(inode))
+	/*
+	 * The caller picked the zoned or the regular path with no lock held.
+	 * The realtime flag cannot change under the MMAPLOCK, so look at it
+	 * again, and start over if it has changed.
+	 */
+	if (xfs_is_zoned_inode(ip) != !!ac) {
+		xfs_warn_ratelimited(ip->i_mount,
+			"RT flag modifying during page fault for inode 0x%llx.",
+			inode->i_ino);
+		ret = VM_FAULT_NOPAGE;
+	} else if (IS_DAX(inode)) {
 		ret = xfs_dax_fault_locked(vmf, order, true);
-	else
+	} else {
 		ret = iomap_page_mkwrite(vmf, &xfs_buffered_write_iomap_ops,
 				ac);
+	}
 	xfs_iunlock(ip, lock_mode);
 
 	sb_end_pagefault(inode->i_sb);
-- 
2.55.0