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

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

[Youling Tang]

From: Youling Tang <tangyouling@kylinos.cn>

__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: stable@vger.kernel.org
Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
---
 fs/exfat/iomap.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/exfat/iomap.c b/fs/exfat/iomap.c
index 4e6f3ed9dcc8..11823591b3a8 100644
--- a/fs/exfat/iomap.c
+++ b/fs/exfat/iomap.c
@@ -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,
-- 
2.43.0

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

[Youling Tang]

From: Youling Tang <tangyouling@kylinos.cn>

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: stable@vger.kernel.org
Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
---
 fs/exfat/iomap.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/fs/exfat/iomap.c b/fs/exfat/iomap.c
index 11823591b3a8..d6b420f615d4 100644
--- a/fs/exfat/iomap.c
+++ b/fs/exfat/iomap.c
@@ -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);
 
-- 
2.43.0

[PATCH 0/2] exfat: fix iomap mapping vs truncate race

[Youling Tang]

From: Youling Tang <tangyouling@kylinos.cn>

This series fixes two related problems in the exfat iomap path.

Patch 1 fixes a race between iomap mapping and truncate: i_size was read
twice, and a shrink in between could make the mapping length negative,
which underflowed the cluster count.

Patch 2 fixes the handling of the EXFAT_EOF_CLUSTER sentinel returned by
exfat_map_cluster().  The iomap code used to convert it into a physical
address; it now maps the valid prefix of the range and reports only the
remaining part as a hole, matching the old buffer_head code.

Patch 2 relies on the non-negative length guaranteed by patch 1 for the
hole mapping.

Youling Tang (2):
  exfat: fix iomap mapping when writeback races with truncate
  exfat: don't map EXFAT_EOF_CLUSTER as a physical cluster

 fs/exfat/iomap.c | 38 ++++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

-- 
2.43.0

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

[Youling Tang]

From: Youling Tang <tangyouling@kylinos.cn>

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: stable@vger.kernel.org
Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
---
 fs/exfat/iomap.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/fs/exfat/iomap.c b/fs/exfat/iomap.c
index 11823591b3a8..d6b420f615d4 100644
--- a/fs/exfat/iomap.c
+++ b/fs/exfat/iomap.c
@@ -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);
 
-- 
2.43.0