[GH-ISSUE #56] msdosfs: newfs_msdos fails with EINVAL on 1024-byte-sector labelled disks and sizes the volume from the whole device #50

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 15, 2026).
Original GitHub issue: https://github.com/RhapsodiOS/RhapsodiOS/issues/56

Symptom

newfs_msdos (src/msdosfs-1/msdos_newfs) fails on partition a of a disk whose NeXT label uses 1024-byte label sectors, the layout vm/ffsbench.py build_scratch and the installed root disk use. It prints a layout and exits 1 with EINVAL, leaving a half-written volume, and mount_msdos then mounts that volume without complaint:

# newfs_msdos /dev/rhd1a
newfs_msdos: /dev/rhd1a: Invalid argument
/dev/rhd1a: 1048208 sectors in 65513 FAT16 clusters (8192 bytes/cluster)
bps=512 spc=16 res=1 nft=2 rde=512 mid=0xf0 spf=256 spt=63 hds=255 hid=0 bsec=1048768
# echo $?
1
# mount_msdos /dev/hd1a /mnt/m
# echo $?
0

The same failure happens with an explicit size, newfs_msdos -s 524288 /dev/rhd1a, which is well inside the partition. On partition a of a disk with a 512-byte-sector label (the vm/ext2_guest.py wrap_volume layout), newfs_msdos -s 524288 /dev/rhd2a works.

Observations

Size: bsec=1048768 is the whole 512 MB device in 512-byte units, label area included. The partition is smaller. getdiskinfo() (newfs_msdos.c ~732) sizes the filesystem from DKIOCBLKSIZE/DKIOCNUMBLKS, which looks like it reports the whole device rather than the partition, so the layout can run past the partition's end.
Sector size: bps=512 comes from DKIOCBLKSIZE, while the disk's label uses 1024-byte sectors. Since #47's fix (4f3c58658), raw transfers that aren't a whole number of the device's sectors fail with EINVAL. The explicit -s 524288 case, which can't run past the end, still fails, which points at this.
Mount: mount_msdos accepts the half-made volume, with no check that the boot sector and FATs are complete.

Reproduce

i386 QEMU guest (v2 image), kernel with msdosfs (the issue-43 kernel was used), a scratch disk made by vm/ffsbench.py build_scratch as hd1, then newfs_msdos /dev/rhd1a. Logs: vm/work/fsrepro/res-mh.log, res-m2.log.

Where to look

newfs_msdos.c getdiskinfo(): take the size and sector size from the partition (the NeXT disk label for the a partition) rather than from DKIOCNUMBLKS/DKIOCBLKSIZE alone, or check that DKIOCNUMBLKS on /dev/rhdNa really is the partition size on this kernel.
Make every write a multiple of the sector size the device accepts. Report a failed write with the offset and length rather than a bare "Invalid argument".
mount_msdos/msdosfs_mountfs: reject a volume whose BPB doesn't fit the partition.

Found during the stress suite's filesystem run (branch stress-phase3). Related: #47, #54, #55.

Originally created by @raynorpat on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/RhapsodiOS/RhapsodiOS/issues/56 ## Symptom `newfs_msdos` (`src/msdosfs-1/msdos_newfs`) fails on partition `a` of a disk whose NeXT label uses 1024-byte label sectors, the layout `vm/ffsbench.py build_scratch` and the installed root disk use. It prints a layout and exits 1 with EINVAL, leaving a half-written volume, and `mount_msdos` then mounts that volume without complaint: ``` # newfs_msdos /dev/rhd1a newfs_msdos: /dev/rhd1a: Invalid argument /dev/rhd1a: 1048208 sectors in 65513 FAT16 clusters (8192 bytes/cluster) bps=512 spc=16 res=1 nft=2 rde=512 mid=0xf0 spf=256 spt=63 hds=255 hid=0 bsec=1048768 # echo $? 1 # mount_msdos /dev/hd1a /mnt/m # echo $? 0 ``` The same failure happens with an explicit size, `newfs_msdos -s 524288 /dev/rhd1a`, which is well inside the partition. On partition `a` of a disk with a 512-byte-sector label (the `vm/ext2_guest.py wrap_volume` layout), `newfs_msdos -s 524288 /dev/rhd2a` works. ## Observations - **Size:** `bsec=1048768` is the whole 512 MB device in 512-byte units, label area included. The partition is smaller. `getdiskinfo()` (`newfs_msdos.c` ~732) sizes the filesystem from `DKIOCBLKSIZE`/`DKIOCNUMBLKS`, which looks like it reports the whole device rather than the partition, so the layout can run past the partition's end. - **Sector size:** `bps=512` comes from `DKIOCBLKSIZE`, while the disk's label uses 1024-byte sectors. Since #47's fix (4f3c58658), raw transfers that aren't a whole number of the device's sectors fail with EINVAL. The explicit `-s 524288` case, which can't run past the end, still fails, which points at this. - **Mount:** `mount_msdos` accepts the half-made volume, with no check that the boot sector and FATs are complete. ## Reproduce i386 QEMU guest (v2 image), kernel with msdosfs (the issue-43 kernel was used), a scratch disk made by `vm/ffsbench.py build_scratch` as hd1, then `newfs_msdos /dev/rhd1a`. Logs: `vm/work/fsrepro/res-mh.log`, `res-m2.log`. ## Where to look - `newfs_msdos.c` `getdiskinfo()`: take the size and sector size from the partition (the NeXT disk label for the `a` partition) rather than from `DKIOCNUMBLKS`/`DKIOCBLKSIZE` alone, or check that `DKIOCNUMBLKS` on `/dev/rhdNa` really is the partition size on this kernel. - Make every write a multiple of the sector size the device accepts. Report a failed write with the offset and length rather than a bare "Invalid argument". - `mount_msdos`/`msdosfs_mountfs`: reject a volume whose BPB doesn't fit the partition. Found during the stress suite's filesystem run (branch `stress-phase3`). Related: #47, #54, #55.

