Bug 270053 - msdosfs: statfs tracks total file nodes, but not free file nodes
Summary: msdosfs: statfs tracks total file nodes, but not free file nodes
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Stefan Eßer
URL: https://reviews.freebsd.org/D38987
Keywords: patch
Depends on:
Blocks:
 
Reported: 2023-03-08 23:04 UTC by Ben Woods
Modified: 2023-05-01 08:10 UTC (History)
1 user (show)

See Also:


Attachments
Test script used to validate the patch in review D38987 (2.81 KB, text/plain)
2023-03-25 22:29 UTC, Stefan Eßer
no flags Details
Test script used to validate the patch in review D38987 (2.91 KB, text/plain)
2023-03-28 21:34 UTC, Stefan Eßer
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ben Woods freebsd_committer freebsd_triage 2023-03-08 23:04:05 UTC
The msdosfs_vfsops.c code is using the statfs(2) f_files limit variable to represent the FAT root directory entities limit, but is not using the corresponding f_ffree available variable to represent the current number of root directory entities. This leads to the df(1) output incorrectly implying that the number of root directory entities is at capacity.

To avoid this confusion, I think the msdosfs_vfsops.c should either set both the f_files and f_ffree statfs variables so that df(1) correctly reports %iused, or should set them both to zero so that df(1) shows %iused as "-".

Example output from FreeBSD 13.1-RELEASE on a FAT16 file system:

$ df -i /boot/efi
Filesystem  512-blocks Used  Avail Capacity iused ifree %iused  Mounted on
/dev/nvd0p1     532352 3680 528672     1%     512     0  100%   /boot/efi
Comment 1 Ben Woods freebsd_committer freebsd_triage 2023-03-08 23:05:37 UTC
Stefan has provided a patch, which can be found in the following phabricator review:
https://reviews.freebsd.org/D38987
Comment 2 Ben Woods freebsd_committer freebsd_triage 2023-03-08 23:08:07 UTC
This was discussed on the freebsd-fs mailing list here:
https://lists.freebsd.org/archives/freebsd-fs/2023-March/001934.html
Comment 3 Stefan Eßer freebsd_committer freebsd_triage 2023-03-25 22:29:40 UTC
Created attachment 241116 [details]
Test script used to validate the patch in review D38987

The latest version of the patch in review D38987 passes the tests using all the file system parameters defined in this script.
Comment 4 Stefan Eßer freebsd_committer freebsd_triage 2023-03-28 21:32:34 UTC
Comment on attachment 241116 [details]
Test script used to validate the patch in review D38987

#!/bin/sh

export BLOCKSIZE=1k
test_dir=/mnt

perform_test () {
	disk_size=$1; shift
	fat_bits=$1; shift
	options="$@"
	md_device=$(mdconfig -s ${disk_size})

	echo "# newfs_msdos -F ${fat_bits} -s ${disk_size} ${options}"
	if newfs_msdos -F ${fat_bits} ${options} /dev/${md_device}; then
		mount -t msdos -o async /dev/${md_device} ${test_dir}
		df0=$(df -i /dev/${md_device})
		echo "${df0}"
		inodes=$(echo "${df0}" | tail -1 | cut -w -f7)
		echo "# Inodes: $inodes"
		
		for i in $(jot $inodes); do
			touch "${test_dir}/TST${i}.DAT" || break
		done

		df1=$(df -i /dev/${md_device})
		echo "${df1}"
		echo "One more file ..." > "${test_dir}/OVERFLOW.DAT"
		umount /dev/${md_device}

		mount -t msdos /dev/${md_device} ${test_dir}
		df2=$(df -i /dev/${md_device})
		echo "${df2}"
		umount /dev/${md_device}

		if [ "${df1}" != "${df2}" ]; then
			echo "# --- FAILED ---"
		fi
	else
		echo "# FAILED"
	fi
	mdconfig -d -u ${md_device}
	echo
}

while read options
do
	perform_test 64m 12 $options
	perform_test 1g 16 $options
