Skip to content

A sent handle of another kind is InvalidArgument whatever its rights, and adopted_services_stay checks only its own copy of toyos - #1280

Merged
Japabu merged 2 commits into
mainfrom
connect-refuses-another-kind-by-kind
Nov 12, 2026
Merged

Japabu merged 2 commits into
mainfrom
connect-refuses-another-kind-by-kind

Conversation

@Japabu

@Japabu Japabu commented Nov 12, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

#1252 landed with two of the shared testcases members it adapted not yet run. Once #1271 made them a guest row, testcases_members_exit_0 went red on both (run 43156966054, main 302fb36 plus #1227's SDK changes):

  • abuse_listener_hijack: connect through an acceptor answered PermissionDenied where the test, connect's ABI doc and the kernel's own lookup contract say InvalidArgument.
  • adopted_services_stay: the adopter exited 101.

Both are main's, from #1252.

Cause and resulting behaviour

The kernel's lookup of a handle a peer sent checked rights before kind. HandleTable::get_sent serves SYS_CONNECT's connector and SYS_SPAWN's place and job. It called get, which refuses a missing right before it looks at the kind. So a handle of another kind that also lacked the right answered PermissionDenied. An acceptor carries no WRITE, and #1252 made connecting need WRITE, which exposed it.

  • get_sent now checks the kind first: a handle of another kind is InvalidArgument whatever rights it carries. A right means something only on its own kind, and get_sent's contract, connect's ABI doc and the spawn fields' docs already promise InvalidArgument.
  • This also changes spawn place and job. A place or a job of another kind without WRITE now answers InvalidArgument, as their docs say, where it answered PermissionDenied.
  • get itself, whose wrong kind ends the caller, is unchanged.

The adopter read the services of another copy of toyos. std links its own copy of toyos, and each copy keeps its own adopted services. std's adopt_services filled std's copy. The adopter's toyos::endow::services() read its own copy, which adopted nothing, so its check that it held fs:/system after adopting went red: in run 43156966054 the adopter failed in assert_failed::<Vec<&str>, [&str; 1]>.

  • The adopter checks that it was endowed no service, through its own copy, which never adopts. The check after the adoption goes: the helper's spawn proves the adoption, since std reads the helper's image through /system, which std holds only by adopting it.
  • std's adopt_services says what ends its adoption: std's own resolution of a file or lookup of a service.
  • The two adopted sets are a defect of their own, the one A process holds its stdio stream state once, with its descriptor table #1226 closed for stdio's state. It is recorded in issues/a-programs-own-toyos-never-sees-the-services-std-adopted.md, whose exit is one adopted set both copies see. It is not fixed here.
  • This PR's first diagnosis, that the adopter's early listing refused its adoption, was wrong: review r1 found the failure in the later check. The history carries only this cause.

Record. The owed-on-metal entry for #1252 in issues/the-kernel-is-small-interrupts-post-and-threads-wait.md now says testcases_members_exit_0 runs the members #1252 adapted on QEMU; what metal owes is unchanged.

Checks and evidence

Evidence branch pr-1280, at 0ce94a131. Each log ends with its exit code; local paths are masked. The head is 8daef7f, on main 302fb36, in two commits: 71876b8, the kernel fix, and 8daef7f, the test, docs and records. Its tree, 7bc0336016d4f2cf9f3e78ca049677cb1d15cdb8, is bd1a044's, the head the guest run below was dispatched at; review r1's fixes were squashed into this history so that no commit states the first, wrong diagnosis. The host checks ran at 0df33fd, from which the head differs only in comments, docs, one guest test and records.

Host checks at 0df33fd, a clean tree (r14/):

  • --build-only;
  • the host suites of the kernel library with sched-check, toyos-abi, toyos and the other crates A child's handles are a list its parent names: labels and namespaces leave the kernel #1252 touched;
  • every kernel clippy shape of build/clippy.rs: host, host with the kernel's controls, x86_64 default, both actuator sets, the instrument set, the two heap-band sets, aarch64 default, both actuator sets and its instrument set;
  • clippy on toyos (all targets and fd-table), the libraries and toyos-abi.

All exit 0.

