writes hit the sector below) #42

New issue

Closed

opened 2026-11-03 22:03:05 -07:00 by gitea-mirror · 2 comments

gitea-mirror commented

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

Owner

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

Symptom

On the i386 IDE disk, a raw-device (/dev/rhdNx) read or write at an offset that is an odd multiple of 512 bytes silently uses the sector below it. The low 512 bytes of the offset are dropped. The device reports 512-byte sectors (DEV_BSIZE = 512, DKIOCBLKSIZE = 512), so 512-aligned I/O should work.

Reproduced with dd on the scratch disk after filling it with a sector-keyed pattern:

dd if=/dev/rhd1a bs=512  skip=3 count=1   # offset 1536: expected sector 3
dd if=/dev/rhd1a bs=1536 skip=1           # offset 1536: returns sectors 2, 3, 4

A read at byte 1536 returns the data that lives at byte 1024. Writes are misplaced the same way.

Found by the stress suite's new stress-disk (ffs-perf branch, src/Commands/stress/disk):

verify: with 512-byte-aligned offsets it FAILs every run, seeds 1, 2 and 3. It even FAILs on regions the writer never touched, because reads at odd-sector offsets come back from the wrong place.
rand: it passes only because its write and its read-back are misplaced identically.
Environment: the v2 image's kernel (master + ffs fixes), i386 under QEMU (PIIX IDE), hd1 a 1 GB scratch disk.

Impact

Anything doing raw I/O at odd-sector offsets reads or writes the wrong sector, silently. That covers fsck/newfs/dump-style tools, disk labelling, and databases using raw partitions. Writes land one sector early and overwrite data that belongs to the previous sector.

Where to look (unverified)

physio() (bsd/kern/kern_physio.c:156) computes bp->b_blkno = uio->uio_offset / blocksize using the blocksize its caller passes. If the i386 IDE disk's raw read/write entry passes 1024 instead of 512, the offset is rounded down to 1 KB. A logical, partition or fragment size, or a driverkit IODiskPartition/IOLogicalDisk block size, would do it.
The i386 ATA/IDE disk driver's raw entry points and how they call physio (compare the ppc ATADiskKernel.m:254/276 calls).
IODiskPartition / IOLogicalDisk in driverkit: how the logical block size relates to the physical one (IOLogicalDisk.m:715-716).

A fix should make raw transfers honour 512-byte offsets, or reject misaligned ones with EINVAL rather than silently rounding.

Regression test

On the dev guest:

STRESS_DISK_DEV=/dev/rhd1a make check-disk

Once the stress-disk safety fix round lands, verify there FAILs with off= until this is fixed.

Related: #41, #45, #46, also found by the stress suite.

Originally created by @raynorpat on GitHub (Oct 14, 2026). Original GitHub issue: https://github.com/RhapsodiOS/RhapsodiOS/issues/47 ## Symptom On the i386 IDE disk, a raw-device (`/dev/rhdNx`) read or write at an offset that is an odd multiple of 512 bytes silently uses the sector *below* it. The low 512 bytes of the offset are dropped. The device reports 512-byte sectors (`DEV_BSIZE` = 512, `DKIOCBLKSIZE` = 512), so 512-aligned I/O should work. Reproduced with `dd` on the scratch disk after filling it with a sector-keyed pattern: ``` dd if=/dev/rhd1a bs=512 skip=3 count=1 # offset 1536: expected sector 3 dd if=/dev/rhd1a bs=1536 skip=1 # offset 1536: returns sectors 2, 3, 4 ``` A read at byte 1536 returns the data that lives at byte 1024. Writes are misplaced the same way. Found by the stress suite's new `stress-disk` (ffs-perf branch, `src/Commands/stress/disk`): - **`verify`:** with 512-byte-aligned offsets it FAILs every run, seeds 1, 2 and 3. It even FAILs on regions the writer never touched, because reads at odd-sector offsets come back from the wrong place. - **`rand`:** it passes only because its write and its read-back are misplaced identically. - **Environment:** the v2 image's kernel (master + ffs fixes), i386 under QEMU (PIIX IDE), hd1 a 1 GB scratch disk. ## Impact Anything doing raw I/O at odd-sector offsets reads or writes the wrong sector, silently. That covers fsck/newfs/dump-style tools, disk labelling, and databases using raw partitions. Writes land one sector early and overwrite data that belongs to the previous sector. ## Where to look (unverified) - **`physio()` (`bsd/kern/kern_physio.c:156`)** computes `bp->b_blkno = uio->uio_offset / blocksize` using the `blocksize` its caller passes. If the i386 IDE disk's raw read/write entry passes 1024 instead of 512, the offset is rounded down to 1 KB. A logical, partition or fragment size, or a driverkit `IODiskPartition`/`IOLogicalDisk` block size, would do it. - **The i386 ATA/IDE disk driver's raw entry points** and how they call `physio` (compare the ppc `ATADiskKernel.m:254/276` calls). - **`IODiskPartition` / `IOLogicalDisk` in driverkit:** how the logical block size relates to the physical one (`IOLogicalDisk.m:715-716`). A fix should make raw transfers honour 512-byte offsets, or reject misaligned ones with EINVAL rather than silently rounding. ## Regression test On the dev guest: ``` STRESS_DISK_DEV=/dev/rhd1a make check-disk ``` Once the stress-disk safety fix round lands, `verify` there FAILs with `off=` until this is fixed. Related: #41, #45, #46, also found by the stress suite.

