Re: [PATCH] fat: fix data race in fat_clusters_flush between free_clusters access

[tejas bharambe]

Thank you for the clarification. You're right — I could not reproduce actual data corruption from this race, and I was not aware that the FSINFO free cluster count is treated as advisory. I defer to your judgment and will withdraw the patch. I appreciate the review.
Regards,
Tejas Bharambe

________________________________________
From: OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>
Sent: Saturday, February 28, 2026 7:33 PM
To: tejas bharambe <tejas.bharambe@xxxxxxxxxxx>
Cc: linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>; syzbot+56be2a55de6142438317@xxxxxxxxxxxxxxxxxxxxxxxxx <syzbot+56be2a55de6142438317@xxxxxxxxxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH] fat: fix data race in fat_clusters_flush between free_clusters access
 
tejas bharambe <tejas.bharambe@xxxxxxxxxxx> writes:

> From b1b914bdde5bc450eb586141823ba34924606b49 Mon Sep 17 00:00:00 2001
> From: Tejas Bharambe <tejas.bharambe@xxxxxxxxxxx>
> Date: Sat, 28 Feb 2026 09:58:20 -0800
> Subject: [PATCH] fat: fix data race in fat_clusters_flush between
>  free_clusters access
>
> fat_clusters_flush() reads sbi->free_clusters and sbi->prev_free
> without holding sbi->fat_lock, while fat_alloc_clusters() modifies
> these fields under the lock. This causes a data race detected by KCSAN.
>
> Fix this by acquiring sbi->fat_lock around the read of these fields
> in fat_clusters_flush().
>
> Reported-by: syzbot+56be2a55de6142438317@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=56be2a55de6142438317
> Signed-off-by: Tejas Bharambe <tejas.bharambe@xxxxxxxxxxx>

What is the real issue with this race other than KCSAN says?
fat_clusters_flush() is called only after marked as dirty. And it is
dirty only after completed the modification of those fields.

AFAICT, on disk data is fixed until unmount even if there is temporary
corruption. Or can you reproduce the real corruption with this race?

Thanks.

> ---
>  fs/fat/misc.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> index b154a51627..7d257a5694 100644
> --- a/fs/fat/misc.c
> +++ b/fs/fat/misc.c
> @@ -89,10 +89,12 @@ int fat_clusters_flush(struct super_block *sb)
>                        le32_to_cpu(fsinfo->signature2),
>                        sbi->fsinfo_sector);
>         } else {
> +               mutex_lock(&sbi->fat_lock);
>                 if (sbi->free_clusters != -1)
>                         fsinfo->free_clusters = cpu_to_le32(sbi->free_clusters);
>                 if (sbi->prev_free != -1)
>                         fsinfo->next_cluster = cpu_to_le32(sbi->prev_free);
> +               mutex_unlock(&sbi->fat_lock);
>                 mark_buffer_dirty(bh);
>         }
>         brelse(bh);

--
OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>