done <<*EOD
-n 2 -e 8 -S 512
-n 2 -e 8 -S 512 -c 4
-n 2 -e 8 -S 4096
-n 2 -e 8 -S 4096 -c 2
-n 2 -e 511 -S 512
-n 2 -e 512 -S 1024
-n 2 -e 512 -S 1024 -c 1
-n 2 -e 512 -S 1024 -c 2
-n 2 -e 512 -S 1024 -c 4
-n 2 -e 512 -S 1024 -c 8
-n 2 -e 512 -S 2048
-n 2 -e 512 -S 4096
-n 2 -e 512 -S 4096 -c 1
-n 2 -e 512 -S 4096 -c 2
-n 2 -e 512 -S 4096 -c 4
-n 2 -e 512 -S 4096 -c 8
-n 2 -e 512 -S 512
-n 2 -e 512 -S 512 -c 1
-n 2 -e 512 -S 512 -c 2
-n 2 -e 512 -S 512 -c 4
-n 2 -e 512 -S 512 -c 8
-n 2 -e 513 -S 512
-n 3 -e 512 -S 512
-n 2 -e 513 -S 512 -c 1
-n 2 -e 513 -S 512 -c 8
-n 2 -e 513 -S 4096 -c 1
-n 2 -e 513 -S 4096 -c 2
-n 2 -e 1024 -S 512
-n 2 -e 2048 -S 512
-n 2 -e 4096 -S 512
-n 2 -e 8192 -S 512
-n 2 -e 16384 -S 512
-n 2 -e 32768 -S 512
-n 2 -e 65535 -S 512
*EOD

echo "# Done"

exit 0

     -F FAT-type
             FAT type (one of 12, 16, or 32).

     -L label
             Volume label (up to 11 characters).  The label should consist of
             only those characters permitted in regular DOS (8+3) filenames.

     -S sector-size
             Number of bytes per sector.  Acceptable values are powers of 2 in
             the range 512 through 32768, inclusive.

     -a FAT-size
             Number of sectors per FAT.

     -b block-size
             File system block size (bytes per cluster).  This should resolve
             to an acceptable number of sectors per cluster (see below).

     -c cluster-size
             Sectors per cluster.  Acceptable values are powers of 2 in the
             range 1 through 128.  If the block or cluster size are not
             specified, the code uses a cluster between 512 bytes and 32K
             depending on the filesystem size.

     -e DirEnts
             Number of root directory entries (FAT12 and FAT16 only).

     -n FATs
             Number of FATs.  Acceptable values are 1 to 16 inclusive.  The
             default is 2.

     -s total
             File system size.
Comment 5 Stefan Eßer freebsd_committer freebsd_triage 2023-03-28 21:34:29 UTC
Created attachment 241169 [details]
Test script used to validate the patch in review D38987
Comment 6 commit-hook freebsd_committer freebsd_triage 2023-03-29 07:51:36 UTC
A commit in branch main references this bug:

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

