[PATCH v0 1/2] exfat: add volume limit bounds checks

[PATCH v0 1/2] exfat: add volume limit bounds checks

[David Timber]

If the user inadvertenly truncates an exFAT volume(mistakenly shrinks
the partition or simply dd'ing from a larger removable device to a
smaller one. eg: device marketed as having 8GB capatity < 8GiB), the
kernel exFAT obliviously mounts the volume and operates on it. No error
is reported to userspace unless the filesystem is accessed with O_SYNC
or O_DIRECT.

Off by one sector test:

	# truncate -s 1073741824 img
	# mkfs.exfat img
	# truncate -s 1073741312 img
	# mount -t exfat img ...

The existing filesystem implementations, prime examples being XFS and
ext*, refuse to mount the volume with such condition. Introduce the
checks similar checks in-place to exFAT.

Also, to prevent UB, add checks against exFAT volumes with maliciously
a crafted main boot sectors with the ClusterCount field equal to or
larger than (2^32 - 11) as per format spec.

Link: https://github.com/exfatprogs/exfatprogs/issues/353
Signed-off-by: David Timber <dxdt@dev.snart.me>
---
 fs/exfat/super.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index 95d87e2d7717..075130fd992a 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -431,6 +431,7 @@ static int exfat_read_boot_sector(struct super_block *sb)
 {
 	struct boot_sector *p_boot;
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
+	unsigned int nb_clusters;
 
 	/* set block size to read super block */
 	if (!sb_min_blocksize(sb, 512)) {
@@ -501,8 +502,17 @@ static int exfat_read_boot_sector(struct super_block *sb)
 	sbi->data_start_sector = le32_to_cpu(p_boot->clu_offset);
 	sbi->num_sectors = le64_to_cpu(p_boot->vol_length);
 	/* because the cluster index starts with 2 */
-	sbi->num_clusters = le32_to_cpu(p_boot->clu_count) +
-		EXFAT_RESERVED_CLUSTERS;
+	nb_clusters = le32_to_cpu(p_boot->clu_count);
+	/*
+	 * The inclusive comparison in the following check seems a bit off(quite
+	 * literally), but the exFAT format section 3.1.9 says
+	 * "lesser of the following". Aye, aye, captain.
+	 */
+	if (nb_clusters >= EXFAT_MAX_NUM_CLUSTER) {
+		exfat_err(sb, "bogus number of clusters : %u", nb_clusters);
+		return -EINVAL;
+	}
+	sbi->num_clusters = nb_clusters + EXFAT_RESERVED_CLUSTERS;
 
 	sbi->root_dir = le32_to_cpu(p_boot->root_cluster);
 	sbi->dentries_per_clu = 1 <<
@@ -591,6 +601,33 @@ static int exfat_verify_boot_region(struct super_block *sb)
 	return 0;
 }
 
+static int exfat_check_volume_sizes(struct super_block *sb)
+{
+	struct exfat_sb_info *sbi = EXFAT_SB(sb);
+	/* sector to start of last cluster */
+	const sector_t lc_start = exfat_cluster_to_sector(sbi, sbi->num_clusters - 1);
+	const sector_t lc_end = lc_start + sbi->sect_per_clus;
+	const sector_t last_sector = lc_end - 1;
+	struct buffer_head *bh;
+
+	/* Volume length(metadata in boot sector) bounds */
+	if (sbi->num_sectors < (unsigned long long)lc_end) {
+		exfat_err(sb, "number of cluster out of volume length bounds : %llu < %lld",
+			  sbi->num_sectors, lc_end);
+		return -EINVAL;
+	}
+
+	/* Actually try reading the last sector to see if it's within the blockdev bounds */
+	bh = sb_bread(sb, last_sector);
+	if (bh == NULL) {
+		exfat_err(sb, "last sector read failed : %lld", last_sector);
+		return -EIO;
+	}
+	brelse(bh);
+
+	return 0;
+}
+
 /* mount the file system volume */
 static int __exfat_fill_super(struct super_block *sb,
 		struct exfat_chain *root_clu)
@@ -610,6 +647,12 @@ static int __exfat_fill_super(struct super_block *sb,
 		goto free_bh;
 	}
 
+	ret = exfat_check_volume_sizes(sb);
+	if (ret) {
+		exfat_warn(sb, "volume bounds check failed. Please run fsck");
+		goto free_bh;
+	}
+
 	/*
 	 * Call exfat_count_num_cluster() before searching for up-case and
 	 * bitmap directory entries to avoid infinite loop if they are missing
-- 
2.53.0.1.ga224b40d3f.dirty

[PATCH v0 2/2] exfat: print warning upon block size calibration

[David Timber]

If the block size specified in the exFAT volume boot sector is different
from the actual logical block size of the device, many implementations
including FUSE-exfat, macos and previous versions of Windows are not
able to mount the volume.

A possible scenario in which this can happen is when the user dd's the
volume in a 4K-blocksize device("Advanced Format") to a 512-blocksize
device. This is a design issue inherent to the exFAT format itself
which layouts the structures of exFAT volumes aligned to the sector
size rather than large byte sizes as seen with other modern file
systems.

Print a kind warning about this potential compatibility issue.

Link: https://github.com/exfatprogs/exfatprogs/issues/349
Signed-off-by: David Timber <dxdt@dev.snart.me>
---
 fs/exfat/super.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index 075130fd992a..89a5f74a37fb 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -409,6 +409,8 @@ static int exfat_calibrate_blocksize(struct super_block *sb, int logical_sect)
 	}
 
 	if (logical_sect > sb->s_blocksize) {
+		const unsigned long saved_bs = sb->s_blocksize;
+
 		brelse(sbi->boot_bh);
 		sbi->boot_bh = NULL;
 
@@ -423,6 +425,11 @@ static int exfat_calibrate_blocksize(struct super_block *sb, int logical_sect)
 				  sb->s_blocksize);
 			return -EIO;
 		}
+
+		exfat_warn(sb, "blocksize calibrated from device logical block size(%lu) to "
+			   "volume format logical sector size(%d)!\n"
+			   "Other implementations may not be able to handle this volume.",
+			   saved_bs, logical_sect);
 	}
 	return 0;
 }
-- 
2.53.0.1.ga224b40d3f.dirty

Re: [PATCH v0 1/2] exfat: add volume limit bounds checks

[Yuezhang.Mo]

> +	/* Actually try reading the last sector to see if it's within the blockdev bounds */
> +	bh = sb_bread(sb, last_sector);
> +	if (bh == NULL) {
> +		exfat_err(sb, "last sector read failed : %lld", last_sector);
> +		return -EIO;
> +	}
> +	brelse(bh);

Instead of checking by reading the last sector, we can use bdev_nr_sectors() to 
get the actual size of the volume.

Re: [PATCH v0 1/2] exfat: add volume limit bounds checks

[David Timber]

On 4/30/26 16:52, Yuezhang.Mo@sony.com wrote:
>> +	/* Actually try reading the last sector to see if it's within the blockdev bounds */
>> +	bh = sb_bread(sb, last_sector);
>> +	if (bh == NULL) {
>> +		exfat_err(sb, "last sector read failed : %lld", last_sector);
>> +		return -EIO;
>> +	}
>> +	brelse(bh);
> Instead of checking by reading the last sector, we can use bdev_nr_sectors() to 
> get the actual size of the volume.
Agreed. No need for exFAT to actually inspect the data at the last
sector, unlike XFS. Monkey see, monkey doo.