Skip to contentSkip to conversation

exfat: fix iomap mapping when writeback races with truncate

PATCH New Youling Tang 1 participant 2 messages Nov 13 1 message not in the archive

Subsystems (2)
Stable branch exFAT file system
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Youling Tang

author
#1 lore raw

PATCH 1/2 exfat: fix iomap mapping when writeback races with truncate

From: Youling Tang <[email protected]>

__exfat_iomap_begin() reads i_size twice: once to detect a range that starts beyond EOF, and once to clamp the mapping to EOF. A concurrent truncate can shrink i_size between the two reads, so the clamp can produce a negative length. The unsigned cluster index subtraction then underflows, making exfat_map_cluster() believe it has to allocate a huge number of clusters and report a hole.

Fix this by reading i_size once for both the EOF check and the block-aligned clamp.

Fixes 82a81a7352bc ("exfat: add iomap buffered I/O support")
Cc [email protected]
Signed-off-by Youling Tang <[email protected]>
---
 fs/exfat/iomap.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
fs/exfat/iomap.c +5 -4
index 4e6f3ed9dcc8..11823591b3a8 100644
@@ -51,8 +51,10 @@ static int __exfat_iomap_begin(struct inode *inode, loff_t offset, loff_t length
 	bool balloc = false;
 
 	if (!may_alloc) {
+		loff_t isize = i_size_read(inode);
+
 		/* Completely beyond EOF. Treat as hole */
-		if (i_size_read(inode) <= offset) {
+		if (offset >= isize) {
 			iomap->type = IOMAP_HOLE;
 			iomap->addr = IOMAP_NULL_ADDR;
 			iomap->offset = offset;

@@ -61,9 +63,8 @@ static int __exfat_iomap_begin(struct inode *inode, loff_t offset, loff_t length
 		}
 
 		/* Clamp length if the requested range goes beyond i_size */
-		if (offset + length > i_size_read(inode))
-			length = round_up(i_size_read(inode),
-					  i_blocksize(inode)) - offset;
+		if (offset + length > isize)
+			length = round_up(isize, i_blocksize(inode)) - offset;
 	}
 
 	num_clusters = exfat_bytes_to_cluster_round_up(sbi,
Signature · 1 line
2.43.0
author
#2 lore raw

PATCH 2/2 exfat: don't map EXFAT_EOF_CLUSTER as a physical cluster

In reply to a message not in the archive

From: Youling Tang <[email protected]>

exfat_map_cluster() reports a hole by returning success with *clu == EXFAT_EOF_CLUSTER for !create. __exfat_iomap_begin() does not check that sentinel and converts it into a physical address far beyond the device. A writeback that races with the i_size/i_blocks window in exfat_cont_expand() can hit this: it submits a bio past the end of the device, and fsync() or msync() returns -EIO. If the resulting iomap length is zero, iomap_add_to_ioend() can loop on a zero-length bio and hang a concurrent truncate.

When a range starts before the end of the cluster chain but extends past it, exfat_map_cluster() returns the sentinel for the whole range. Map the valid prefix first and only report the part beyond the chain as a hole. Otherwise a writeback that straddles the end of the chain would skip valid dirty data.

Report the remaining range as a hole instead of turning the sentinel into an address, matching the old exfat_get_block() behavior.

Fixes 82a81a7352bc ("exfat: add iomap buffered I/O support")
Cc [email protected]
Signed-off-by Youling Tang <[email protected]>
---
 fs/exfat/iomap.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
fs/exfat/iomap.c +29 -0
index 11823591b3a8..d6b420f615d4 100644
@@ -74,6 +74,22 @@ static int __exfat_iomap_begin(struct inode *inode, loff_t offset, loff_t length
 	iomap->bdev = inode->i_sb->s_bdev;
 	iomap->offset = offset;
 
+	if (!may_alloc) {
+		unsigned int ondisk_clusters =
+			exfat_bytes_to_cluster(sbi, exfat_ondisk_size(inode));
+		unsigned int clu_offset =
+			exfat_bytes_to_cluster(sbi, offset);
+
+		/*
+		 * exfat_map_cluster() returns the EOF sentinel for the whole
+		 * range if it extends past the cluster chain.  Map the valid
+		 * prefix first so the hole only covers the part beyond it.
+		 */
+		if (clu_offset < ondisk_clusters &&
+		    num_clusters > ondisk_clusters - clu_offset)
+			num_clusters = ondisk_clusters - clu_offset;
+	}
+
 	err = exfat_map_cluster(inode, exfat_bytes_to_cluster(sbi, offset),
 			&cluster, &num_clusters, may_alloc, &balloc);
 	if (err == -ENOSPC && !(flags & IOMAP_DIRECT)) {

@@ -90,6 +106,19 @@ static int __exfat_iomap_begin(struct inode *inode, loff_t offset, loff_t length
 	if (err)
 		goto out;
 
+	/*
+	 * exfat_map_cluster() reports a hole by returning success with
+	 * EXFAT_EOF_CLUSTER.  Return it as a hole instead of turning the
+	 * sentinel into a physical address.
+	 */
+	if (!may_alloc && cluster == EXFAT_EOF_CLUSTER) {
+		iomap->type = IOMAP_HOLE;
+		iomap->addr = IOMAP_NULL_ADDR;
+		iomap->offset = offset;
+		iomap->length = length;
+		goto out;
+	}
+
 	cluster_offset = exfat_cluster_offset(sbi, offset);
 	cluster_length = exfat_cluster_to_bytes(sbi, num_clusters);
Signature · 1 line
2.43.0