search

dorimanx / exfat-nofuse

Android ARM Linux non-fuse read/write kernel driver for exFat and VFat Android file systems
GNU General Public License v2.0
707 stars 326 forks source link

LOCKBIT mechanism for the buffer cache doesn't feel safe #128

Open PypeBros opened 6 years ago

PypeBros commented 6 years ago

Hi. I'm currently having a review of your exFAT driver.

The LOCKBIT mechanism in exfat_cache seems either incomplete or un-needed. There are many places where a buffer is first located with buf_getblk(), and later locked with buf_lock(). Nothing really guarantees (from an exfat_cache perspective) that the sector used by that cache entry hasn't changed between the two operations, although I expect them to be unlikely given that the buffer returned by bug_getblk() is pushed at the head of the recently-used list while allocating happens near the tail.

Similarly, I see many functions doing get_entry_in_dir(); /* modify entry */ ; buf_modify(); activity, with no strong guarantee that the contents of the cached entry remains the same between read and modify. Some of these functions will obviously be mutually exclusive due to the v_sem (volume-level semaphore ?). Exclusive access is less clear for others (e.g. exfat_init_dir_entry).

Finally, I note that exfat_cache doesn't prevent alone that all entries in the LRU list get their LOCKBIT set, in which case buf_cache_get() would happily return the LRU list header, which __buf_getblk() doesn't seem to check. Next call to buf_cache_get() could then loop forever.

If some higher-level exclusive access makes all the above non-issues, then what is the value added by LOCKBIT ?

Best regards, /PypeBros.

v-fox commented 6 years ago

Well, as far as I know, it's not his exFAT driver, it's a fork of abandoned Samsung's implementation. So if you know better how it should be designed, please, for the benefit of whole community, push some patches here. SD-cards are only going to get bigger and most Android builds refuse to work with anything but the FAT crap which is the weakest part of Linux kernel now (in-kernel NTFS implementation is long obsolete and exFAT just doesn't exist).