HandleTable has no host test: the kernel's handle table is not built for the host. The guest row is the test of the lookup, and abuse_listener_hijack holds connect's answer.

Guest rows, one run at bd1a044, whose tree is the head's: 43162379536, all 3 pass, and testcases says 242 members exited 0 (verdicts). 43160566562 at 0df33fd was cancelled while queued.

  • testcases_members_exit_0: every testcases member, abuse_listener_hijack and adopted_services_stay among them, and the four members A child's handles are a list its parent names: labels and namespaces leave the kernel #1252 adapted (poll_belongs_to_its_handle, messages_are_whole, moved_rights, mutual_kill);
  • a_launch_gives_each_unsent_stdio_slot_a_null and session_lifecycle: the supervisor's launches, which name a place and a job, whose lookup this changes. They run on x86-64 QEMU; each is one of MACHINE_TESTS.

Control: main 302fb36 without this change, the coordinator's run 43155677796 (testcases_members_exit_0, shared_debug_members_exit_0). It was cancelled before it ran, so no clean control of main exists yet. The reds this PR fixes are those of #1227's run 43156966054, at main 302fb36 plus #1227's SDK changes, which touch neither the kernel nor these two members' failing lines.

No metal is staged or requested: the benches are down, and what metal owes is in the record above.

Risks

  • A caller that told "no WRITE" from "no such kind" by the order now sees InvalidArgument for both on a wrong kind. The three callers are the ones named above, and their docs already say InvalidArgument.

🤖 Generated with Claude Code

@Japabu

Japabu commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator Author

Review r1 of #1280 at 0df33fd

Architecture. The kernel half is right and small. get_sent is the lookup for a handle whose kind is a peer's word. Checking kind before rights there gives one answer per defect: wrong kind is InvalidArgument, and the right kind without the right is PermissionDenied. get keeps rights first, which endowment_denied's SysCap policy relies on. It is correct for all three callers, verified by grep: sys_connect (kernel/src/syscall/ipc.rs:69), spawn_place (proc.rs:63) and spawn_job (proc.rs:77). Nothing leaks:

  • The manual (authority, Communication) and ReceivedEntry::kind already report every received handle's kind to its holder, so the new InvalidArgument reveals nothing the holder lacks.
  • It grants nothing. Before, a handle carrying WRITE already revealed whether it was a connector.

The ABI docs for connect, SpawnArgs::place and SpawnArgs::job now read the only consistent way. No userland code branches on the old order: the supervisor, toyos::spawn and std map no specific refusal. The one visible change is in endow::service, where a non-connector under a service label is now Refused(InvalidArgument) and was NotEndowed when it lacked WRITE. That is the honest answer to a parent's bug.

The test half rests on a wrong diagnosis and does not fix the red.

Findings

B1 tests/guest/rust-tests/src/bin/adopted_services_stay.rs:54-60: the cause is misdiagnosed, and the fix leaves the failing assertion in place.

  • Where the red run failed. In run 43156966054 the adopter (pid 52, code=101) failed in core::panicking::assert_failed::<alloc::vec::Vec<&str>, [&str; 1usize]>, frame 12 of its backtrace. That is the post-adoption assert_eq!(toyos::endow::services().collect::<Vec<_>>(), [SYSTEM], …). It is not the .expect("the adopter adopts /system"), which would fail in core::result::unwrap_failed. adopt_services was not refused: it succeeded.
  • Why. std links its own copy of toyos: sdk/toyos/src/lib.rs:341-343 says "std links a copy of its own", and std depends on it with rustc-dep-of-std. The test links another copy with std. Each copy has its own ADOPTED.
    • std's adopt_services fills std's copy.
    • toyos::endow::services() in the program reads the program's copy. The program was endowed no service and adopted nothing there, so it lists [].
    • The deleted first line sealed only the program's copy, which std's adoption never consults. It was harmless.
  • Consequence. At the head the adopter still lists [] against [SYSTEM] at line 60 and exits 101. I expect testcases_members_exit_0 in 43160566562 to stay red on test_rs_adopted_services_stay.
  • Fix.
    • Keep a "was endowed no service" check through the program's copy. services().count() == 0 is valid at any point, because that copy never adopts.
    • Drop the == [SYSTEM] check. The helper's spawn already proves the adoption: std reads the image through std's copy, which can resolve /system only through what it adopted.
    • Remove the line-54 comment. Its premise, that a lookup here would refuse the adoption, is false.
  • Records. Correct the commit message and the PR body's cause section.