commit c33db74b5323480fba7adef58e8aa88f6091d134
Author:     Stefan Eßer <se@FreeBSD.org>
AuthorDate: 2023-03-29 06:46:01 +0000
Commit:     Stefan Eßer <se@FreeBSD.org>
CommitDate: 2023-03-29 06:46:01 +0000

    fs/msdosfs: add tracking of free root directory entries

    This update implements tallying of free directory entries during
    create, delete, or rename operations on FAT12 and FAT16 file systems.

    Prior to this change, the total number of root directory entries
    was reported as number of inodes, but 0 as the number of free
    inodes, causing system health monitoring software to warn about
    a suspected disk full issue.

    The FAT12 and FAT16 file systems provide a limited number of
    root directory entries, e.g. 512 on typical hard disk formats.
    The valid range of values is 1 to 65535, but the msdosfs code
    will effectively round up "odd" values to the next multiple of 16
    (e.g. 513 would allow for 528 root directory entries).

    This update implements tracking of directory entries during create,
    delete, or rename operations, with initial values determined by
    scanning the directory when the file system is mounted.

    Total and free directory entries are reported in the f_files and
    f_ffree elements of struct statfs, despite differences in semantics
    of these values:

    - There is no limit on the number of files and directories that can
      be created on a FAT file system. Only the root directory of FAT12
      and FAT16 file systems is limited, any number of files can still be
      created in sub-directories, even when 0 free "inodes" are reported.

    - A single file can require 1 to 21 directory entries, depending on
      the character set, structure, and length of the name. The DOS 8.3
      style file name takes up 1 entry, and if the name does not comply
      with the syntax of a DOS 8.3 file name, 1 additional entry is used
      for each 13 characters of the file name. Since all these entries
      have to be contiguous, it is possible that a file or directory with
      a long name can not be created, despite a sufficient total number of
      free directory entries.

    - Renaming a file can require more directory entries than currently
      allocated to store its long name, which may prevent an in-place
      update of the name if more entries are needed. This may cause a
      rename operation to fail if no contiguous range of free entries for
      the new name can be found.

    - The volume label is stored in a directory entry. An empty FAT file
      system with a volume label will therefore show 1 used "inode" in
      df.

    - The perceentage of free inodes shown in df or monitoring tools does
      only represent the state of the root directory of a FAT12 or FAT16
      file system. Neither does a reported value of 0% free inodes does
      prevent files from being created in sub-directories, nor does a
      value of 50% free inodes guarantee that even a single file with
      a "long" name can be created in the root directory (if every other
      directory entry is occupied and there are no 2 contiguous entries).

    The statfs(2) and df(1) man pages have been updated with a notice
    regarding the possibly different semantics of values reported as
    total and free inodes for non-Unix file systems.

    PR:             270053
    Reported by:    Ben Woods <woodsb02@freebsd.org>
    Approved by:    mckusick
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D38987

 bin/df/df.1                     |  13 ++++-
 lib/libc/sys/statfs.2           |  18 ++++++-
 sys/fs/msdosfs/msdosfs_lookup.c |   3 ++
 sys/fs/msdosfs/msdosfs_vfsops.c | 105 +++++++++++++++++++++++++++++++++++++++-
 sys/fs/msdosfs/msdosfsmount.h   |  17 +++++++
 5 files changed, 152 insertions(+), 4 deletions(-)
Comment 7 commit-hook freebsd_committer freebsd_triage 2023-05-01 08:10:38 UTC
A commit in branch stable/13 references this bug:

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