gitea-mirror

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

closed this issue
added the
bug
label

gitea-mirror commented

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

Author

Owner

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

Root cause: the raw read/write entry points passed [disk blockSize] to physio. For a labelled partition that is the label's logical block size (1024 on the scratch disk), even though DKIOCBLKSIZE reports 512. physio therefore floored the offset to 1 KB, and a read at byte 1536 started at 1024.

Fix, on branch issue-47 (commit 4f3c58658), not yet merged:

ata_hd_read/ata_hd_write call physio with DEV_BSIZE and return EINVAL for offsets that are not a multiple of 512.
ata_hd_strategy turns physio buffers (B_PHYS|B_RAW) into a byte offset and calls a new IOLogicalDisk asyncTransfer:byteOffset:.... It reblocks when the request is not aligned to the physical block size.
MAX_REBLOCK_SIZE is raised from 2048 to 4096, so 4K sectors fit.

Tested on i386 QEMU with a snapshot of the v2 image and a scratch hd1: the stock kernel reproduced the bug (reads at skip=0..7 gave S1,S1,S3,S3,S5,S5,S7,S7). With the fix, reads and writes at odd-sector offsets hit the right sectors and the full image compared identical. newfs, fsck, mount and file copies on hd1a were clean, and the machine booted to multi-user.

Not tested: the EINVAL check was added after the last kernel build, so it is uncompiled. The cluster-I/O path on a 1024-label disk, ppc, and an actual 4K disk are also untested.

@raynorpat commented on GitHub (Oct 14, 2026): Root cause: the raw read/write entry points passed `[disk blockSize]` to `physio`. For a labelled partition that is the label's logical block size (1024 on the scratch disk), even though `DKIOCBLKSIZE` reports 512. `physio` therefore floored the offset to 1 KB, and a read at byte 1536 started at 1024. Fix, on branch `issue-47` (commit 4f3c58658), not yet merged: - `ata_hd_read`/`ata_hd_write` call `physio` with `DEV_BSIZE` and return EINVAL for offsets that are not a multiple of 512. - `ata_hd_strategy` turns `physio` buffers (`B_PHYS|B_RAW`) into a byte offset and calls a new `IOLogicalDisk asyncTransfer:byteOffset:...`. It reblocks when the request is not aligned to the physical block size. - `MAX_REBLOCK_SIZE` is raised from 2048 to 4096, so 4K sectors fit. Tested on i386 QEMU with a snapshot of the v2 image and a scratch hd1: the stock kernel reproduced the bug (reads at `skip=0..7` gave S1,S1,S3,S3,S5,S5,S7,S7). With the fix, reads and writes at odd-sector offsets hit the right sectors and the full image compared identical. `newfs`, `fsck`, mount and file copies on hd1a were clean, and the machine booted to multi-user. Not tested: the EINVAL check was added after the last kernel build, so it is uncompiled. The cluster-I/O path on a 1024-label disk, ppc, and an actual 4K disk are also untested.

gitea-mirror commented

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

Author

Owner

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

Fixed by 4f3c58658 (kernel: honour 512-byte raw disk offsets on i386 IDE and reject sub-sector ones with EINVAL), merged into the ffs-perf branch in b6820ec8f. It reaches master when ffs-perf is merged.

@raynorpat commented on GitHub (Oct 15, 2026): Fixed by 4f3c58658 (kernel: honour 512-byte raw disk offsets on i386 IDE and reject sub-sector ones with EINVAL), merged into the ffs-perf branch in b6820ec8f. It reaches master when ffs-perf is merged.

gitea-mirror referenced this issue

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

[GH-ISSUE #68] fork: a child forked from a cthreads process intermittently dies with SIGSEGV (stress-thread fork) #63