[PATCH] fat: calculate data area start without overflow

[PATCH] fat: calculate data area start without overflow

[hengyul]

From: Hengyu Liang <hengyul@cs.unc.edu>

On 32-bit architectures, sbi->fat_length, sbi->dir_start and
sbi->data_start are unsigned long. The number of FATs is an 8-bit BPB
field, while the FAT32 length is a 32-bit BPB field. Therefore, the
calculation

    sbi->fat_start + sbi->fats * sbi->fat_length

can wrap before data_start is checked against total_sectors.

For example, with fat_start=32, fats=2 and fat_length=0x80000001,
the unwrapped data area start is 0x100000022 (4294967330), but the
calculation wraps to 34 on i386. With total_sectors=36, the
validation then incorrectly passes.

The following script creates an image that demonstrates the problem:

    python3 - <<'PY'
    import struct

    S = 512
    b = bytearray(36 * S)

    def p(off, fmt, value):
        struct.pack_into(fmt, b, off, value)

    # FAT32 BPB
    b[0:3] = b'\xeb\x58\x90'
    b[3:11] = b'MSWIN4.1'
    p(11, '<H', 512)
    b[13] = 1
    p(14, '<H', 32)             # reserved sectors
    b[16] = 2                   # number of FATs
    p(17, '<H', 0)
    p(19, '<H', 0)
    b[21] = 0xf8
    p(22, '<H', 0)
    p(24, '<H', 1)
    p(26, '<H', 1)
    p(32, '<I', 36)             # total sectors
    p(36, '<I', 0x80000001)     # FAT32 length
    p(44, '<I', 2)              # root cluster
    p(48, '<H', 1)              # FSINFO sector
    b[64] = 0x80
    b[66] = 0x29
    p(67, '<I', 0x12345678)
    b[71:82] = b'OVERFLOW   '
    b[82:90] = b'FAT32   '
    b[510:512] = b'\x55\xaa'

    # FSINFO
    p(S, '<I', 0x41615252)
    p(S + 484, '<I', 0x61417272)
    p(S + 488, '<I', 0xffffffff)
    p(S + 492, '<I', 0xffffffff)

    # Wrapped FAT starts at sector 32
    fat = 32 * S
    p(fat + 8, '<I', 0x0fffffff)   # FAT[2]
    p(fat + 12, '<I', 0x0fffffff)  # FAT[3]

    # Wrapped data_start == 34
    root = 34 * S
    b[root:root + 11] = b'ESCAPE  TXT'
    b[root + 11] = 0x20
    p(root + 26, '<H', 3)
    p(root + 28, '<I', 4)
    b[35 * S:35 * S + 4] = b'OOB!'

    open('fat-overflow.img', 'wb').write(b)
    PY

Attach fat-overflow.img as /dev/sdb and run:

    mount -t vfat -o ro /dev/sdb /mnt
    cat /mnt/ESCAPE.TXT

On an unpatched i386 kernel, the mount succeeds and reading the file
returns:

    OOB!

On an x86-64 kernel and on a patched i386 kernel, mounting is rejected
with:

    mount: mounting /dev/sdb on /mnt failed: Invalid argument

Calculate dir_start and data_start in u64 using mul_u32_u32(), and
perform the total_sectors check before storing them in the existing
unsigned long fields. Since a successful check guarantees that
data_start, and therefore dir_start, does not exceed the u32
total_sectors value, assigning them to unsigned long is safe on both
32-bit and 64-bit architectures.

Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
---
 fs/fat/inode.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index f775a004cae1..b09185d204ce 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -20,6 +20,7 @@
 #include <linux/blkdev.h>
 #include <linux/backing-dev.h>
 #include <linux/unaligned.h>
+#include <linux/math64.h>
 #include <linux/random.h>
 #include <linux/iversion.h>
 #include <linux/fs_struct.h>
