[PATCH v8 00/15] reflink and atomic write bugs

[PATCH v8 00/15] reflink and atomic write bugs

[Chris Mason]

[ the send command got interrupted apologies for the two dups ]

This series did a few rounds privately, but looks ready for the list
now.  Carlos has accepted patches 1-4, but I've kept them here so the
whole series can still be tested.

Except for Christoph's patch, these were all found and fixed
by an LLM.

The meat of the series is fixing reflink corner cases that incorrectly
let you write into the source of a reflink.  LLMs also tripped over a
few ways to use atomic writes to expose stale blocks on the drive, and a
few races with realtime flag and extent size hint manipulations.

v7->v8:

Christoph pointed out that only page faults can cope with the -EAGAIN
from "xfs: restart faults racing with realtime flag changes".  pNFS is
the one other caller that can really get there, so now faults get
-EAGAIN and everyone else gets -EINVAL.

Sashiko had two complaints about the same patch.  The stale reservation
is fixed by the two "resample" patches.  Those are the rest of the 3
additional patches I promised in v7, I just managed to leave them out of
the posting.

The second complaint turned out to be on disk corruption for realtime
filesystems with extents bigger than one block, with nothing returning
an error.  That's the new "xfs: test for an extsize hint again under the
ILOCK".

While checking what a DAX fault does with the -EAGAIN, we found that DAX
and zoned inodes don't work together at all.  With a zoned filesystem on
pmem and dax=always, the first store into a mapped file oopses on
7.4-rc2, and so does ftruncate().  "xfs: don't enable DAX on zoned
inodes" turns DAX off for zoned inodes.

Added Christoph's review to "xfs: test for always COW after the dio
write checks".

v6->v7:

Sashiko noticed a few pre-existing corners that I had ignored, so we
have 3 additional patches dealing with races between FS_IOC_FSSETXATTR
and decisions about file writes.

"xfs: restart faults racing with realtime flag changes" changed enough
that I dropped Christoph's review.

Chris Mason (14):
  xfs: don't skip the flush before shifting extents
  xfs: flush the shifted range before changing the file
  xfs: don't change the block map under a dirty folio
  iomap: don't punch delalloc under a dirty EOF block
  fs: validate atomic writes after O_APPEND and limits
  xfs: recheck direct write alignment after write checks
  xfs: test for reflink after the dio write checks
  xfs: test for always COW after the dio write checks
  xfs: refuse fallocate racing with realtime flag changes
  xfs: restart faults racing with realtime flag changes
  xfs: test for an extsize hint again under the ILOCK
  xfs: resample the extsize hint after taking the ILOCK
  xfs: resample the COW extsize hint after cycling ILOCK
  xfs: don't enable DAX on zoned inodes

Christoph Hellwig (1):
  xfs: refuse writes to files racing with realtime inode flag changes

 fs/ext4/file.c         |   4 -
 fs/iomap/buffered-io.c |  34 ++++++++-
 fs/read_write.c        |  13 ++++
 fs/xfs/xfs_bmap_util.c |  19 ++++-
 fs/xfs/xfs_file.c      | 167 ++++++++++++++++++++++++++++++++---------
 fs/xfs/xfs_ioctl.c     |   8 +-
 fs/xfs/xfs_iomap.c     |  73 ++++++++++++++++--
 fs/xfs/xfs_iops.c      |   4 +
 fs/xfs/xfs_reflink.c   |  16 +++-
 9 files changed, 282 insertions(+), 56 deletions(-)

[PATCH v8 01/15] xfs: don't skip the flush before shifting extents

[Chris Mason]

On an XFS file larger than 4 EiB, collapse range and insert range can
drop dirty data without writing it back, and then fail with
EFSCORRUPTED.

xfs_prepare_shift() flushes the page cache from its offset to EOF, but
passes i_size as the length. Above 4 EiB the end of that range can
overflow and turn negative. filemap_write_and_wait_range() then writes
nothing, but truncate_pagecache_range() takes the end as unsigned and
drops every folio from the start of the range onward.

A delalloc extent under a dropped folio stays in the data fork, and the
shift returns -EFSCORRUPTED for it.

The fix used here is to pass i_size - offset as the length.

Fixes: 7f9f71be84bc ("xfs: extent shifting doesn't fully invalidate page cache")
Fixes: feeb9b269523 ("filemap: skip write and wait if end offset precedes start")
Signed-off-by: Chris Mason <mason@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---

Notes:
    v7->v8:
    - No change.
    
    v6->v7:
    - No change.

 fs/xfs/xfs_bmap_util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 865aec3ae5ab..537c6b29417c 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -980,7 +980,7 @@ xfs_prepare_shift(
 	 * Writeback and invalidate cache for the remainder of the file as we're
 	 * about to shift down every extent from offset to EOF.
 	 */
-	error = xfs_flush_unmap_range(ip, offset, XFS_ISIZE(ip));
+	error = xfs_flush_unmap_range(ip, offset, XFS_ISIZE(ip) - offset);
 	if (error)
 		return error;
 
-- 
2.55.0

[PATCH v8 02/15] xfs: flush the shifted range before changing the file

[Chris Mason]

If writeback of the range to be shifted fails, collapse range on XFS can
fail with its hole already punched, and insert range with the file
already grown.

Both calls change the file first: collapse range punches its hole, and
insert range grows i_size. Only then does xfs_prepare_shift() flush the
range from the offset to EOF, and that flush fails if the writeback
fails.

The fix used here is to flush and invalidate that range before the file
is changed.

Signed-off-by: Chris Mason <mason@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---

Notes:
    v7->v8:
    - No change.
    
    v6->v7:
    - No change.

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

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 396aca8c66a9..be1f775128d1 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1201,6 +1201,22 @@ xfs_falloc_setsize(
 			&iattr);
 }
 
