[PATCH] fat: fix data race in fat_add_entries() over unlocked buffer write

[Igor Putko]

fat_add_entries() writes new directory entries directly into
bh->b_data via memcpy() without holding the buffer lock. This is
inconsistent with every other buffer-filling call site in this file
(fat_zeroed_cluster(), fat_alloc_new_dir(), fat_add_new_entries())
and in fs/fat/fatent.c (fat_mirror_bhs()), all of which wrap their
memcpy()/memset() calls in lock_buffer()/unlock_buffer() specifically
"to avoid race with userspace read via bdev".

Commit 07bfa4415ab6 ("fat: work around race with userspace's read
via blockdev while mounting") added that locking to those four call
sites in 2019, but missed the structurally identical pattern in
fat_add_entries()'s "Second stage: filling the free entries with new
entries" block, which writes both the long-name and short-name slots
into pre-existing (already uptodate) buffer heads the same way.

Since a FAT directory's buffer_head aliases a page in the backing
block device's page cache, a concurrent write into that page via the
bdev (e.g. through a loop device backed by shmem, as in the syzbot
reproducer) races with this unlocked memcpy(), as reported by KCSAN.

Wrap both the long-name and short-name slot writes in
fat_add_entries() with lock_buffer()/unlock_buffer(), matching the
pattern used everywhere else in this file.

Reported-by: syzbot+f72da8b30ddc89cc2371@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=f72da8b30ddc89cc2371
Fixes: 07bfa4415ab6 ("fat: work around race with userspace's read via blockdev while mounting")
Signed-off-by: Igor Putko <igorpetindev@xxxxxxxxx>
---
 fs/fat/dir.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 35bdb6294..c9e2bf472 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -1394,7 +1394,10 @@ int fat_add_entries(struct inode *dir, void *slots, int nr_slots,
 		/* Fill the long name slots. */
 		for (i = 0; i < long_bhs; i++) {
 			int copy = umin(sb->s_blocksize - offset, size);
+			/* Avoid race with userspace read via bdev */
+			lock_buffer(bhs[i]);
 			memcpy(bhs[i]->b_data + offset, slots, copy);
+			unlock_buffer(bhs[i]);
 			mmb_mark_buffer_dirty(bhs[i],
 					      &MSDOS_I(dir)->i_metadata_bhs);
 			offset = 0;
@@ -1406,7 +1409,10 @@ int fat_add_entries(struct inode *dir, void *slots, int nr_slots,
 		if (!err && i < nr_bhs) {
 			/* Fill the short name slot. */
 			int copy = umin(sb->s_blocksize - offset, size);
+			/* Avoid race with userspace read via bdev */
+			lock_buffer(bhs[i]);
 			memcpy(bhs[i]->b_data + offset, slots, copy);
+			unlock_buffer(bhs[i]);
 			mmb_mark_buffer_dirty(bhs[i],
 					      &MSDOS_I(dir)->i_metadata_bhs);
 			if (IS_DIRSYNC(dir))
-- 
2.47.3