Bug 277239 - Potential inode collision on FAT12 and FAT16 causes directory corruption
Summary: Potential inode collision on FAT12 and FAT16 causes directory corruption
Status: In Progress
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: Unspecified
Hardware: Any Any
: --- Affects Some People
Assignee: Stefan Eßer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-23 10:43 UTC by Stefan Eßer
Modified: 2024-02-23 17:37 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Eßer freebsd_committer freebsd_triage 2024-02-23 10:43:44 UTC
FAT file systems do not use inodes, instead all file meta-information is stored in directory entries.
FAT12 and FAT16 use a fixed size area for root directories, with typically 512 entries of 32 bytes each (for a total of 16 KB) on hard disk formats.

The file system data is stored in clusters of typically 512 to 4096 bytes (max. supported is 32 KB)
The current code uses the offset of a DOS 8.3 style directory entry as a pseudo-inode, which leads to inode values of 0 to 16368 for typical root directory entries.
Sub-directories use 2 cluster length plus the byte offset of the directory entry in the data area for the pseudo-inode, which may be as low as 1024 in case of 512 byte clusters, 8192 in case of 4 KB clusters.

This issue is demonstrated by the following test script:

#!/bin/sh

FS=/mnt/MSDOSFS-TEST
MDUNIT=0

cleanup () {
	cd /
	umount /dev/md$MDUNIT
	rm -rf $FS
	fsck_msdosfs -n /dev/md$MDUNIT
	mdconfig -u $MDUNIT -d
}

mdconfig -u $MDUNIT -t malloc -s 4m
newfs_msdos -F 16 -c 1 /dev/md$MDUNIT
mkdir -p $FS
mount -t msdos /dev/md$MDUNIT $FS

trap "cleanup" EXIT

cd $FS
mkdir TESTDIR
touch TESTDIR/TEST

for i in $(jot 33)
do
	touch TEST.$i
done

This script reports an error when writing the 32nd entry and the file system will have been remounted r/o when the 33rd file is to be written:

touch: TEST.32: Bad file descriptor
touch: TEST.33: Read-only file system

The 32th file gets a pseudo-inode value of 1024, the same value already assigned to TESTDIR, leading to a directory and a file with identical inode numbers.

The patch in review D43978 changes the calculation of pseudo-inodes to account for the actual number of directory entries in the root file system and avoids the collision for all supported file-system parameters.
Comment 1 Stefan Eßer freebsd_committer freebsd_triage 2024-02-23 10:47:05 UTC
The fix in review D43978 has been committed to the main branch with commit ID 445d3d227e68f. An MFC to stable/13 and stable/14 will be performed after 3 days.
A commit to releng/13.3 is planned, if approved by the release engineer.
Comment 2 commit-hook freebsd_committer freebsd_triage 2024-02-23 10:50:38 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=fba1a994acfe31d957e9e26a5f12a6fdd1689bd2

commit fba1a994acfe31d957e9e26a5f12a6fdd1689bd2
Author:     Stefan Eßer <se@FreeBSD.org>
AuthorDate: 2024-02-20 12:02:24 +0000
Commit:     Stefan Eßer <se@FreeBSD.org>
CommitDate: 2024-02-23 10:47:58 +0000

    msdosfs: fix potential inode collision on FAT12 and FAT16

    PR:             277239
    Approved by:    mckusick

    (cherry picked from commit 445d3d227e68f85157d0301d1706aa488e8423da)

 sys/fs/msdosfs/denode.h         |  8 +++++++-
 sys/fs/msdosfs/msdosfs_denode.c | 19 ++++++++++++++++---
 sys/fs/msdosfs/msdosfs_lookup.c |  2 +-
 3 files changed, 24 insertions(+), 5 deletions(-)
Comment 3 commit-hook freebsd_committer freebsd_triage 2024-02-23 10:52:39 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=a495e7f5c5c2088bf32cd7349e2ca344ee089552

commit a495e7f5c5c2088bf32cd7349e2ca344ee089552
Author:     Stefan Eßer <se@FreeBSD.org>
AuthorDate: 2024-02-20 12:02:24 +0000
Commit:     Stefan Eßer <se@FreeBSD.org>
CommitDate: 2024-02-23 10:50:31 +0000

    msdosfs: fix potential inode collision on FAT12 and FAT16

    PR:             277239
    Approved by:    mckusick

    (cherry picked from commit 445d3d227e68f85157d0301d1706aa488e8423da)

 sys/fs/msdosfs/denode.h         |  8 +++++++-
 sys/fs/msdosfs/msdosfs_denode.c | 19 ++++++++++++++++---
 sys/fs/msdosfs/msdosfs_lookup.c |  2 +-
 3 files changed, 24 insertions(+), 5 deletions(-)
Comment 4 commit-hook freebsd_committer freebsd_triage 2024-02-23 17:37:31 UTC
A commit in branch releng/13.3 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=6831288c8c4091c533c590d2d3cbb07459f2d28b

commit 6831288c8c4091c533c590d2d3cbb07459f2d28b
Author:     Stefan Eßer <se@FreeBSD.org>
AuthorDate: 2024-02-20 12:02:24 +0000
Commit:     Stefan Eßer <se@FreeBSD.org>
CommitDate: 2024-02-23 12:42:34 +0000

    msdosfs: fix potential inode collision on FAT12 and FAT16

    FAT file systems do not use inodes, instead all file meta-information
    is stored in directory entries.

    FAT12 and FAT16 use a fixed size area for root directories, with
    typically 512 entries of 32 bytes each (for a total of 16 KB) on hard
    disk formats. The file system data is stored in clusters of typically
    512 to 4096 bytes, depending on the size of the file system.

    The current code uses the offset of a DOS 8.3 style directory entry as
    a pseudo-inode, which leads to inode values of 0 to 16368 for typical
    root directories with 512 entries.

    Sub-directories use 2 cluster length plus the byte offset of the
    directory entry in the data area for the pseudo-inode, which may be
    as low as 1024 in case of 512 byte clusters. A sub-directory in
    cluster 2 and with 512 byte clusters will therefore lead to a
    re-use of inode 1024 when there are at least 32 DOS 8.3 style
    filenames in the root directory (or 11 14-character Windows
    long file names, each of which takes up 3 directory entries).

    FAT32 file systems are not affected by this issue and FAT12/FAT16
    file systems with larger cluster sizes are unlikely to have as
    many directory entries in the root directory as are required to
    cause the collision.

    This commit leads to inode numbers that are guaranteed to not collide
    for all valid FAT12 and FAT16 file system parameters. It does also
    provide a small speed-up due to more efficient use of the vnode cache.

    PR:             277239
    Reviewed by:    mckusick
    Approved by:    re (cperciva)

    (cherry picked from commit 445d3d227e68f85157d0301d1706aa488e8423da)
    (cherry picked from commit a495e7f5c5c2088bf32cd7349e2ca344ee089552)

 sys/fs/msdosfs/denode.h         |  8 +++++++-
 sys/fs/msdosfs/msdosfs_denode.c | 19 ++++++++++++++++---
 sys/fs/msdosfs/msdosfs_lookup.c |  2 +-
 3 files changed, 24 insertions(+), 5 deletions(-)