+/*
+ * Collapse and insert range change the file in several steps, and each step
+ * writes back and empties the part of the page cache that it needs.  That can
+ * fail.  Do it here for everything that is going to move, before the first
+ * step, so that a failure leaves the file as it was.  xfs_prepare_shift()
+ * starts one allocation unit before the offset, so do the same.
+ */
+static int
+xfs_falloc_flush_shift_range(
+	struct xfs_inode	*ip,
+	loff_t			offset)
+{
+	offset -= min_t(loff_t, offset, xfs_inode_alloc_unitsize(ip));
+	return xfs_flush_unmap_range(ip, offset, XFS_ISIZE(ip) - offset);
+}
+
 static int
 xfs_falloc_collapse_range(
 	struct file		*file,
@@ -1222,6 +1238,10 @@ xfs_falloc_collapse_range(
 	if (offset + len >= i_size_read(inode))
 		return -EINVAL;
 
+	error = xfs_falloc_flush_shift_range(XFS_I(inode), offset);
+	if (error)
+		return error;
+
 	error = xfs_collapse_file_space(XFS_I(inode), offset, len, ac);
 	if (error)
 		return error;
@@ -1263,9 +1283,10 @@ xfs_falloc_insert_range(
 	 * fork. This shifts back out beyond EOF, but creates an expectedly
 	 * written post-eof block. The insert is going to flush, unmap and
 	 * cancel prealloc across this whole range, so flush EOF now before we
-	 * bump i_size to provide consistent behavior.
+	 * bump i_size to provide consistent behavior.  Flush and unmap the rest
+	 * of the range as well, so that a failure to do so leaves i_size alone.
 	 */
-	error = filemap_write_and_wait_range(inode->i_mapping, isize, isize);
+	error = xfs_falloc_flush_shift_range(XFS_I(inode), offset);
 	if (error)
 		return error;
 
-- 
2.55.0

[PATCH v8 03/15] xfs: don't change the block map under a dirty folio

[Chris Mason]

With CONFIG_TRANSPARENT_HUGEPAGE=y, writeback on XFS can write in place
to a shared block and corrupt the other file that shares it.

xfs_map_blocks() writes in place unless the COW fork has an extent for
the block. write() and write faults reserve one before they dirty a
folio over a shared block, so xfs_flush_unmap_range() writes back and
truncates the range before a clone changes the block map. But the holder
of a GUP write pin takes no filesystem lock, and the truncate keeps a
large dirty folio that straddles the range:

    clone                         pin holder         writeback
    =====                         ==========         =========
    writes the range back
                                  dirties the folio
    truncate keeps the folio
    maps a shared block under it
                                  dirties it again
                                                     finds no COW extent
                                                     writes in place
                                                     /* shared block */

The fix used here is to invalidate the range instead of truncating it,
and to fail the call with -EINVAL if a dirty folio is left, before the
block map changes.

Fixes: 6795801366da ("xfs: Support large folios")
Signed-off-by: Chris Mason <mason@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---

Notes:
    v7->v8:
    - No change.
    
    v6->v7:
    - No change.

 fs/xfs/xfs_bmap_util.c | 17 +++++++++++++++--
 fs/xfs/xfs_reflink.c   |  2 +-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 537c6b29417c..4673d5174645 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -863,8 +863,21 @@ xfs_flush_unmap_range(
 	error = filemap_write_and_wait_range(inode->i_mapping, start, end);
 	if (error)
 		return error;
-	truncate_pagecache_range(inode, start, end);
-	return 0;
+
+	/*
+	 * The callers go on to change the block map, so no folio may stay over
+	 * the range.  The holder of a GUP pin can mark a folio dirty again
+	 * after the flush, with no lock of ours.  A truncate drops such a folio
+	 * if it lies inside the range.  If it is a large folio that straddles
+	 * the range, the truncate cannot split it, so it zeroes a part of the
+	 * folio, keeps it, and reports nothing.  The invalidation removes whole
+	 * folios, changes no data, and fails on a dirty folio.
+	 */
+	error = invalidate_inode_pages2_range(inode->i_mapping,
+			start >> PAGE_SHIFT, end >> PAGE_SHIFT);
+	if (error == -EBUSY)
+		error = -EINVAL;
+	return error;
 }
 
 int
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 104542b10d9e..ff89516154bd 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -1663,7 +1663,7 @@ xfs_reflink_zero_posteof(
 /*
  * Prepare two files for range cloning.  Upon a successful return both inodes
  * will have the iolock and mmaplock held, the page cache of the out file will
- * be truncated, and any leases on the out file will have been broken.  This
+ * be invalidated, and any leases on the out file will have been broken.  This
  * function borrows heavily from xfs_file_aio_write_checks.
  *
  * The VFS allows partial EOF blocks to "match" for dedupe even though it hasn't
-- 
2.55.0

[PATCH v8 04/15] iomap: don't punch delalloc under a dirty EOF block

[Chris Mason]

After a failed buffered write, writeback on XFS can write in place to
the shared block that contains EOF, and corrupt the other file that
shares it.

After a failed buffered write, iomap_write_delalloc_release() punches
the delalloc reservation of the write, except under dirty folios, which
still need it. It looks for dirty folios only up to i_size, and punches
the rest unconditionally. But XFS rounds that punch down to a block
boundary, so the block that contains EOF loses its COW fork reservation,
and writeback goes to the shared block:

    block size 4096, i_size 5000, reservation 4096 to 8192
    searched for dirty folios:  4096 to 5000
    punched unconditionally:    5000 to 8192
    rounded to blocks by XFS:   4096 to 8192

The fix used here is to look for dirty folios up to the end of the block
that contains EOF.

Fixes: f43dc4dc3eff ("iomap: buffered write failure should not truncate the page cache")
Fixes: f6f91d290c8b ("xfs: punch delalloc extents from the COW fork for COW writes")
Signed-off-by: Chris Mason <mason@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---

Notes:
    v7->v8:
    - No change.
    
    v6->v7:
    - No change.

 fs/iomap/buffered-io.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index d6e2f1ded5ca..d0980a7f27d1 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -1495,9 +1495,34 @@ static void iomap_write_delalloc_scan(struct inode *inode,
  * mapping_seek_hole_data() will return it, and we will skip the punch because
  * the folio is dirty. THis is incorrect - we always need to punch out delalloc
  * beyond EOF in this case as writeback will never write back and covert that
- * delalloc block beyond EOF. Hence we limit the cached data scan range to EOF,
- * resulting in always punching out the range from the EOF to the end of the
- * range the iomap spans.
+ * delalloc block beyond EOF. Hence we limit the cached data scan range, and
+ * always punch out the range from the end of the scan to the end of the range
+ * the iomap spans.
+ *
+ * The scan cannot simply end at EOF, because EOF is a byte offset and a
+ * delalloc reservation covers whole blocks. If EOF lies inside a block, it
+ * splits that block in two:
+ *
+ *	[block start, EOF)	File data. If it is dirty it will be written
+ *				back, so the block must keep its delalloc
+ *				backing.
+ *	[EOF, block end)	Beyond EOF. By the rule above, this would always
+ *				be punched out.
+ *
+ * Both parts share one reservation, and it cannot be split: a punch of a part
+ * of a block takes away the reservation for the whole block. If the scan ended
+ * at EOF, we would skip the punch for the first part because it is dirty, and
+ * then punch the second part, which takes the reservation away from under the
+ * dirty data in the first part. On XFS, writeback that has lost a COW fork
+ * reservation like this writes in place, over a block that is shared with
+ * another file.
+ *
+ * Hence we treat the EOF block as a whole, and let the cached data over it
+ * decide: the scan range is rounded up to the end of the block that EOF lies
+ * in. If the data over the EOF block is dirty, the scan moves the punch start
+ * past the block. If it is clean, the block is punched like any other block in
+ * the range. Only blocks that start at or beyond EOF are punched
+ * unconditionally. If EOF is block aligned, the round up changes nothing.
  *
  * Intervals are of the form [start_byte, end_byte) (i.e. open ended) because it
  * matches the intervals returned by mapping_seek_hole_data(). i.e. SEEK_DATA
@@ -1511,7 +1536,8 @@ void iomap_write_delalloc_release(struct inode *inode, loff_t start_byte,
 		iomap_punch_t punch)
 {
 	loff_t punch_start_byte = start_byte;
-	loff_t scan_end_byte = min(i_size_read(inode), end_byte);
+	loff_t scan_end_byte = min_t(u64, end_byte,
+			round_up((u64)i_size_read(inode), i_blocksize(inode)));
 
 	/*
 	 * The caller must hold invalidate_lock to avoid races with page faults
-- 
2.55.0

[PATCH v8 05/15] fs: validate atomic writes after O_APPEND and limits

[Chris Mason]

An RWF_ATOMIC write on XFS with reflink can expose stale disk contents
if O_APPEND or RLIMIT_FSIZE leaves it sector aligned but not block
aligned.

XFS and ext4 call generic_atomic_write_valid() on the range that the
caller passed. But generic_write_checks() changes the range later:
O_APPEND moves the position, and RLIMIT_FSIZE cuts the length. XFS can
then fill only a part of a new block, and still map the whole block into
the file:

    block size 4096, i_size 512, O_APPEND
    validated:        0 to 4095
    written:          512 to 4607
    new blocks:       0 to 8191
    stale below EOF:  0 to 511

The fix used here is to validate an atomic write in
generic_write_checks(), after its range is final, and to refuse one that
was cut. XFS no longer picks the path of an atomic write by the
alignment of the range that the caller passed.

Fixes: f096207d3276 ("xfs: Validate atomic writes")
Fixes: 43c696f9d094 ("ext4: Check for atomic writes support in write iter")
Signed-off-by: Chris Mason <mason@kernel.org>
---

Notes:
    v7->v8:
    - No change.
    
    v6->v7:
    - No change.

 fs/ext4/file.c    |  4 ----
 fs/read_write.c   | 13 +++++++++++++
 fs/xfs/xfs_file.c | 17 +++++++++++------
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 50d3c92709c8..e301d4faf1c3 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -802,10 +802,6 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 		if (len < EXT4_SB(inode->i_sb)->s_awu_min ||
 		    len > EXT4_SB(inode->i_sb)->s_awu_max)
 			return -EINVAL;
-
-		ret = generic_atomic_write_valid(iocb, from);
-		if (ret)
-			return ret;
 	}
 
 	if (iocb->ki_flags & IOCB_DIRECT)
diff --git a/fs/read_write.c b/fs/read_write.c
index 676728310899..1b34b7382619 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1775,6 +1775,19 @@ ssize_t generic_write_checks(struct kiocb *iocb, struct iov_iter *from)
 	if (ret)
 		return ret;
 
+	/*
+	 * O_APPEND has moved the position and the file size limits have
+	 * shortened the write by now, so this is the range to validate.  An
+	 * atomic write is all or nothing, so refuse it if it got shorter.
+	 */
+	if (iocb->ki_flags & IOCB_ATOMIC) {
+		if (count != iov_iter_count(from))
+			return -EINVAL;
+		ret = generic_atomic_write_valid(iocb, from);
+		if (ret)
+			return ret;
+	}
+
 	iov_iter_truncate(from, count);
 	return iov_iter_count(from);
 }
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index be1f775128d1..153f53f964b9 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -926,12 +926,21 @@ xfs_file_dio_write(
 	if ((iocb->ki_pos | count) & target->bt_logical_sectormask)
 		return -EINVAL;
 
+	/*
+	 * An atomic write is at least one block long, and generic_write_checks()
+	 * refuses it unless its final position is a multiple of its length.  So
+	 * there is no unaligned case to handle for it.
+	 */
+	if (iocb->ki_flags & IOCB_ATOMIC) {
+		if (xfs_is_zoned_inode(ip))
+			return xfs_file_dio_write_zoned(ip, iocb, from);
+		return xfs_file_dio_write_atomic(ip, iocb, from);
+	}
+
 	if ((iocb->ki_pos | count) & ip->i_mount->m_blockmask)
 		return xfs_file_dio_write_unaligned(ip, iocb, from);
 	if (xfs_is_zoned_inode(ip))
 		return xfs_file_dio_write_zoned(ip, iocb, from);
-	if (iocb->ki_flags & IOCB_ATOMIC)
-		return xfs_file_dio_write_atomic(ip, iocb, from);
 	return xfs_file_dio_write_aligned(ip, iocb, from,
 			&xfs_direct_write_iomap_ops, &xfs_dio_write_ops, NULL);
 }
@@ -1128,10 +1137,6 @@ xfs_file_write_iter(
 
 		if (ocount > xfs_get_atomic_write_max(ip))
 			return -EINVAL;
-
-		ret = generic_atomic_write_valid(iocb, from);
-		if (ret)
-			return ret;
 	}
 
 	if (IS_DAX(inode))
-- 
2.55.0

[PATCH v8 06/15] xfs: recheck direct write alignment after write checks

[Chris Mason]

A direct write to a reflinked file on XFS can expose stale disk contents
if O_APPEND or RLIMIT_FSIZE leaves it sector aligned but not block
aligned.

A direct write cannot fill only a part of a shared block, so XFS does
such a write through the page cache. xfs_file_dio_write() decides that
from the range that the caller passed. But O_APPEND and RLIMIT_FSIZE
change the range later, in xfs_file_write_checks():

    block size 4096, i_size 4608, O_APPEND, block 1 is shared
    checked:        0 to 4095, block aligned
    written:        4608 to 8703
    new COW block:  4096 to 8191
    never written:  4096 to 4607

The fix used here is to check the alignment again once the range is
final, and to start over on the unaligned path if it no longer holds. A
zoned file now falls back to buffered I/O ahead of that path.

Fixes: 0613f16cd201 ("xfs: implement CoW for directio writes")
Fixes: 3c68d44a2b49 ("xfs: allocate direct I/O COW blocks in iomap_begin")
Signed-off-by: Chris Mason <mason@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---

Notes:
    v7->v8:
    - No change.
    
    v6->v7:
    - No change.

 fs/xfs/xfs_file.c | 44 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 153f53f964b9..8ee88910b487 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -710,6 +710,17 @@ xfs_file_dio_write_aligned(
 	if (ret)
 		goto out_unlock;
 
+	/*
+	 * O_APPEND moves the position and the file size limits shorten the
+	 * write, so only now is the range final.  If it is not block aligned,
+	 * this is the wrong path for it.  Return -EAGAIN to
+	 * xfs_file_dio_write(), which picks the path.
+	 */
+	if ((iocb->ki_pos | iov_iter_count(from)) & ip->i_mount->m_blockmask) {
+		ret = -EAGAIN;
+		goto out_unlock;
+	}
+
 	/*
 	 * We don't need to hold the IOLOCK exclusively across the IO, so demote
 	 * the iolock back to shared if we had to take the exclusive lock in
@@ -921,6 +932,7 @@ xfs_file_dio_write(
 	struct xfs_inode	*ip = XFS_I(file_inode(iocb->ki_filp));
 	struct xfs_buftarg      *target = xfs_inode_buftarg(ip);
 	size_t			count = iov_iter_count(from);
+	ssize_t			ret;
 
 	/* direct I/O must be aligned to device logical sector size */
 	if ((iocb->ki_pos | count) & target->bt_logical_sectormask)
@@ -937,12 +949,32 @@ xfs_file_dio_write(
 		return xfs_file_dio_write_atomic(ip, iocb, from);
 	}
 
-	if ((iocb->ki_pos | count) & ip->i_mount->m_blockmask)
-		return xfs_file_dio_write_unaligned(ip, iocb, from);
-	if (xfs_is_zoned_inode(ip))
-		return xfs_file_dio_write_zoned(ip, iocb, from);
-	return xfs_file_dio_write_aligned(ip, iocb, from,
-			&xfs_direct_write_iomap_ops, &xfs_dio_write_ops, NULL);
+	/*
+	 * O_APPEND and the file size limits can still change the range, in
+	 * xfs_file_write_checks().  So a range that is block aligned here may
+	 * not stay so, and the write then comes back with -EAGAIN.  If we are
+	 * doing nonblocking user I/O, propagate the error.
+	 */
+	if (!((iocb->ki_pos | count) & ip->i_mount->m_blockmask)) {
+		if (xfs_is_zoned_inode(ip))
+			ret = xfs_file_dio_write_zoned(ip, iocb, from);
+		else
+			ret = xfs_file_dio_write_aligned(ip, iocb, from,
+					&xfs_direct_write_iomap_ops,
+					&xfs_dio_write_ops, NULL);
+		if (ret != -EAGAIN || (iocb->ki_flags & IOCB_NOWAIT))
+			return ret;
+	}
+
+	/*
+	 * Zoned inodes always write out of place, which does not work for a
+	 * part of a block.  Fall back to buffered I/O.
+	 */
+	if (xfs_is_zoned_inode(ip)) {
+		trace_xfs_reflink_bounce_dio_write(iocb, from);
+		return -ENOTBLK;
+	}
+	return xfs_file_dio_write_unaligned(ip, iocb, from);
 }
 
 static noinline ssize_t
-- 
2.55.0

[PATCH v8 07/15] xfs: test for reflink after the dio write checks

[Chris Mason]

A direct write that is not block aligned can expose stale disk contents
on XFS if it races with the first clone of the file.

xfs_file_dio_write_unaligned() falls back to buffered I/O if the file is
reflinked, and tests for that right after it takes the iolock. But
xfs_file_write_checks() runs after the test and can drop the iolock, for
example if S_NOSEC is not set. A clone can run in that gap:

    direct write                        clone
    ============                        =====
    takes the iolock shared
    sees that the file is not reflinked
    drops the iolock
                                        shares the blocks
    takes the iolock exclusive
    writes a part of a new COW block
    maps the whole block into the file
      /* the rest was never written */

The fix used here is to move the test after xfs_file_write_checks().

Fixes: ed1128c2d0c8 ("xfs: reduce exclusive locking on unaligned dio")
Signed-off-by: Chris Mason <mason@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---

Notes:
    v7->v8:
    - No change.
    
    v6->v7:
    - No change.

 fs/xfs/xfs_file.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 8ee88910b487..d888470d092a 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -877,9 +877,15 @@ xfs_file_dio_write_unaligned(
 	if (ret)
 		return ret;
 
+	ret = xfs_file_write_checks(iocb, from, &iolock, NULL);
+	if (ret)
+		goto out_unlock;
+
 	/*
 	 * We can't properly handle unaligned direct I/O to reflink files yet,
-	 * as we can't unshare a partial block.
+	 * as we can't unshare a partial block.  xfs_file_write_checks() can
+	 * drop the iolock and take it again, and the file can be reflinked in
+	 * between, so test for that only now.
 	 */
 	if (xfs_is_cow_inode(ip)) {
 		trace_xfs_reflink_bounce_dio_write(iocb, from);
@@ -887,10 +893,6 @@ xfs_file_dio_write_unaligned(
 		goto out_unlock;
 	}
 
-	ret = xfs_file_write_checks(iocb, from, &iolock, NULL);
-	if (ret)
-		goto out_unlock;
-
 	/*
 	 * If we are doing exclusive unaligned I/O, this must be the only I/O
 	 * in-flight.  Otherwise we risk data corruption due to unwritten extent
-- 
2.55.0

[PATCH v8 08/15] xfs: refuse writes to files racing with realtime inode flag changes

[Chris Mason]

From: Christoph Hellwig <hch@lst.de>

Zoned writes need to reserve space before taking the iolock to avoid
deadlocks with garbage collection.  But this means that the realtime inode
flag which indicates if the file will be placed on the zoned realtime
device or the main data device can be flipped after taking the reservation
and before taking the write, or after deciding to go down the regular
write path, both of which can't work.

Re-sample the realtime flag just before the actual write, and refuse the
write if the flag changed.  Do not retry the write using the now correct
path, as switching the flag under a write is not a normal use case, and
trying to flip between the zoned and regular path is an inherent life
lock as it could be switched around again the next time this check is
performed.

Fixes: 058dd70c65ab ("xfs: implement buffered writes to zoned RT devices")
Fixes: 2e2383405824 ("xfs: implement direct writes to zoned RT devices")
Reported-by: Chris Mason <mason@kernel.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Chris Mason <mason@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
---

Notes:
    v7->v8:
    - No change.
    
    v6->v7:
    - No change.

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

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index d888470d092a..515ddafc6673 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -441,6 +441,19 @@ xfs_file_write_checks(
 		goto restart;
 	}
 
+	/*
+	 * Now that we are in the same IOLOCK critical section as the actual
+	 * write, check that the inode still has the realtime flag in the state
+	 * that the caller expects.  If not, some deliberately cleared it while
+	 * I/O is ongoing, so bail out.
+	 */
+	if (xfs_is_zoned_inode(XFS_I(inode)) != !!ac) {
+		xfs_warn_ratelimited(XFS_I(inode)->i_mount,
+			"RT flag modifying during write for inode 0x%llx.",
+			inode->i_ino);
+		return -EINVAL;
+	}
+
 	/*
 	 * If the offset is beyond the size of the file, we need to zero all
 	 * blocks that fall between the existing EOF and the start of this
-- 
2.55.0

[PATCH v8 09/15] xfs: test for always COW after the dio write checks

[Chris Mason]

A direct write to a zoned file from vectors that are not block aligned
can silently corrupt the file's data, if the realtime flag is cleared
and set again while the write starts.

Every bio of an always COW write has to cover whole blocks, so
xfs_file_dio_write_aligned() passes IOMAP_DIO_FSBLOCK_ALIGNED for such
an inode. But it tests for always COW before it takes the IOLOCK, and
the realtime flag of a file with no blocks can change until then:

    xfs_file_dio_write_zoned()            FS_IOC_FSSETXATTR
    ==========================            =================
    xfs_zoned_write_space_reserve()
                                          clears the realtime flag
    xfs_file_dio_write_aligned()
      xfs_is_always_cow_inode() is false
                                          sets the realtime flag
      xfs_ilock_iocb_for_write()
      xfs_file_write_checks()
      iomap_dio_rw()
        splits a bio inside a block

The fix used here is to test for always COW once the IOLOCK is held.

Before commit 8caec6c9fef7 ("xfs: support sub-block aligned vectors in
always COW mode"), xfs_file_dio_write() made the same test with no lock
held, to send such vectors down the unaligned path. So v6.15 to v6.18
have the same race, and need a different version of this fix.

Fixes: 2e2383405824 ("xfs: implement direct writes to zoned RT devices")
Signed-off-by: Chris Mason <mason@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---

Notes:
    v7->v8:
    - Added the Reviewed-by of Christoph Hellwig.
    
    v6->v7:
    - New patch.

 fs/xfs/xfs_file.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 515ddafc6673..5fa8b07a779d 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -708,14 +708,6 @@ xfs_file_dio_write_aligned(
 	unsigned int		dio_flags = 0;
 	ssize_t			ret;
 
-	/*
-	 * For always COW inodes, each bio must be aligned to the file system
-	 * block size and not just the device sector size because we need to
-	 * allocate a block-aligned amount of space for each write.
-	 */
-	if (xfs_is_always_cow_inode(ip))
-		dio_flags |= IOMAP_DIO_FSBLOCK_ALIGNED;
-
 	ret = xfs_ilock_iocb_for_write(iocb, &iolock);
 	if (ret)
 		return ret;
@@ -743,6 +735,15 @@ xfs_file_dio_write_aligned(
 		xfs_ilock_demote(ip, XFS_IOLOCK_EXCL);
 		iolock = XFS_IOLOCK_SHARED;
 	}
+
+	/*
+	 * For always COW inodes, each bio must be aligned to the file system
+	 * block size and not just the device sector size because we need to
+	 * allocate a block-aligned amount of space for each write.  The
+	 * realtime flag can change until we hold the IOLOCK, so test only now.
+	 */
+	if (xfs_is_always_cow_inode(ip))
+		dio_flags |= IOMAP_DIO_FSBLOCK_ALIGNED;
 	if (mapping_stable_writes(iocb->ki_filp->f_mapping))
 		dio_flags |= IOMAP_DIO_BOUNCE;
 	trace_xfs_file_direct_write(iocb, from);
-- 
2.55.0

[PATCH v8 10/15] xfs: refuse fallocate racing with realtime flag changes

[Chris Mason]

On a zoned XFS, fallocate() can dereference a NULL pointer if the
realtime flag of the file is set during the call.

xfs_file_fallocate() reserves zoned space only for a zoned inode, and
tests for that before it takes the iolock. FS_IOC_FSSETXATTR can change
the realtime flag of a file with no blocks in that gap.
xfs_falloc_force_zero() then sees a zoned inode and reads the
reservation, which is NULL:

    fallocate(FALLOC_FL_ZERO_RANGE)     FS_IOC_FSSETXATTR
    ===============================     =================
                                        takes the iolock
    sees that the inode is not zoned
    waits for the iolock
                                        sets the realtime flag
                                        drops the iolock
    xfs_falloc_force_zero()
      sees that the inode is zoned
      /* ac is NULL */

The fix used here is to test again under the iolock, and to fail the
call with -EINVAL if the answer has changed, as the write path does.

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>
---

Notes:
    v7->v8:
    - No change.
    
    v6->v7:
    - No change.
    - Was 9/10.

 fs/xfs/xfs_file.c | 41 ++++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 5fa8b07a779d..78c9668c806b 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1573,6 +1573,21 @@ xfs_falloc_allocate_range(
 		 FALLOC_FL_ZERO_RANGE |	FALLOC_FL_INSERT_RANGE |	\
 		 FALLOC_FL_UNSHARE_RANGE | FALLOC_FL_WRITE_ZEROES)
 
+/*
+ * For zoned file systems, zeroing the first and last block of a hole punch
+ * requires allocating a new block to rewrite the remaining data and new zeroes
+ * out of place, which needs a space reservation.
+ */
+static inline bool
+xfs_falloc_needs_zoned_resv(
+	struct xfs_inode	*ip,
+	int			mode)
+{
+	return xfs_is_zoned_inode(ip) &&
+		(mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE |
+			 FALLOC_FL_COLLAPSE_RANGE));
+}
+
 STATIC long
 __xfs_file_fallocate(
 	struct file		*file,
@@ -1591,6 +1606,19 @@ __xfs_file_fallocate(
 	if (error)
 		goto out_unlock;
 
+	/*
+	 * The caller looked at the realtime flag before we took the IOLOCK, to
+	 * decide if it has to reserve space.  Bail out if the flag has changed
+	 * since.
+	 */
+	if (xfs_falloc_needs_zoned_resv(ip, mode) != !!ac) {
+		xfs_warn_ratelimited(ip->i_mount,
+			"RT flag modifying during fallocate for inode 0x%llx.",
+			inode->i_ino);
+		error = -EINVAL;
+		goto out_unlock;
+	}
+
 	/*
 	 * Must wait for all AIO to complete before we continue as AIO can
 	 * change the file size on completion without holding any locks we
@@ -1686,16 +1714,11 @@ xfs_file_fallocate(
 		return -EOPNOTSUPP;
 
 	/*
-	 * For zoned file systems, zeroing the first and last block of a hole
-	 * punch requires allocating a new block to rewrite the remaining data
-	 * and new zeroes out of place.  Get a reservations for those before
-	 * taking the iolock.  Dip into the reserved pool because we are
-	 * expected to be able to punch a hole even on a completely full
-	 * file system.
+	 * Get the reservation before taking the iolock.  Dip into the reserved
+	 * pool because we are expected to be able to punch a hole even on a
+	 * completely full file system.
 	 */
-	if (xfs_is_zoned_inode(XFS_I(inode)) &&
-	    (mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE |
-		     FALLOC_FL_COLLAPSE_RANGE)))
+	if (xfs_falloc_needs_zoned_resv(XFS_I(inode), mode))
 		return xfs_file_zoned_fallocate(file, mode, offset, len);
 	return __xfs_file_fallocate(file, mode, offset, len, NULL);
 }
-- 
2.55.0

[PATCH v8 11/15] xfs: restart faults racing with realtime flag changes

[Chris Mason]

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 make the fault start over if the flag has
changed. Any other caller that gets there fails, as a write does.

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

Notes:
    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_iomap.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index c3465204b8d5..d36650450cbb 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -264,6 +264,25 @@ 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.  The realtime flag cannot change under the ILOCK,
+ * so look at it again there, before reserving or allocating blocks.  Only a
+ * fault can start over, so refuse any other caller, as the write path does.
+ */
+static int
+xfs_iomap_rtflag_changed(
+	struct xfs_inode	*ip,
+	unsigned int		flags)
+{
+	xfs_warn_ratelimited(ip->i_mount,
+		"RT flag modifying during block mapping for inode 0x%llx.",
+		I_INO(ip));
+	if (flags & IOMAP_FAULT)
+		return -EAGAIN;
+	return -EINVAL;
+}
+
 int
 xfs_iomap_write_direct(
 	struct xfs_inode	*ip,
@@ -334,6 +353,11 @@ xfs_iomap_write_direct(
 	if (error)
 		return error;
 
+	if (xfs_is_zoned_inode(ip)) {
+		error = xfs_iomap_rtflag_changed(ip, flags);
+		goto out_trans_cancel;
+	}
+
 	error = xfs_iext_count_extend(tp, ip, XFS_DATA_FORK, nr_exts);
 	if (error)
 		goto out_trans_cancel;
@@ -919,6 +943,11 @@ xfs_direct_write_iomap_begin(
 	if (error)
 		return error;
 
+	if (xfs_is_zoned_inode(ip)) {
+		error = xfs_iomap_rtflag_changed(ip, flags);
+		goto out_unlock;
+	}
+
 	/*
 	 * The reflink iflag could have changed since the earlier unlocked
 	 * check, check if it again and relock if needed.
@@ -1666,9 +1695,7 @@ xfs_zoned_buffered_write_iomap_begin(
 	struct xfs_iext_cursor	icur;
 	int			error = 0;
 
-	ASSERT(!xfs_get_extsz_hint(ip));
 	ASSERT(!(flags & IOMAP_UNSHARE));
-	ASSERT(ac);
 
 	if (xfs_is_shutdown(mp))
 		return -EIO;
@@ -1682,6 +1709,12 @@ xfs_zoned_buffered_write_iomap_begin(
 	if (error)
 		return error;
 
+	if (!xfs_is_zoned_inode(ip) || !ac) {
+		error = xfs_iomap_rtflag_changed(ip, flags);
+		goto out_unlock;
+	}
+	ASSERT(!xfs_get_extsz_hint(ip));
+
 	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 +1898,11 @@ xfs_buffered_write_iomap_begin(
 	if (error)
 		return error;
 
+	if (xfs_is_zoned_inode(ip)) {
+		error = xfs_iomap_rtflag_changed(ip, flags);
+		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

[PATCH v8 12/15] xfs: test for an extsize hint again under the ILOCK

[Chris Mason]

A store through a shared mapping can corrupt the data fork and the free
block counter on disk, if the realtime flag of the file is set during
the write fault and realtime extents are larger than one block.

Such a realtime inode has an extent size hint, and cannot use delayed
allocation. xfs_buffered_write_iomap_begin() tests for a hint before it
takes the ILOCK. FS_IOC_FSSETXATTR can change the flag of a file with no
blocks until then, since a fault never takes the IOLOCK:

    write fault                           FS_IOC_FSSETXATTR
    ===========                           =================
    xfs_buffered_write_iomap_begin()
      xfs_get_extsz_hint() is 0
                                          sets the realtime flag
      xfs_ilock_for_iomap()
      xfs_bmapi_reserve_delalloc()
        /* 1 block, 0 realtime extents */

The fix used here is to test for a hint again once the ILOCK is held,
and to allocate the blocks right away if the inode has got one.

Signed-off-by: Chris Mason <mason@kernel.org>
---

Notes:
    v7->v8:
    - New patch.

 fs/xfs/xfs_iomap.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index d36650450cbb..b414e2cff346 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1903,6 +1903,16 @@ xfs_buffered_write_iomap_begin(
 		goto out_unlock;
 	}
 
+	/*
+	 * A write fault doesn't hold the IOLOCK, so the inode may have got an
+	 * extent size hint since the test above.
+	 */
+	if (xfs_get_extsz_hint(ip)) {
+		xfs_iunlock(ip, lockmode);
+		return xfs_direct_write_iomap_begin(inode, offset, count,
+				flags, iomap, srcmap);
+	}
+
 	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

[PATCH v8 13/15] xfs: resample the extsize hint after taking the ILOCK

[Chris Mason]

A write fault on a file with an extent size hint can overrun its block
reservation and shut down the filesystem, if it races with a change of
the hint or of the realtime flag.

xfs_iomap_write_direct() sizes the reservation from the hint and the
flag before xfs_trans_alloc_inode() takes the ILOCK, and the allocator
reads both again under it. But a write fault only holds the MMAPLOCK,
and FS_IOC_FSSETXATTR changes both on a file with no blocks without it:

    write fault                         FS_IOC_FSSETXATTR
    ===========                         =================
    xfs_iomap_write_direct()
      hint is 1 block
      dblocks = 1 + bmbt blocks
                                        xfs_ilock(ILOCK_EXCL)
                                        hint = 4096 blocks
                                        xfs_iunlock(ILOCK_EXCL)
      xfs_trans_alloc_inode()
      xfs_bmapi_write()
        allocates 4096 blocks
        xfs_trans_mod_sb()
          t_blk_res_used > t_blk_res

The fix used here is to compare both with the values that sized the
reservation once the ILOCK is held, and to start over if either has
changed.

Signed-off-by: Chris Mason <mason@kernel.org>
---

Notes:
    v7->v8:
    - New patch.

 fs/xfs/xfs_iomap.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index b414e2cff346..7fb61f56a414 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -295,9 +295,11 @@ xfs_iomap_write_direct(
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_trans	*tp;
 	xfs_filblks_t		resaligned;
+	xfs_extlen_t		extsz;
 	int			nimaps;
 	unsigned int		dblocks, rblocks;
 	bool			force = false;
+	bool			rt;
 	int			error;
 	int			bmapi_flags = XFS_BMAPI_PREALLOC;
 	int			nr_exts = XFS_IEXT_ADD_NOSPLIT_CNT;
@@ -312,9 +314,11 @@ xfs_iomap_write_direct(
 	 */
 	count_fsb = min_t(xfs_filblks_t, count_fsb, XFS_MAX_BMBT_EXTLEN);
 
-	resaligned = xfs_aligned_fsb_count(offset_fsb, count_fsb,
-					   xfs_get_extsz_hint(ip));
-	if (unlikely(XFS_IS_REALTIME_INODE(ip))) {
+retry:
+	extsz = xfs_get_extsz_hint(ip);
+	rt = XFS_IS_REALTIME_INODE(ip);
+	resaligned = xfs_aligned_fsb_count(offset_fsb, count_fsb, extsz);
+	if (unlikely(rt)) {
 		dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
 		rblocks = resaligned;
 	} else {
@@ -358,6 +362,17 @@ xfs_iomap_write_direct(
 		goto out_trans_cancel;
 	}
 
+	/*
+	 * A write fault doesn't hold the IOLOCK, so what sized the reservation
+	 * can change until we hold the ILOCK.
+	 */
+	if (extsz != xfs_get_extsz_hint(ip) ||
+	    rt != XFS_IS_REALTIME_INODE(ip)) {
+		xfs_trans_cancel(tp);
+		xfs_iunlock(ip, XFS_ILOCK_EXCL);
+		goto retry;
+	}
+
 	error = xfs_iext_count_extend(tp, ip, XFS_DATA_FORK, nr_exts);
 	if (error)
 		goto out_trans_cancel;
-- 
2.55.0

[PATCH v8 14/15] xfs: resample the COW extsize hint after cycling ILOCK

[Chris Mason]

A write fault on a shared extent of a file with an extent size hint, or
of a DAX file, can overrun its block reservation and shut down the
filesystem, if it races with a change of the COW extent size hint.

xfs_reflink_fill_cow_hole() sizes the reservation from the hint and
drops the ILOCK for xfs_trans_alloc_inode(), and the allocator reads the
hint again once the lock is back. But a write fault only holds the
MMAPLOCK, and FS_IOC_FSSETXATTR changes the hint without it:

    write fault                         FS_IOC_FSSETXATTR
    ===========                         =================
    xfs_reflink_fill_cow_hole()
      hint is 1 block
      dblocks = 1 + bmbt blocks
      xfs_iunlock()
                                        xfs_ilock(ILOCK_EXCL)
                                        hint = 4096 blocks
                                        xfs_iunlock(ILOCK_EXCL)
      xfs_trans_alloc_inode()
      xfs_bmapi_write()
        allocates 4096 blocks
        xfs_trans_mod_sb()
          t_blk_res_used > t_blk_res

The fix used here is to compare the hint with the one that sized the
reservation once the ILOCK is held again, and to start over if it has
changed.

Signed-off-by: Chris Mason <mason@kernel.org>
---

Notes:
    v7->v8:
    - New patch.

 fs/xfs/xfs_reflink.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index ff89516154bd..665ee75cd349 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -445,14 +445,17 @@ xfs_reflink_fill_cow_hole(
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_trans	*tp;
 	xfs_filblks_t		resaligned;
+	xfs_extlen_t		cowextsz;
 	unsigned int		seq_before = READ_ONCE(ip->i_df.if_seq);
 	unsigned int		dblocks = 0, rblocks = 0;
 	int			nimaps;
 	int			error;
 	bool			found;
 
+retry:
+	cowextsz = xfs_get_cowextsz_hint(ip);
 	resaligned = xfs_aligned_fsb_count(imap->br_startoff,
-		imap->br_blockcount, xfs_get_cowextsz_hint(ip));
+		imap->br_blockcount, cowextsz);
 	if (XFS_IS_REALTIME_INODE(ip)) {
 		dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
 		rblocks = resaligned;
@@ -471,6 +474,15 @@ xfs_reflink_fill_cow_hole(
 
 	*lockmode = XFS_ILOCK_EXCL;
 
+	/*
+	 * A write fault doesn't hold the IOLOCK, so the hint that sized the
+	 * reservation may have changed while we dropped the ILOCK.
+	 */
+	if (cowextsz != xfs_get_cowextsz_hint(ip)) {
+		xfs_trans_cancel(tp);
+		goto retry;
+	}
+
 	/*
 	 * The data fork mapping may have changed while we dropped the ILOCK
 	 * (a racing O_DIRECT writer under IOLOCK_SHARED can complete a full
-- 
2.55.0

[PATCH v8 15/15] xfs: don't enable DAX on zoned inodes

[Chris Mason]

A store through a shared mapping can dereference a NULL pointer if the
file is zoned and has S_DAX.

DAX writes in place, and zoned inodes are only written out of place, so
the two can't work together. XFS doesn't check for this: with dax=always
or FS_XFLAG_DAX, an inode gets S_DAX if its device does DAX, and an
internal zoned section on pmem does. The write fault then takes the
zoned path:

    xfs_filemap_fault()
      xfs_write_fault()
        xfs_write_fault_zoned()
          page_folio(vmf->page)     /* NULL in a DAX fault */

ftruncate() crashes in a similar way, in xfs_reflink_end_cow(), on
kernels without "xfs: restart faults racing with realtime flag changes".

The fix used here is to never set S_DAX on a zoned inode, and to refuse
to change the realtime flag of an S_DAX inode on a zoned file system.

Zoned files now use the page cache with dax=always or FS_XFLAG_DAX. A
file whose realtime flag is cleared stays without DAX until its inode
is evicted.

Fixes: be458049ffe3 ("xfs: enable the zoned RT device feature")
Signed-off-by: Chris Mason <mason@kernel.org>
---

Notes:
    v7->v8:
    - New patch.

 fs/xfs/xfs_ioctl.c | 8 +++++---
 fs/xfs/xfs_iops.c  | 4 ++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index f81b6e52ac40..8f042610db03 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -564,11 +564,13 @@ xfs_ioctl_setattr_xflags(
 
 		/*
 		 * If S_DAX is enabled on this file, we can only switch the
-		 * device if both support fsdax.  We can't update S_DAX because
-		 * there might be other threads walking down the access paths.
+		 * device if both support fsdax, and zoned inodes never do.  We
+		 * can't update S_DAX because there might be other threads
+		 * walking down the access paths.
 		 */
 		if (IS_DAX(VFS_I(ip)) &&
-		    (mp->m_ddev_targp->bt_daxdev == NULL ||
+		    (xfs_has_zoned(mp) ||
+		     mp->m_ddev_targp->bt_daxdev == NULL ||
 		     (mp->m_rtdev_targp &&
 		      mp->m_rtdev_targp->bt_daxdev == NULL)))
 			return -EINVAL;
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index d307ec97fac8..03ca1873b6d7 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1373,6 +1373,10 @@ xfs_inode_supports_dax(
 	if (mp->m_sb.sb_blocksize != PAGE_SIZE)
 		return false;
 
+	/* Zoned inodes are always written out of place, DAX writes in place. */
+	if (xfs_is_zoned_inode(ip))
+		return false;
+
 	/* Device has to support DAX too. */
 	return xfs_inode_buftarg(ip)->bt_daxdev != NULL;
 }
-- 
2.55.0