B2 sdk/std/os/fs.rs:73 and sdk/toyos/src/endow.rs:263: both new doc lines come from the B1 misdiagnosis and mislead the caller who reads them.

  • std. "…or looked a service up, as listing them does" sends a std caller to listing, which std does not offer. The listing a program has is toyos::endow::services() in its own copy, and it does not end std's adoption.
  • toyos. "A lookup, as this listing is, ends what [adopt] may add" holds only within one copy. It repeats adopt's own contract ("Refused once this process has adopted any or has looked one up", line 302). A program reading it would wrongly think its listing affects std::os::toyos::fs::adopt_services.
  • Fix. Delete the toyos line. Restore the std line to what is true: std's own resolution of a file, or a service std itself connects to, ends it.

B3 tests/guest/rust-tests/src/bin/process_tree.rs:277: this change makes the comment stale. "A pipe's write end carries WRITE, so the kernel reaches the type it is not" explains the old rights-first order of get_sent, which spawn_place uses. The kernel now reaches the type whatever the handle carries. Drop the causal clause. This needs no rerun.

S1 kernel/src/object/handle.rs:44: WrongType's doc still says it is InvalidArgument "through get_sent". get_sent no longer builds this variant. Something like "Ends the caller like the two above; get_sent answers a wrong type InvalidArgument and never builds one" would match the code.

S2 (discovery; record or document, do not implement here) The two toyos copies hold separate adoptions. toyos::endow::connector/services ("then one it adopted") in a program never see what std adopted, and adopt's doc speaks of "this process". The supervisor avoids this because it keeps its own connectors. Anyone else reading through their own copy hits B1's trap. A one-line contract on adopt, or an issues/ entry, would prevent the next occurrence.

Evidence

  • Host checks: r14/ is at 0df33fd with a clean tree, and every log exits 0. The claim that HandleTable has no host test holds: the kernel's [[bin]] has test = false, and object/ is not part of pure/lib.rs.
  • Kernel change:
    • abuse_listener_hijack:80 covers kind-first.
    • endowment_denied::a_connector_without_write_connects_nowhere keeps the other half pinned: the right kind without WRITE is PermissionDenied.
    • Place and job share the same generic body. Inspection suffices there; process_tree's place refusals are metal-only.
  • Guest run 43160566562 and control 43155677796 are pending. The kernel half is expected green. For adopted_services_stay, see B1.

PR body (for the orchestrator)

The kernel section is accurate, including the three callers. The adopted_services_stay cause is false: "std's adopt_services was refused AlreadyExists and its .expect panicked" is wrong, and so is "the check after the adoption … already says it was endowed no service". That check is the one that failed. "The SDK is right and the test was wrong" is half right: the test is wrong, for the two-copy reason.

Net: kernel +6/−4, SDK docs +2/−1, test +3/−2, issue +4/−5.

SEND BACK

Japabu added a commit that referenced this pull request Nov 12, 2026
…toyos

Correction to the previous commit's message: the adopter's early listing
did not refuse its adoption. std links its own copy of `toyos`, and each
copy keeps its own adopted services. std's `adopt_services` filled std's
copy; the adopter's `toyos::endow::services()` read its own, which adopted
nothing, so the check that it held `fs:/system` after adopting went red
(run 43156966054, `assert_failed::<Vec<&str>, [&str; 1]>`), and the listing
before it had sealed only the copy std never consults.

- The adopter checks again that it was endowed no service, through its own
  copy, which never adopts. The check after the adoption goes: the helper's
  spawn proves the adoption, since std reads its image through `/system`,
  which std holds only by adopting it.
- The two doc lines the wrong diagnosis added go. std's `adopt_services`
  says what ends its adoption: std's own resolution of a file or lookup of
  a service.
