Page MenuHomeFreeBSD

Fix MSDOSFS rename (in case target exists)
ClosedPublic

Authored by se on Feb 17 2024, 6:25 PM.
Tags
None
Referenced Files
Unknown Object (File)
Oct 4 2024, 5:36 AM
Unknown Object (File)
Oct 3 2024, 9:55 AM
Unknown Object (File)
Sep 25 2024, 4:13 PM
Unknown Object (File)
Sep 24 2024, 6:25 AM
Unknown Object (File)
Sep 23 2024, 5:20 AM
Unknown Object (File)
Sep 18 2024, 9:55 AM
Unknown Object (File)
Sep 17 2024, 10:50 PM
Unknown Object (File)
Sep 9 2024, 2:17 AM
Subscribers
None

Details

Summary
  • IMHO this is a candidate for the upcoming FreeBSD-13.3 release ***

The is a bug in MSDOSFS that can be triggered when the target of a rename operation exists. It is caused by the lack of inodes in the FAT file system, which are substituted by the location of the DOS 8.3 directory entry in the file system. This causes the "inode" of a file to change when its directory entry is moved to a different location.

The rename operation wants to re-use the existing directory entry position of an existing target file name (POS1). But the code does instead locate the first position in the directory that provides sufficient free directory slots (POS2) to hold the target file name and fills it with the directory data.
The rename operation continues and at the end writes directory data to the initially retrieved location (POS1) of the old target directory.
This leads to 2 directory entries for the target file, but with inconsistent data in the directory and in the cached file system state.
The location that should have been re-used (POS1) is marked as deleted in the directory, and new directory data has been written to a different location (POS2).
But the VFS cache has the newly written data stored under the inode number that corresponds to the initially planned position (POS1).
If then a new file is written, it can allocate the deleted directory entries (POS1) and when it queries the cache, it retrieves data that is valid for the target of the prior rename operation, leading to a corrupt directory entry (at POS1) being written (DOS file name of the earlier rename target combined with the Windows long file name of the newly written file).

Test Plan

Apply patch and create a FAT file system of either type (FAT12/FAT16/FAT32) and different file system parameters (e.g., varying cluster size).
Perform multiple rsync operations using the same source and targets, for example:

MDUNIT=0
FS=/mnt/test
mdconfig -u $MDUNIT -t malloc -s 512m
newfs_msdos -c 8 -F 32 /dev/md$MDUNIT
mkdir -p $FS
mount -t msdos /dev/md$MDUNIT $FS
rsync -r /usr/src/lib/libsysdecode $FS
rsync -r /usr/src/lib/libsysdecode $FS
rsync -r /usr/src/lib/libsysdecode $FS
umount $FS
fsck_msdos /dev/md$MDUNIT

This test copies a small set of files with names that are known to trigger the bug when writing to a FAT file system.
Without the patch, rsync reports a number of errors and the FAT file system will be remounted R/O due to directory corruption.
With the fix applied, the rsync operations succeed (also tested with larger source trees).

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

se requested review of this revision.Feb 17 2024, 6:25 PM
se created this revision.

Update diff to prefer the old location of the target directory.

This patch fixes an omission from the patch committed as 2c9cbc2d45b94.

This revision is now accepted and ready to land.Feb 17 2024, 6:59 PM

Non-obvious bug, but once figured out the fix is simple and clearly correct.