MentOS Files

20 commits since v0.9.6, closing the 20 issues of the v0.9.7 milestone.

The theme of this release is failures that were reported as success. Eleven of the twenty issues are a function that could not tell its caller something had gone wrong: void where an error was needed, -1 standing for three different causes, or an errno overwritten one line after being set correctly. Where a caller cannot distinguish failure from success it will act on the failure, and the result is silent: wrong data, leaked space, or a file that was never written.

The block layer, and why it comes first

ata_device_read_sector was void. On failure it logged and returned, and the PIO read jumps out before its inportsw on every error path, so the caller's buffer kept whatever it already held — for two paths in ata_read that is the previous sector, handed back as though it were the one requested (#291).

Making it propagate found the bug live on the third suite run:

[ATA   ] ata_device_read_sector: PIO failed (sector 44, rc=-16)
[ATA   ] ata_read: failed to read sector 44 (-16)
[EXT2  ] Failed to read inode table block (inode 2, block 5).
[EXT2  ] Failed to read the inode (2).
[t_dir_entries] 397 of the 400 files created can be found by name

A transient -EBUSY on a sector of the inode table. Before the fix, the same event handed ext2 a stale block as the root inode and nothing above could tell. Because the condition is a status race rather than a bad sector, it is now retried, with the recovery logged; a persistent failure propagates. Both transfer functions are warn_unused_result, so dropping the error is a build failure.

Every filesystem fix below sat on top of that.

Filesystem

• A truncating open wrote kernel heap bytes into the file, readable by any program that could read the file back: the write length was computed as an offset, so every iteration after the first asked for more bytes than the one-block buffer held (#315).
• O_TRUNC zeroed a file's contents but never set its size to zero, so stat reported the old length, reading returned that many zero bytes instead of end-of-file, and the blocks were never released (#320).
• rmdir removed a directory that still held dot-prefixed files, leaving them with no path and inodes nothing would ever free. The check for . and .. compared one character, so .bashrc and everything like it was invisible to it (#341).
• write() reported success when it could not allocate a block, so a full filesystem discarded data silently (#303).
• A directory needing a second block replaced its first one, losing every entry it held (#309).
• The directory-entry iterator looped forever on a zeroed directory block, so one rmdir on a corrupt directory hung the kernel (#304).
• Unlinking a large file leaked its indirect index blocks (#302).
• A write ending exactly on a block boundary allocated and wrote one block more than it needed, permanently attributed to the file (#311).
• ext2_creat returned NULL without touching errno and left the inode it had allocated marked in use, and leaked the parent reference on every success (#305, [#323]).
• On-disk names of exactly 255 characters were copied into 255-byte fields with no room for a terminator (#285).

Kernel, types and VFS

• int64_t and uint64_t were 32 bits wide. Casts made specifically to gain range gained nothing and could not be warned about, which left the ELF bounds checks unable to detect the overflow they were written for, truncated ext2 offsets past 4 GiB, and made the ATA IDENTIFY structure 508 bytes instead of 512 — so the driver read 254 of the drive's 256 words (#270). Every width in stdint.h is now checked at compile time.
• A mount point claimed the paths just outside it: the longest-prefix match compared only the prefix, so open("/dev/nullx") succeeded as the null device and /procx was handed to procfs (#289).
• vfs_creat overwrote whatever the filesystem had reported with ENOENT, so a full disk, a read-only device and a request to create the root directory were all reported as "no such file or directory" (#289).
• A process whose SIGSEGV handler returned got one kernel error line per delivery — 341722 of 341824 lines in a minute — starving every other process rather than only itself (#296). The fault loop itself matches Linux and is not a defect.
• getcwd reported failure with (char *)-1 instead of NULL, which made every == NULL check already written in the tree dead code (#231).
• /tmp and /var/tmp were created world-writable without their sticky bit, so any user could delete another user's files there; /root stayed at 0755 instead of 0700 (#263).

Structure and tooling

• The ext2 driver is now one translation unit per concern: ten units and a private header, 220 to 690 lines each, replacing a single 4412-line file. Every function body was verified byte-identical to the one it replaced (#317). Every symbol it exports carries an ext2_ prefix, and inside the module a leading __ now means static without exception (#319).
• scripts/run-qemu-test refuses to boot an image older than what it should contain, naming the offending file. Running the wrapper by hand had produced a silently passing verification three times, which is worse than a failing one because it is believed (#289).
• t_fhs treats a directory-mode mismatch as a failure rather than a warning. It counted one as a pass "since the directory exists", which is how three wrong modes shipped under a green suite (#263).
• ext2_clean_inode_content and the dead CREAT_LAST_COMPONENT flag are gone (#327, [#289]).

Verification

68 tests, 0 failures, 0 panics — three consecutive Debug runs and one Release run on the tagged tree. Release was checked deliberately, because [#270] changed integer widths and [#285] had recorded a Debug/Release divergence at the same boundary.

Every fix carries a regression test and a negative control: the fix reverted, the specific test shown failing with its exact output, then restored. The suite went from 59 registered tests to 68.

Known issues

Not regressions, and each has a reproduction in its issue.

• [#191] syscalls do not validate user pointers, with [#259] as the concrete instance in sys_uname. A deliberate state of a teaching kernel, but not a property to assume away.
• [#344], [#342] and [#343] were filed after the milestone closed and are not fixed here. Several ext2 read-modify-write sequences discard the read result, so a failed read writes a zeroed metadata block back — the inode table being the worst case. They only became visible because [#291] made a failed sector read reportable at all.
• [#338] there is no way to make a sector read fail on demand, so the storage error paths, including several fixed in this release, have no test that reaches them.
• [#336] intermittent single-test failures whose likely cause was [#291]. The mechanism fits and the suite has been green since, which is an explanation rather than a proof.
• [#346] creat on an existing directory returns a descriptor for it instead of EISDIR.
• [#332] vsprintf has no length modifiers, so %lld prints literally and consumes no argument; [#333] the ATA 48-bit offset is truncated by every caller; [#335] four getcwd callers ignore the return value.

Corrections made during this cycle

Three claims of mine turned out to be wrong and were corrected on their issues rather than quietly dropped:

• #285 claimed a kernel stack corruption reachable from user space. Measuring the struct as declared showed the out-of-bounds byte lands in the object's own trailing padding, and the [#284] work had already closed the user-space route.
• #289 item 4 I had called harmless with today's mounts. It was not: open("/dev/nullx") succeeded and returned the null device.
• #296 I had reported the fault loop as the defect. It matches Linux; the defect was the per-delivery log line, and the issue was narrowed to it.

Note on test coverage

t_nospace is registered but kept out of all_tests[]: it fills the image to force an allocation failure, which costs about two minutes of guest time. Run it by hand when touching the ext2 allocation paths. t_big_write remains skipped as before.