- `process_tree`'s `a_pipe_is_no_place` no longer explains its refusal by
  the order the kernel no longer has, and `WrongType`'s doc says that
  `get_sent` never builds it.
- `issues/a-programs-own-toyos-never-sees-the-services-std-adopted.md`
  records the two adopted sets, the defect #1226 closed for stdio's state.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu Japabu changed the title A sent handle of another kind is InvalidArgument whatever its rights, and adopted_services_stay adopts before it looks a service up A sent handle of another kind is InvalidArgument whatever its rights, and adopted_services_stay checks only its own copy of toyos Nov 12, 2026
Japabu and others added 2 commits November 12, 2026 07:37
… rights

`HandleTable::get_sent`, the lookup of a handle a peer sent (`SYS_CONNECT`'s
connector, `SYS_SPAWN`'s place and job), checked the handle's rights before
its kind, so a handle of another kind that also lacked the right answered
`PermissionDenied`. Its own contract, `connect`'s and the spawn fields' ABI
docs all answer such a handle `InvalidArgument`, and a right means something
only on its own kind. `connect` through an acceptor, which carries no
`WRITE`, is the case `abuse_listener_hijack` holds, and it went red in
`testcases_members_exit_0` once #1252 made connecting need `WRITE`.

The kind is now checked first. A place or a job of another kind without
`WRITE` answers `InvalidArgument` too, as their docs already say.
`WrongType`'s doc says `get_sent` never builds it, and `process_tree`'s
`a_pipe_is_no_place` no longer explains its refusal by the old order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The adopter, which std's `adopt_services` gives `fs:/system`, then checked
that `toyos::endow::services()` listed it. std links its own copy of
`toyos`, and each copy keeps its own adopted services: std's adoption filled
std's copy, and the program's listing read its own, which adopted nothing.
So the check went red in `testcases_members_exit_0` (run 43156966054,
`assert_failed::<Vec<&str>, [&str; 1]>`).

- The adopter still checks, through its own copy, that it was endowed no
  service: that copy never adopts, so the check holds at any point. The
  check after the adoption goes. The helper's spawn proves the adoption,
  since std reads the helper's image through `/system`, which std holds
  only by adopting it.
- std's `adopt_services` says what ends its adoption: std's own resolution
  of a file or lookup of a service.
- `issues/a-programs-own-toyos-never-sees-the-services-std-adopted.md`
  records the two adopted sets, the defect #1226 closed for stdio's state.
- The owed-on-metal record for #1252 says `testcases_members_exit_0` runs
  the members #1252 adapted on QEMU.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu
Japabu force-pushed the connect-refuses-another-kind-by-kind branch from bd1a044 to 8daef7f Compare November 12, 2026 05:37
@Japabu

Japabu commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator Author

Review r2 of #1280 at bd1a044

Re-check of r1's findings. The tree at bd1a044 is 7bc033601; the reworded 0df33fd has not been pushed yet. Net change against main: +54/−17. The kernel half is unchanged since r1 and stays approved.

r1 findings

  • B1, resolved. adopted_services_stay.rs:54 checks "endowed no service" through the program's own copy. That check holds at any point, because that copy never adopts. The == [SYSTEM] check and its false comment are gone.
    • The new non-vacuity comment (lines 58-59) is true. std's spawn reads the helper's image with crate::sys::fs::read_image (sdk/std/sys/process.rs, the (None, None) arm) through std's own copy. The adopter was spawned services_only(&[]) and line 54 confirms it holds no service, so that copy can resolve /system only through what it adopted.
    • The helper's empty services() still exposes any adopted service a spawn would wrongly pass on: inheritable() passes only what was endowed.
  • B2, resolved. The toyos::endow::services line is deleted, so that file matches main. std's adopt_services doc (sdk/std/os/fs.rs:72-73) now names what ends its adoption: std's own file resolution, or std's own service lookup. Both seal std's copy. The doc says "looked up a service", which slightly overstates: a lookup that finds an endowed service does not seal. For the one intended caller, which is endowed none, it is exact.
  • B3, resolved. process_tree.rs:277 no longer explains the refusal by the old order.
  • S1, taken. handle.rs:44 matches the code: get_sent never builds WrongType.
  • S2, recorded. issues/a-programs-own-toyos-never-sees-the-services-std-adopted.md is well-formed under issues/README.md: open, defect, a date, an owner and an exit. I checked its claims: std_io's "std links a copy of its own"; adopt's "this process" (endow.rs:301-302); and A process holds its stdio stream state once, with its descriptor table #1226's stdio state, which lives in the fd-table copy behind toyos_stdio_* beside toyos_fd_* (sdk/toyos/src/log/stdio.rs, fd.rs). Its exit removes the second adopted set rather than documenting around it.

Evidence

Host checks r14/ ran at 0df33fd. That evidence still covers bd1a044: the delta changes comments, docs and the new issue file, adds one assertion that is identical to main's compiled line, and removes one assertion. Nothing it removed was the only use of an import. The guest test is compiled in run 43162379536, at bd1a044's tree. Status as of this review:

  • Run 43162379536, head: queued.
  • Control 43155677796, main: queued.
  • Run 43160566562, at 0df33fd: cancelled.

The coordinator judges these runs. I expect both members green at the head and both red in the control.

Records

  • bd1a044e3's message describes the true cause.
  • For the planned reword. The reword must leave the tree at 7bc0336016d4f2cf9f3e78ca049677cb1d15cdb8, so that 43162379536 still covers it. Once 0df33fd's own message states the true cause, the r1 fix commit's opening "Correction to the previous commit's message: …" no longer describes anything and should go in the same rebase.
  • PR body. The PR body becomes the merge commit's message. Its "Correction:" bullet cites 0df33fd by hash, so after the reword it needs the new hash, or it should go entirely. That is the orchestrator's correction. The rest of the body is accurate.

No new blocker.

LAND

@Japabu

Japabu commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator Author

Review r2 of #1280 at 8daef7f

This replaces the r2 comment I posted at bd1a044. The tree is unchanged, so that comment's findings carry over, and this one adds the rewritten history.

Tree. 8daef7f's tree is 7bc0336016d4f2cf9f3e78ca049677cb1d15cdb8, the same as bd1a044's (checked with git rev-parse). The r2 findings therefore stand without change:

  • B1, B2 and B3 are resolved.
  • S1 is taken, and S2 is recorded in issues/a-programs-own-toyos-never-sees-the-services-std-adopted.md, with its claims checked.
  • Run 43162379536 is at bd1a044, so it tests this tree. fix-hold-bd1a044e3 on the remote holds that commit until the run checks it out. Delete the branch once it has served.

History. There are two commits on 302fb36.

  • 71876b8 changes kernel/src/object/handle.rs (the get_sent reorder, its doc and WrongType's doc) and the process_tree.rs:277 comment. Its message is accurate: the three callers, the acceptor case, and the place/job consequence. It says nothing about the adopter. Both comments it changes are ones the reorder made stale, so they belong in this commit.
  • 8daef7f carries the test, std's doc, the new issue file and the A child's handles are a list its parent names: labels and namespaces leave the kernel #1252 record. Its message gives the true cause: two toyos copies, two adopted sets, and the failing assert_failed::<Vec<&str>, [&str; 1]> in run 43156966054. It correctly says the "endowed no service" check stays, since that line is main's and is unchanged here. No commit now states the first diagnosis.

PR body. The body is accurate: the head, both commits, the tree identity, the guest run's tree, and evidence reused from 0df33fd for a delta of comments, docs, one test and records. The note on the first diagnosis now refers to no hash, so it no longer goes stale.

No blocker.

LAND

@Japabu

Japabu commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator Author

Orchestrator verification at 8daef7f. Review r2 is LAND. Landing run 43162379536 at bd1a044, whose tree 7bc03360 is the head's, passed all 3 rows, testcases_members_exit_0 included, so both members now exit 0.

The control is 43156966054: main 302fb36 plus #1227's SDK-only changes, red on exactly these two members. Main's own control run is cancelled because it would only repeat that. The body is updated to match.

The PR merges cleanly onto main ee15a2e, since #1258 touches neither get_sent nor the adoption. Queueing, because this un-reds every other PR's testcases row.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant