On Tue, Jul 29, 2025 at 10:35:58AM +0100, Al Viro wrote: > On Tue, Jul 29, 2025 at 02:17:10PM +0800, Edward Adam Davis wrote: > > syzbot reports data-race in fat32_ent_get/fat32_ent_put. > > > > CPU0(Task A) CPU1(Task B) > > ==== ==== > > vfs_write > > new_sync_write > > generic_file_write_iter > > fat_write_begin > > block_write_begin vfs_statfs > > fat_get_block statfs_by_dentry > > fat_add_cluster fat_statfs Sorry, no - you've missed an intermediate fat_chain_add() in the call chain here. And that makes your race a non-issue. > > fat_ent_write fat_count_free_clusters > > fat32_ent_put fat32_ent_get fat_count_free_clusters() doesn't care about exact value of entry; the only thing that matters is whether it's equal to FAT_ENT_FREE. Actualy changes of that predicate (i.e. allocating and freeing of clusters) still happens under fat_lock() - nothing has changed in that area. *That* is not happening simultaneously with reads in fat_count_free_clusters(). It's attaching the new element to the end of chain that is outside of fat_lock(). And that operation does not affect that predicate at all; it changes the value of the entry for last cluster of file (FAT_ENT_EOF) to the number of cluster being added to the file. Neither is equal to FAT_ENT_FREE, so there's no problem - value you get ->ent_get() is affected by that store, the value of ops->ent_get(&fatent) == FAT_ENT_FREE isn't. Probably worth a comment in fat_chain_add() re "this store is safe outside of fat_lock() because of <list of reasons>". Changes to this chain (both extending and truncating it) are excluded by <lock> and as far as allocator is concerned, we are not changing the state of any cluster. Basically, FAT encodes both the free block map (entry[block] == FAT_ENT_FREE <=> block is not in use) and linked lists of blocks for individual files - they store the number of file's first block within directory entry and use FAT for forwards pointers in the list. FAT_ENT_EOF is used for "no more blocks, it's the end of file". Allocator and fat_count_free_clusters care only about the free block map part of that thing; access to file contents - the linked list for that file.