commit d767bf361b3ebdb3955473cd378f8a8dcf9c85f0
Author:     Stefan Eßer <se@FreeBSD.org>
AuthorDate: 2023-03-08 16:58:00 +0000
Commit:     Stefan Eßer <se@FreeBSD.org>
CommitDate: 2023-05-01 08:09:33 +0000

    msdosfs: fix debug print format and parameter

    Building with -DMSDOSFS_DEBUG failed due to a format mismatch and
    a variable that has been renamed but not updated in the printf()
    parameter list.

    (cherry picked from commit 2d8cf575d5778781928699f9b7cfb448bd2f1f8e)

    fs/msdosfs: add tracking of free root directory entries

    This update implements tallying of free directory entries during
    create, delete, or rename operations on FAT12 and FAT16 file systems.

    Prior to this change, the total number of root directory entries
    was reported as number of inodes, but 0 as the number of free
    inodes, causing system health monitoring software to warn about
    a suspected disk full issue.

    The FAT12 and FAT16 file systems provide a limited number of
    root directory entries, e.g. 512 on typical hard disk formats.
    The valid range of values is 1 to 65535, but the msdosfs code
    will effectively round up "odd" values to the next multiple of 16
    (e.g. 513 would allow for 528 root directory entries).

    This update implements tracking of directory entries during create,
    delete, or rename operations, with initial values determined by
    scanning the directory when the file system is mounted.

    Total and free directory entries are reported in the f_files and
    f_ffree elements of struct statfs, despite differences in semantics
    of these values:

    - There is no limit on the number of files and directories that can
      be created on a FAT file system. Only the root directory of FAT12
      and FAT16 file systems is limited, any number of files can still be
      created in sub-directories, even when 0 free "inodes" are reported.

    - A single file can require 1 to 21 directory entries, depending on
      the character set, structure, and length of the name. The DOS 8.3
      style file name takes up 1 entry, and if the name does not comply
      with the syntax of a DOS 8.3 file name, 1 additional entry is used
      for each 13 characters of the file name. Since all these entries
      have to be contiguous, it is possible that a file or directory with
      a long name can not be created, despite a sufficient total number of
      free directory entries.

    - Renaming a file can require more directory entries than currently
      allocated to store its long name, which may prevent an in-place
      update of the name if more entries are needed. This may cause a
      rename operation to fail if no contiguous range of free entries for
      the new name can be found.

    - The volume label is stored in a directory entry. An empty FAT file
      system with a volume label will therefore show 1 used "inode" in
      df.

    - The perceentage of free inodes shown in df or monitoring tools does
      only represent the state of the root directory of a FAT12 or FAT16
      file system. Neither does a reported value of 0% free inodes does
      prevent files from being created in sub-directories, nor does a
      value of 50% free inodes guarantee that even a single file with
      a "long" name can be created in the root directory (if every other
      directory entry is occupied and there are no 2 contiguous entries).

    The statfs(2) and df(1) man pages have been updated with a notice
    regarding the possibly different semantics of values reported as
    total and free inodes for non-Unix file systems.

    PR:             270053
    Reported by:    Ben Woods <woodsb02@freebsd.org>
    Approved by:    mckusick
    Differential Revision:  https://reviews.freebsd.org/D38987

    (cherry picked from commit c33db74b5323480fba7adef58e8aa88f6091d134)

    fs/msdosfs: Fix potential panic and size calculations

    Some combinations of FAT12 file system parameters could cause a kernel
    panic due to an unmapped access if the size of the FAT was larger than
    the CPU page size. The reason is that FAT12 uses 3 bytes to store
    2 FAT pointers, leading to partial FAT pointers at the end of buffers
    of a size that is not a multiple of 3.

    With a typical page size of 4 KB, this caused the FAT entry at byte
    offsets 4095 and 4096 to cross the page boundary, with only the first
    page mapped. This was fixed by adjusting the mapping to always cover
    both bytes of each FAT entry.

    Testing revealed 2 other inconsistencies that are fixed by this commit:

    1) The calculation of the size of the data area did not take into
       account the fact that the first two data block numbers are reserved
       and that the data area starts with block 2. This could cause a
       FAT12 file system created with the maximum supported number of
       blocks to be incorrectly identified as FAT16.

    2) The root directory does not take up space in the data area of a
       FAT12 or FAT16 file system, since it is placed into a reserved
       area outside of that data area. This commits makes stat() report
       the logical size of the root directory, but with 0 blocks allocated
       from the data area.

    PR:             270587
    Reviewed by:    mckusick
    Differential Revision:  https://reviews.freebsd.org/D39386

    (cherry picked from commit 0728695c63efda298feccefb3615c23cb6682929)

 bin/df/df.1                     |  13 ++++-
 lib/libc/sys/statfs.2           |  18 +++++-
 sys/fs/msdosfs/msdosfs_denode.c |   2 +-
 sys/fs/msdosfs/msdosfs_fat.c    |  12 +++-
 sys/fs/msdosfs/msdosfs_lookup.c |   5 +-
 sys/fs/msdosfs/msdosfs_vfsops.c | 120 +++++++++++++++++++++++++++++++++++++---
 sys/fs/msdosfs/msdosfs_vnops.c  |   7 ++-
 sys/fs/msdosfs/msdosfsmount.h   |  17 ++++++
 8 files changed, 177 insertions(+), 17 deletions(-)