Messages in this thread |  | | From | OGAWA Hirofumi <> | | Subject | Re: [PATCH] drivers: example: fix memory leak | | Date | Tue, 02 Sep 2025 21:38:34 +0900 |
| |
YangWen <anmuxixixi@gmail.com> writes:
> On Tue, 02 Sep 2025 23:13:42 +0900, OGAWA Hirofumi wrote: >> Hm, what is wrong with temporary inconsistent? >> >> If it had the race with future modification, it can be temporary >> inconsistent. However, future modification will fix it by updating with >> latest blocks, right? >> >> Or did you actually get the inconsistent state after clean unmount? > > Thanks for your comment. > > This is not only a temporary in-memory inconsistency. KCSAN detected a > race where fat12_ent_put() updates two bytes of a 12-bit FAT entry while > fat_mirror_bhs() concurrently memcpy()’s the entire sector. The mirror > FAT may therefore receive a torn entry. > > Since fat_mirror_bhs() marks those buffers dirty, the corrupted mirror > content can be flushed to disk. In our syzkaller testing, this already > resulted in runtime errors such as: > > FAT-fs (loop4): error, clusters badly computed (421 != 418) > FAT-fs (loop4): error, fat_bmap_cluster: request beyond EOF (i_pos 2075) > > These errors occurred even after a clean unmount, which suggests that the > inconsistent FAT entries were actually written to disk and not corrected > later by "future modification". > > FAT16/32 do not suffer from this problem because their entries are > naturally aligned 16/32-bit accesses, which are atomic on supported > architectures. FAT12 is special because of the 12-bit packing across > two bytes. > > So I think it is necessary to protect memcpy() in fat_mirror_bhs() with > fat12_entry_lock to avoid copying a torn FAT12 entry.
Sounds like strange. FAT driver never read the mirror FAT area in runtime. Why did you think the mirror FAT affect to it?
Thanks. -- OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
|  |