0.7.0 — the crash a consumer could not localise, and openkal 0.9 - #18
Merged
Conversation
⚠️ ⚠️ `signal(SIGABRT, …)` DESTROYED ITS CALLER'S RETURN ADDRESS. `SYS_rt_sigprocmask` cleared `sizeof(sigset_t)` --- 128 bytes --- into whatever the caller passed, while `a4` is the SIGSETSIZE the caller declared, which is eight on this architecture and is what the kernel writes. Sixteen of musl's seventeen callers pass a 128-byte `sigset_t` and saw nothing. The seventeenth is `src/signal/sigaction.c`, which declares one word and reaches this only when the signal is SIGABRT; that local sits at -0x20 in a frame of 0x30, so 120 bytes past it lay the saved frame pointer and the return address, and `__sigaction` returned to zero. ⭐ The whole of the reproduction is three lines: int main(void) { signal(SIGABRT, h); return 0; } SIGSEGV, instruction pointer at zero, stack top zero rather than a return address. Reported in openkal-linux#13 with a dozen forks and four threads in front of it, because that is what the program that met it was doing; none of it was necessary. Both earlier exclusions in that report were correct.⚠️ AND IT WAS NOT ONLY INSTALLING A HANDLER. `signal(SIGABRT, SIG_IGN)` and a plain enquiry, `sigaction(SIGABRT, NULL, &old)`, died the same way --- so a program that only READ what SIGABRT was set to could not survive doing so, which a terminal library, a test framework's death tests and any crash reporter all do before they do anything else. `SYS_rt_sigaction` had the same defect reversed: it cleared the size of a structure declared here, three fields, where `struct k_sigaction` is four, so eight bytes of the caller's structure were left holding the stack and were copied out to the program. ⭐ EACH IS THE SAME MISTAKE: A SIZE TAKEN FROM THE TYPE AT HAND RATHER THAN FROM THE CONTRACT BEING ANSWERED. `examples/posix` now touches every signal's disposition in all three forms --- enquiry, ignore, handler. It had thirty-six observations, three about `abort`, and no call to `signal` anywhere: it examined whether `abort` ends the program and never whether a program may ask what a signal is set to. --- and four more the same consumer would have met ------------------------ * `getpgrp()` answered -38. There was no case for the number, and musl's `getpgrp` deliberately does not pass the result through `__syscall_ret` because POSIX says the call cannot fail --- so a negated error was handed over as a process group, with no errno and nothing to check. * `st_dev` and `st_ino` were constants, so every file was the same file: `std::filesystem::equivalent` on two separately created files answered TRUE with no error. openkal 0.9 carries an identity and this reports it, and reports nothing rather than a constant where the implementation has none. * `stat` and `lstat` were one call. The flag was ignored while `open` resolved, so a program was told a name referred to a link when opening it would have reached a file --- and one such node made a whole tree uncopyable for the C++ library above. * `symlink` and `readlink` answer, over `openkal.fs`'s new operations, after asking `kal_fs_props` whether the volume has such nodes. * `SYS_membarrier` has a case of its own so the trace does not report it. It comes from musl's own `pthread_create`, and its result is assigned to nothing --- one of the six numbers the first user of that switch reported was a false alarm, and the cost of that fell on them. * The started-program bound is stated in the README and raised to 256. The sixty-fifth `posix_spawn` of a program that waited for none reported EAGAIN, which is what POSIX says --- what was wrong is that the number was invisible, so the failure landed on an operation with no evident relation to the ones that caused it. --- openkal 0.9 ------------------------------------------------------------ Transfers return one signed word, which is what every site here computed from the earlier two-word result by hand. Values are copied into this library's own buffers rather than pointed at. `kal_node_info` states its size and reports what was filled. The page size is asked for rather than fixed at build time --- it was 4096 in two places, and is what this library reports as `_SC_PAGESIZE`, rounds mappings to, and reports as `st_blksize`. ⭐ And the one weak reference that needed a different spelling no longer does: `kal_process_props` was an object, so testing it the way every other weak reference is tested WAS the null dereference the test existed to prevent. Every report is an operation now. Built and verified for x86_64-linux-musl and aarch64-linux-musl.
Every README here opens by showing what a program writes in its manifest, which is the first thing a reader copies and the last thing anyone edits. These lines had drifted --- the specification's own README asked for a version four minor releases old --- and nothing checked them. `openkal/tools/check-readme-versions.sh` now does.
…unremovable openkal states that opening resolves and offers no form that declines to -- deliberately, since a program that opens a link to read its bytes is asking what kal_fs_link_read answers. So do_openat resolved, and for a link whose target is absent it answered ENOENT. That is a different answer to a different question. O_NOFOLLOW does not ask to open the link and does not ask to open its target: it asks whether the name is a link, and POSIX says ELOOP when it is. Nothing nearby looked wrong. Every operation this port offers still held -- symlink, readlink, stat, lstat, unlink, getdents all behaved as the host does. What failed was three layers up: libc++'s remove_all descends by opening each entry O_DIRECTORY|O_NOFOLLOW and reads ENOENT as "the entry is already gone", so it unlinked nothing and then reported ENOTEMPTY for a directory it had just declined to empty. The host toolchain removed the same tree. Measured on a tree holding a dangling link, a live link and a link to a directory: 6 removed on both, none left behind. The answer comes from the enquiry openkal 0.9 added -- ask about the name itself -- on a path taken only when the caller passed the flag. Also here: the traceprobe step asserted that an absent operation reports itself by naming symlinkat, which this port now implements. It reported zero diagnostics for a hundred attempts, which is the same reading a broken diagnostic channel gives. It now names mknodat and asserts that the dispatcher does not handle it, so the day that changes this step says so instead of passing while measuring nothing. And openkal-windows was pinned at 0.3.0 here against a package now at 0.4.0.
okm_opt.h exists so that the decision "is this interface present?" is made once
rather than at forty call sites, and its opening comment says why: "a
forty-first added later would be the one that was missed -- and missed
silently, because the way it shows is a link failure on a target nobody was
building at the time."
do_readlink and do_symlink were the forty-first. They named kal_fs_link_* di-
rectly and tested the weak symbol themselves, which is correct where openkal.fs
is present and is not a declaration at all where it is absent: the weak
declarations live inside that branch, so in the OKM_HAS_FS == 0 configuration
the names came from <openkal/fs.h> strong, and a backend with no filesystem
failed to link.
ld.lld: error: undefined symbol: kal_fs_link_read
>>> referenced by okm_syscall.c:340 ... (do_readlink)
Found by openkal-opensbi's bare-metal row, which is the only row that builds
that configuration. The null test now lives in the seam, once, and the callers
name okm_fs_link_*. Verified by building examples/same-source for
riscv64-none-elf over openkal-opensbi.
Also: the identity assertion in the posix probe now says where to look when it
fails. This port puts zero in st_dev/st_ino when the implementation reports no
identity, so every node compares equal to every other -- and the defect is in
that implementation, not here. It was measured against openkal-windows.
…page size They are different quantities wearing the same name. openkal's granularity is the coarsest quantum a caller must respect, and an implementation for a machine with no memory management unit answers ONE -- correctly: there is no page, and nothing needs rounding. openkal-opensbi does. libc.page_size is what this library rounds heap growth to, reports as sysconf(_SC_PAGESIZE) and as st_blksize, and whose arithmetic its allocator assumes is a power of two no smaller than its own quantum. Given one, the allocator asked the environment for one-byte extents and the program stopped inside the first allocation large enough to need a new one. Measured, and only on the machine that answers that way. Over openkal-opensbi the same-source example printed three of its four lines and stopped: containers, exceptions and unwinding all held, and the fourth line was the first to format a string. Over openkal-linux, whose answer is 4096, nothing was wrong. Confirmed by putting the defect back and running it again -- the fourth line disappears and returns with the fix. The probe's own assertion did not catch it and could not have: it said "a positive power of two", and one is both. It now says what the allocator requires, and asks for several pages in one allocation and writes every byte of them -- which is the property that actually broke.
A property of the arrangement rather than an omission, and one a reader meets: openkal offers no opening that declines to resolve, so this port asks first and reports ELOOP. Answering ENOENT is a different answer to a different question, and libc++'s remove_all reads it as "the entry has already gone".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
signal(SIGABRT, …)destroyed its caller's return addressSYS_rt_sigprocmaskclearedsizeof(sigset_t)— 128 bytes — into whatever thecaller passed, while
a4is the sigsetsize the caller declared, which iseight here and is what the kernel writes.
Sixteen of musl's seventeen callers pass a 128-byte
sigset_tand saw nothing.The seventeenth is
src/signal/sigaction.c, which declares one word and reachesthis only when the signal is
SIGABRT; that local sits at-0x20in a frame of0x30, so 120 bytes past it lay the saved frame pointer and the return address,and
__sigactionreturned to zero.⭐ The whole of the reproduction is three lines:
SIGSEGV, instruction pointer at zero, stack top zero rather than a return
address. Reported in
mcpplibs/openkal-linux#13with a dozen forks and fourthreads in front of it, because that is what the program that met it was doing;
none of it was necessary. Both exclusions in that report were correct.
signal(SIGABRT, SIG_IGN)and aplain enquiry,
sigaction(SIGABRT, NULL, &old), died the same way — so a programthat only read what SIGABRT was set to could not survive doing so, which a
terminal library, a test framework's death tests and any crash reporter all do
before they do anything else.
SYS_rt_sigactionhad the same defect reversed: it cleared the size of astructure declared here, three fields, where
struct k_sigactionis four.⭐ Each is the same mistake: a size taken from the type at hand rather than from
the contract being answered.
examples/posixnow touches every signal'sdisposition in all three forms. It had thirty-six observations, three about
abort, and no call tosignalanywhere: it examined whetherabortends theprogram and never whether a program may ask what a signal is set to.
Four more the same consumer would have met
getpgrp()answered −38. musl deliberately does not pass the result through__syscall_retbecause POSIX says the call cannot fail — so a negated error washanded over as a process group, with no errno and nothing to check.
st_devandst_inowere constants, so every file was the same file:std::filesystem::equivalenton two separately created files answered truewith no error.
statandlstatwere one call. The flag was ignored whileopenresolved,so a program was told a name referred to a link when opening it would have
reached a file — and one such node made a whole tree uncopyable for the C++
library above.
symlinkandreadlinkanswer, after askingkal_fs_propswhether thevolume has such nodes.
SYS_membarrierhas a case of its own so the trace does not report it: it comesfrom musl's own
pthread_createand its result is assigned to nothing. One ofthe six numbers the first user of that switch reported was a false alarm.
openkal 0.9
Transfers return one signed word — which is what every site here computed from the
two-word result by hand. Values are copied into this library's own buffers. The
page size is asked for rather than fixed at build time: it was 4096 in two places,
and is what this library reports as
_SC_PAGESIZE, rounds mappings to, andreports as
st_blksize.⭐ And the one weak reference that needed a different spelling no longer does:
kal_process_propswas an object, so testing it the way every other weakreference is tested was the null dereference the test existed to prevent.
Built and verified for x86_64-linux-musl and aarch64-linux-musl. The
std::filesystembehaviour now matches the host's own toolchain row for row.