[GH-ISSUE #58] msdosfs: on 1024-byte-sector labelled partitions the kernel addresses blocks in 512-byte units, corrupting reads and the FAT #51

New issue

Closed

opened 2026-11-03 22:03:14 -07:00 by gitea-mirror · 1 comment

gitea-mirror commented

2026-11-03 22:03:14 -07:00

Owner

Originally created by @raynorpat on GitHub (Oct 16, 2026).
Original GitHub issue: https://github.com/RhapsodiOS/RhapsodiOS/issues/58

Symptom

On partition a of a disk whose NeXT label uses 1024-byte sectors (the vm/ffsbench.py build_scratch layout, like the installed root disk), an msdos volume made correctly by the fixed newfs_msdos (#56; 1024 bytes/sector, passes fsck_msdos -n) mounts, but:

Reads fail: reading back small files and a subdirectory fails with EINVAL.
The FAT is corrupted: after unmounting, fsck_msdos finds FAT 1 entries at about twice the cluster numbers written in FAT 0.

The same tests on a 512-byte-label partition pass: a 3 MB file, a directory and 11 small files written, remounted, read back, and fsck clean.

Cause (from a probe on the guest)

On the 1024-label partition /dev/rhd1a, DKIOCBLKSIZE/DKIOCNUMBLKS report 512 × 1048768, which is the whole drive, while DKIOCGPARTINFO/DKIOCGLABEL report 1024 × 524288. The kernel's msdosfs computes block numbers in 512-byte DEV_BSIZE units (de_bn2kb/cntobn-style conversions, and bread with DEV_BSIZE-based block numbers), but the IDE disk driver interprets a block number on this partition in the label's 1024-byte units. Every metadata and data address is therefore doubled.

Reproduce

i386 QEMU guest on a kernel with msdosfs (e.g. vm/work/stress-phase3/mach_kernel), hd1 from ffsbench.build_scratch:

newfs_msdos /dev/rhd1a          # ffs-perf's fixed newfs_msdos (84cd4eed3)
mount_msdos /dev/hd1a /mnt/m
cp some files; ls -lR /mnt/m; umount /mnt/m; fsck_msdos -n /dev/rhd1a

Probe and scripts: vm/work/issue56/ (probe.c).

Where to look

Block numbers: bsd/msdosfs/msdosfs_vfsops.c msdosfs_mountfs(), and how the block numbers it hands to bread/VOP_STRATEGY are scaled. FFS on this kernel uses the device's block size (or the label's) when it converts fragments to device blocks. msdosfs assumes 512 (DEV_BSIZE). Scale by the partition's block size (DKIOCGPARTINFO/DKIOCBLKSIZE on the partition) the way ffs_mountfs does.
Missing check: also in msdosfs_mountfs, reject a volume whose BPB total sectors × bytes per sector exceeds the partition size. Today a half-made or oversized volume mounts silently.

Found while fixing #56. Related: #47 (raw-offset handling on i386 IDE), #54, #55.

Originally created by @raynorpat on GitHub (Oct 16, 2026). Original GitHub issue: https://github.com/RhapsodiOS/RhapsodiOS/issues/58 ## Symptom On partition `a` of a disk whose NeXT label uses 1024-byte sectors (the `vm/ffsbench.py build_scratch` layout, like the installed root disk), an msdos volume made correctly by the fixed `newfs_msdos` (#56; 1024 bytes/sector, passes `fsck_msdos -n`) mounts, but: - **Reads fail:** reading back small files and a subdirectory fails with EINVAL. - **The FAT is corrupted:** after unmounting, `fsck_msdos` finds FAT 1 entries at about twice the cluster numbers written in FAT 0. The same tests on a 512-byte-label partition pass: a 3 MB file, a directory and 11 small files written, remounted, read back, and fsck clean. ## Cause (from a probe on the guest) On the 1024-label partition `/dev/rhd1a`, `DKIOCBLKSIZE`/`DKIOCNUMBLKS` report 512 × 1048768, which is the whole drive, while `DKIOCGPARTINFO`/`DKIOCGLABEL` report 1024 × 524288. The kernel's msdosfs computes block numbers in 512-byte `DEV_BSIZE` units (`de_bn2kb`/`cntobn`-style conversions, and `bread` with `DEV_BSIZE`-based block numbers), but the IDE disk driver interprets a block number on this partition in the label's 1024-byte units. Every metadata and data address is therefore doubled. ## Reproduce i386 QEMU guest on a kernel with msdosfs (e.g. `vm/work/stress-phase3/mach_kernel`), hd1 from `ffsbench.build_scratch`: ``` newfs_msdos /dev/rhd1a # ffs-perf's fixed newfs_msdos (84cd4eed3) mount_msdos /dev/hd1a /mnt/m cp some files; ls -lR /mnt/m; umount /mnt/m; fsck_msdos -n /dev/rhd1a ``` Probe and scripts: `vm/work/issue56/` (`probe.c`). ## Where to look - **Block numbers:** `bsd/msdosfs/msdosfs_vfsops.c` `msdosfs_mountfs()`, and how the block numbers it hands to `bread`/`VOP_STRATEGY` are scaled. FFS on this kernel uses the device's block size (or the label's) when it converts fragments to device blocks. msdosfs assumes 512 (`DEV_BSIZE`). Scale by the partition's block size (`DKIOCGPARTINFO`/`DKIOCBLKSIZE` on the partition) the way `ffs_mountfs` does. - **Missing check:** also in `msdosfs_mountfs`, reject a volume whose BPB total sectors × bytes per sector exceeds the partition size. Today a half-made or oversized volume mounts silently. Found while fixing #56. Related: #47 (raw-offset handling on i386 IDE), #54, #55.

gitea-mirror closed this issue

2026-11-03 22:03:14 -07:00

gitea-mirror commented

2026-11-03 22:03:16 -07:00

Author

Owner

@raynorpat commented on GitHub (Oct 16, 2026):

Fixed by 883ed93c2 (kernel: address msdosfs blocks in the device's block size and refuse volumes larger than the partition), merged into ffs-perf in 83e9d741b.

Root cause. On the block device, the i386 IDE driver counts b_blkno in the partition's block size (1024 on these labels): ata_hd_strategy passes it straight to the disk (bsd/dev/ata_hd_registry.m:886-897), and ata_hd_size reports the label block size, which set_blocksize stores as v_specsize / VOP_DEVBLOCKSIZE. msdosfs hard-coded DEV_BSIZE (512) for pm_BlkPerSec, pm_bnshift, pm_bpcluster, pm_rootdirsize, pm_fatblocksec, the FAT block size (msdosfs_fat.c) and the root directory size (msdosfs_denode.c), so every address was doubled. FFS already scales by VOP_DEVBLOCKSIZE.

Fix.

mountmsdosfs uses VOP_DEVBLOCKSIZE everywhere it used DEV_BSIZE.
It now refuses, with EINVAL and a console message, a volume whose bytes/sector is smaller than the device block size.
It also refuses a volume whose BPB size exceeds the partition (from DKIOCGPARTINFO).

Verified on a kernel with this fix and a rebuilt driverkit (so #47 is included). On a 1024-byte-label and a 512-byte-label partition, with default and -s sizes:

newfs, a 3 MB file plus a directory tree and 20 files, remount, cmp/diff -r equal, and fsck_msdos -n clean.
An oversized volume is refused at mount on both.
stress-fs rw/append/tree/fsync and stress-mmap shared/mixed pass on both msdos mounts.
UFS and the root fs are unaffected.
Before the fix, on the same kernel line, cp -r failed with EINVAL and fsck found FAT 0/1 mismatches.

Limitation. A FAT volume with 512 bytes/sector on a 1024-byte-block partition is refused at mount, not supported. Standard layouts put the FAT at byte 512, which a 1024-unit block device can't address, and #56's newfs already refuses -S 512 there.

Not checked: the ppc build and GEMDOS mounts. Separately, newfs_msdos -S 1024 on a 512-byte partition without -s sizes the volume at twice the partition; the new mount check refuses it.

@raynorpat commented on GitHub (Oct 16, 2026): Fixed by 883ed93c2 (kernel: address msdosfs blocks in the device's block size and refuse volumes larger than the partition), merged into ffs-perf in 83e9d741b. **Root cause.** On the block device, the i386 IDE driver counts `b_blkno` in the partition's block size (1024 on these labels): `ata_hd_strategy` passes it straight to the disk (`bsd/dev/ata_hd_registry.m:886-897`), and `ata_hd_size` reports the label block size, which `set_blocksize` stores as `v_specsize` / `VOP_DEVBLOCKSIZE`. msdosfs hard-coded `DEV_BSIZE` (512) for `pm_BlkPerSec`, `pm_bnshift`, `pm_bpcluster`, `pm_rootdirsize`, `pm_fatblocksec`, the FAT block size (`msdosfs_fat.c`) and the root directory size (`msdosfs_denode.c`), so every address was doubled. FFS already scales by `VOP_DEVBLOCKSIZE`. **Fix.** - `mountmsdosfs` uses `VOP_DEVBLOCKSIZE` everywhere it used `DEV_BSIZE`. - It now refuses, with EINVAL and a console message, a volume whose bytes/sector is smaller than the device block size. - It also refuses a volume whose BPB size exceeds the partition (from `DKIOCGPARTINFO`). **Verified** on a kernel with this fix and a rebuilt driverkit (so #47 is included). On a 1024-byte-label and a 512-byte-label partition, with default and `-s` sizes: - newfs, a 3 MB file plus a directory tree and 20 files, remount, `cmp`/`diff -r` equal, and `fsck_msdos -n` clean. - An oversized volume is refused at mount on both. - `stress-fs` rw/append/tree/fsync and `stress-mmap` shared/mixed pass on both msdos mounts. - UFS and the root fs are unaffected. Before the fix, on the same kernel line, `cp -r` failed with EINVAL and fsck found FAT 0/1 mismatches. **Limitation.** A FAT volume with 512 bytes/sector on a 1024-byte-block partition is refused at mount, not supported. Standard layouts put the FAT at byte 512, which a 1024-unit block device can't address, and #56's newfs already refuses `-S 512` there. **Not checked:** the ppc build and GEMDOS mounts. Separately, `newfs_msdos -S 1024` on a 512-byte partition without `-s` sizes the volume at twice the partition; the new mount check refuses it.