@@ -1577,6 +1578,7 @@ int fat_fill_super(struct super_block *sb, struct fs_context *fc,
 	struct msdos_sb_info *sbi;
 	u16 logical_sector_size;
 	u32 total_sectors, total_clusters, fat_clusters, rootdir_sectors;
+	u64 dir_start, data_start;
 	long error;
 	char buf[50];
 	struct timespec64 ts;
@@ -1752,7 +1754,6 @@ int fat_fill_super(struct super_block *sb, struct fs_context *fc,
 	sbi->dir_per_block = sb->s_blocksize / sizeof(struct msdos_dir_entry);
 	sbi->dir_per_block_bits = ffs(sbi->dir_per_block) - 1;
 
-	sbi->dir_start = sbi->fat_start + sbi->fats * sbi->fat_length;
 	sbi->dir_entries = bpb.fat_dir_entries;
 	if (sbi->dir_entries & (sbi->dir_per_block - 1)) {
 		if (!silent)
@@ -1763,19 +1764,23 @@ int fat_fill_super(struct super_block *sb, struct fs_context *fc,
 
 	rootdir_sectors = sbi->dir_entries
 		* sizeof(struct msdos_dir_entry) / sb->s_blocksize;
-	sbi->data_start = sbi->dir_start + rootdir_sectors;
+	dir_start = sbi->fat_start +
+		mul_u32_u32(sbi->fats, sbi->fat_length);
+	data_start = dir_start + rootdir_sectors;
 	total_sectors = bpb.fat_sectors;
 	if (total_sectors == 0)
 		total_sectors = bpb.fat_total_sect;
 
-	if (total_sectors < sbi->data_start) {
+	if (total_sectors < data_start) {
 		if (!silent)
 			fat_msg(sb, KERN_ERR,
-				"data area starts beyond volume (%lu > %u)",
-				sbi->data_start, total_sectors);
+				"data area starts beyond volume (%llu > %u)",
+				(llu)data_start, total_sectors);
 		goto out_invalid;
 	}
 
+	sbi->dir_start = dir_start;
+	sbi->data_start = data_start;
 	total_clusters = (total_sectors - sbi->data_start) / sbi->sec_per_clus;
 
 	if (!is_fat32(sbi))

Re: [PATCH] fat: calculate data area start without overflow

[OGAWA Hirofumi]

hengyul@cs.unc.edu writes:

> From: Hengyu Liang <hengyul@cs.unc.edu>
>
> On 32-bit architectures, sbi->fat_length, sbi->dir_start and
> sbi->data_start are unsigned long. The number of FATs is an 8-bit BPB
> field, while the FAT32 length is a 32-bit BPB field. Therefore, the
> calculation
>
>     sbi->fat_start + sbi->fats * sbi->fat_length
>
> can wrap before data_start is checked against total_sectors.

[...]
> -	sbi->dir_start = sbi->fat_start + sbi->fats * sbi->fat_length;
>  	sbi->dir_entries = bpb.fat_dir_entries;
>  	if (sbi->dir_entries & (sbi->dir_per_block - 1)) {
>  		if (!silent)
> @@ -1763,19 +1764,23 @@ int fat_fill_super(struct super_block *sb, struct fs_context *fc,
>  
>  	rootdir_sectors = sbi->dir_entries
>  		* sizeof(struct msdos_dir_entry) / sb->s_blocksize;
> -	sbi->data_start = sbi->dir_start + rootdir_sectors;
> +	dir_start = sbi->fat_start +
> +		mul_u32_u32(sbi->fats, sbi->fat_length);
> +	data_start = dir_start + rootdir_sectors;

Maybe, since u32 overflow is always bigger than total_sectors, so we
should use check_*_overflow() instead, and detect early?

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH] fat: calculate data area start without overflow

[David Laight]

On Wed, 02 Sep 2026 21:03:26 +0900
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> wrote:

> hengyul@cs.unc.edu writes:
> 
> > From: Hengyu Liang <hengyul@cs.unc.edu>
> >
> > On 32-bit architectures, sbi->fat_length, sbi->dir_start and
> > sbi->data_start are unsigned long. The number of FATs is an 8-bit BPB
> > field, while the FAT32 length is a 32-bit BPB field. Therefore, the
> > calculation
> >
> >     sbi->fat_start + sbi->fats * sbi->fat_length
> >
> > can wrap before data_start is checked against total_sectors.  
> 
> [...]
> > -	sbi->dir_start = sbi->fat_start + sbi->fats * sbi->fat_length;
> >  	sbi->dir_entries = bpb.fat_dir_entries;
> >  	if (sbi->dir_entries & (sbi->dir_per_block - 1)) {
> >  		if (!silent)
> > @@ -1763,19 +1764,23 @@ int fat_fill_super(struct super_block *sb, struct fs_context *fc,
> >  
> >  	rootdir_sectors = sbi->dir_entries
> >  		* sizeof(struct msdos_dir_entry) / sb->s_blocksize;
> > -	sbi->data_start = sbi->dir_start + rootdir_sectors;
> > +	dir_start = sbi->fat_start +
> > +		mul_u32_u32(sbi->fats, sbi->fat_length);
> > +	data_start = dir_start + rootdir_sectors;  
> 
> Maybe, since u32 overflow is always bigger than total_sectors, so we
> should use check_*_overflow() instead, and detect early?

Why do the work twice?

David

> 
> Thanks.