linux-kernel - [PATCH] fat: fix data-race between fat12_ent_put() and fat_mirror

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250902081727.7146-1-anmuxixixi@gmail.com>
Date: Tue,  2 Sep 2025 16:17:27 +0800
From: YangWen <anmuxixixi@...il.com>
To: hirofumi@...l.parknet.co.jp
Cc: linux-kernel@...r.kernel.org,
	YangWen <anmuxixixi@...il.com>
Subject: [PATCH] fat: fix data-race between fat12_ent_put() and fat_mirror_bhs()

KCSAN reported a data-race between fat12_ent_put() and fat_mirror_bhs():
one CPU was updating a FAT12 entry while another CPU was copying the
whole sector to mirror FATs.Fix this by protecting memcpy() in
fat_mirror_bhs() with the same fat12_entry_lock that guards
fat12_ent_put(),so that the writer and the mirror operation
are mutually exclusive.

FAT-fs (loop5): error, clusters badly computed (404 != 401)
==================================================================
BUG: KCSAN: data-race in fat12_ent_put / fat_mirror_bhs

write to 0xffff888106c953e9 of 1 bytes by task 7452 on cpu 1:
 fat12_ent_put+0x74/0x170 fs/fat/fatent.c:168
 fat_ent_write+0x69/0xe0 fs/fat/fatent.c:417
 fat_chain_add+0x15d/0x440 fs/fat/misc.c:136
 fat_add_cluster fs/fat/inode.c:112 [inline]
 __fat_get_block fs/fat/inode.c:154 [inline]
 fat_get_block+0x46c/0x5e0 fs/fat/inode.c:189
 __block_write_begin_int+0x3fd/0xf90 fs/buffer.c:2145
 block_write_begin fs/buffer.c:2256 [inline]
 cont_write_begin+0x5fc/0x970 fs/buffer.c:2594
 fat_write_begin+0x4f/0xe0 fs/fat/inode.c:229
 cont_expand_zero fs/buffer.c:2522 [inline]
 cont_write_begin+0x1ad/0x970 fs/buffer.c:2584
 fat_write_begin+0x4f/0xe0 fs/fat/inode.c:229
 generic_perform_write+0x184/0x490 mm/filemap.c:4175
 __generic_file_write_iter+0x9e/0x120 mm/filemap.c:4292
 generic_file_write_iter+0x8d/0x2f0 mm/filemap.c:4318
 new_sync_write fs/read_write.c:593 [inline]
 vfs_write+0x52a/0x960 fs/read_write.c:686
 ksys_pwrite64 fs/read_write.c:793 [inline]
 __do_sys_pwrite64 fs/read_write.c:801 [inline]
 __se_sys_pwrite64 fs/read_write.c:798 [inline]
 __x64_sys_pwrite64+0xfd/0x150 fs/read_write.c:798
 x64_sys_call+0xc4d/0x2ff0 arch/x86/include/generated/asm/syscalls_64.h:19
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xd2/0x200 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

read to 0xffff888106c95200 of 512 bytes by task 7433 on cpu 0:
 fat_mirror_bhs+0x1df/0x320 fs/fat/fatent.c:395
 fat_ent_write+0xd0/0xe0 fs/fat/fatent.c:423
 fat_free fs/fat/file.c:363 [inline]
 fat_truncate_blocks+0x353/0x550 fs/fat/file.c:394
 fat_write_failed fs/fat/inode.c:218 [inline]
 fat_write_end+0xba/0x160 fs/fat/inode.c:246
 generic_perform_write+0x312/0x490 mm/filemap.c:4196
 __generic_file_write_iter+0x9e/0x120 mm/filemap.c:4292
 generic_file_write_iter+0x8d/0x2f0 mm/filemap.c:4318
 new_sync_write fs/read_write.c:593 [inline]
 vfs_write+0x52a/0x960 fs/read_write.c:686
 ksys_write+0xda/0x1a0 fs/read_write.c:738
 __do_sys_write fs/read_write.c:749 [inline]
 __se_sys_write fs/read_write.c:746 [inline]
 __x64_sys_write+0x40/0x50 fs/read_write.c:746
 x64_sys_call+0x27fe/0x2ff0 arch/x86/include/generated/asm/syscalls_64.h:2
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xd2/0x200 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Signed-off-by: YangWen <anmuxixixi@...il.com>
---
 fs/fat/fatent.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
index a7061c2ad8e4..e25775642489 100644
--- a/fs/fat/fatent.c
+++ b/fs/fat/fatent.c
@@ -379,6 +379,7 @@ static int fat_mirror_bhs(struct super_block *sb, struct buffer_head **bhs,
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 	struct buffer_head *c_bh;
 	int err, n, copy;
+	bool is_fat12 = is_fat12(sbi);
 
 	err = 0;
 	for (copy = 1; copy < sbi->fats; copy++) {
@@ -392,7 +393,17 @@ static int fat_mirror_bhs(struct super_block *sb, struct buffer_head **bhs,
 			}
 			/* Avoid race with userspace read via bdev */
 			lock_buffer(c_bh);
+			/*
+			 * For FAT12, protect memcpy() of the source sector
+			 * against concurrent 12-bit entry updates in
+			 * fat12_ent_put(), otherwise we may copy a torn
+			 * pair of bytes into the mirror FAT.
+			 */
+			if (is_fat12)
+				spin_lock(&fat12_entry_lock);
 			memcpy(c_bh->b_data, bhs[n]->b_data, sb->s_blocksize);
+			if (is_fat12)
+				spin_unlock(&fat12_entry_lock);
 			set_buffer_uptodate(c_bh);
 			unlock_buffer(c_bh);
 			mark_buffer_dirty_inode(c_bh, sbi->fat_inode);
-- 
2.43.0