gitea-mirror closed this issue

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

gitea-mirror commented

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

Author

Owner

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

Fixed by 84cd4eed3 (msdosfs: size newfs_msdos volumes from the labelled partition and write in its sector size), merged into ffs-perf in 56b7c5a31.

Root cause (probe on the guest): both guesses in the issue were real.

On a 1024-byte-label partition, DKIOCBLKSIZE/DKIOCNUMBLKS report the whole drive in 512-byte units (512 x 1048768), while DKIOCGPARTINFO reports the partition (1024 x 524288). The old code sized every volume from the drive, overrunning the partition on both disk layouts.
Raw I/O on that partition is only accepted at 1024-byte-aligned offsets on these kernels, so the old newfs failed with EINVAL on its second 512-byte sector (offset 512). That's why -s 524288 failed too.

The fix takes the sector size and size from DKIOCGPARTINFO first (1024 bps there, which is a valid FAT value), keeps the boot-sector signature and FSInfo fields at their fixed offsets inside the first 512 bytes, and reports a failed write's offset and length.

Verified on a guest: newfs with and without -s on both the 1024- and 512-byte-label disks, fsck_msdos clean; on the 512-label disk a full mount / write / remount / read-back cycle, fsck clean; -C images of 1440k, 32m and 512m and a plain file all fine, and a -C 32m image is byte-identical to the old binary's.

Mounting the new volume on a 1024-byte-label partition still fails. That's a separate kernel msdosfs block-unit bug, along with the missing BPB-vs-partition size check on mount, now #58.

@raynorpat commented on GitHub (Oct 16, 2026): Fixed by 84cd4eed3 (msdosfs: size newfs_msdos volumes from the labelled partition and write in its sector size), merged into ffs-perf in 56b7c5a31. Root cause (probe on the guest): both guesses in the issue were real. - On a 1024-byte-label partition, DKIOCBLKSIZE/DKIOCNUMBLKS report the whole drive in 512-byte units (512 x 1048768), while DKIOCGPARTINFO reports the partition (1024 x 524288). The old code sized every volume from the drive, overrunning the partition on both disk layouts. - Raw I/O on that partition is only accepted at 1024-byte-aligned offsets on these kernels, so the old newfs failed with EINVAL on its second 512-byte sector (offset 512). That's why `-s 524288` failed too. The fix takes the sector size and size from DKIOCGPARTINFO first (1024 bps there, which is a valid FAT value), keeps the boot-sector signature and FSInfo fields at their fixed offsets inside the first 512 bytes, and reports a failed write's offset and length. Verified on a guest: newfs with and without `-s` on both the 1024- and 512-byte-label disks, fsck_msdos clean; on the 512-label disk a full mount / write / remount / read-back cycle, fsck clean; `-C` images of 1440k, 32m and 512m and a plain file all fine, and a `-C 32m` image is byte-identical to the old binary's. Mounting the new volume on a 1024-byte-label partition still fails. That's a separate kernel msdosfs block-unit bug, along with the missing BPB-vs-partition size check on mount, now #58.