Fix SkHermesMessage payload-aliasing defect (task 3.8 findings log)

sk_hermes_send_one() -- the single funnel every sender, including
sk_hermes_publish(), already goes through -- used to store the
caller's own payload_addr pointer as-is. Two sends before either
drains meant both messages pointed at the same caller-owned buffer,
whichever send wrote last silently winning: real, confirmed live (a
second identity thumbdrive attached at boot alongside Zuse's own left
its WIREBIND pairing silently never happening).

SkHermesMessage gains an inline payload_buf[SK_HERMES_CHUNK_MAX_
PAYLOAD] field; sk_hermes_send_one() now memcpy()s the caller's
payload into it and points payload_addr at that copy instead. No
sender or reader call site needed to change -- every existing reader
already only ever reads through payload_addr, which still points at
valid bytes of the same length, now message-owned. g_kh_blk_attach_buf/
g_kh_console_cmd_buf (repl.c, tasks 3.8/3.9) no longer need to survive
past their own send call; their doc comments, which had claimed the
old aliasing shape was benign, are corrected.

Verified the original bug is actually gone: reproduced the exact
original scenario (a real, sequentially-minted rajames identity
attached at boot alongside Zuse's own, two simultaneous BLK-ATTACH-
EVENT sends in one idle-loop pass) -- WIREBIND pairing, USE, and the
Stage D relay all now work where WIREBIND previously silently failed.
Standard three-ISA acceptance also clean: zero UNKNOWN WORD, dict_hash
identical across all three and matching every prior acceptance run in
this document.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Robert Allan James
2026-09-22 14:16:29 -04:00
co-authored by Claude Sonnet 5
parent 21734b1a52
commit 5c07745c18
14 changed files with 46639 additions and 35 deletions
+37
View File
@@ -1323,6 +1323,43 @@ kept as the reproduction — a second identity drive attached at boot is what ex
default single-drive boot this whole document's other acceptance runs used never will. Task 3.9
is paused pending Captain Bob's decision on when this gets fixed.
**2026-09-22 — CLOSED.** Captain Bob's own instruction, after 3.9/prompt-format both landed:
"finish the work" before cleanup/3.10. Fixed exactly as scoped above — `SkHermesMessage`
(`kernel_hermes.h`) gained an inline `payload_buf[SK_HERMES_CHUNK_MAX_PAYLOAD]` field;
`sk_hermes_send_one()` (`kernel_hermes.c`, the single funnel every sender/`sk_hermes_publish()`
already goes through) now `memcpy()`s the caller's payload into it and points `payload_addr` at
that copy, instead of storing the caller's own pointer. No sender or reader call site needed to
change — every existing reader (`sk_hermes_drain_checkpoint()`, `sk_hermes_reassemble()`) already
only ever reads through `payload_addr`, which still points at valid bytes of the same length,
now message-owned. `g_kh_blk_attach_buf`/`g_kh_console_cmd_buf` (`repl.c`, tasks 3.8/3.9) no
longer need to survive past their own send call — their doc comments corrected accordingly;
left `static` as a harmless convenience, not changed to stack-local (out of this fix's own
scope).
**Verified the original bug is actually gone, not just no-longer-observed.** Reproduced the exact
original scenario — `S" Captain Bob"...MINT`'d a real `rajames` identity sequentially first
(so both devices are genuinely eligible, not one blank/one real), then rebooted with `rajames`'s
now-minted drive attached at launch alongside Zuse's own (`QEMU_EXTRA`, port 2), reproducing two
simultaneous `BLK-ATTACH-EVENT` sends in one idle-loop pass before either drains
(`logs/20260922-140405/amd64/`: `ledger held=4096`, the same two-admission signature the original
bug's own log showed). **Before this fix, that scenario left `rajames`'s own WIREBIND pairing
silently never happening** (`FABRIC-3.6.md`'s own earlier finding: `S" bob" USE` returned `USE:
bob not found`). **After this fix, in the identical scenario, `WIREBIND: rajames attached and
ready` fires, `USE rajames` succeeds, and the Stage D relay still works on top of it** — confirmed
live, interactively, in that same log. `dict_hash` for Artemis differs from this document's other
acceptance runs in that one log only, expected and already-documented (task 0.0's own finding:
`disk/artemis.img` was deliberately left unreset across the two boots this reproduction needed,
so Artemis took the `resuming` branch, not `blank -- formatting`) — not a cross-architecture
divergence, since this reproduction was amd64-only by design.
Standard three-ISA acceptance (single-drive boot, disks reset before each arch, matching every
other task in this document): `logs/20260922-140634/amd64/`, `logs/20260922-140908/aarch64/`,
`logs/20260922-141236/riscv64/` — all three reach `[zuse@Hera] ok>`, zero `UNKNOWN WORD`,
`dict_hash` identical across all three and matching every prior acceptance run in this document
exactly (unaffected — the fix changes message storage, not dictionary content). Both `USE
rajames` (task 3.9's earlier finding that the documented `FABRIC-3.md §XXX` USE/BINDSTEP crash
did not reproduce, confirmed again here) and the Stage D relay verified working on all three.
**2026-09-22, out-of-band request during task 3.9's own verification — console prompt format
changed on Captain Bob's direct instruction, not a reshuffle task, no task number.** The bracket
line prefix (`console.c`'s `emit_prefix()`, unchanged) used to read
+48 -9
View File
@@ -73,6 +73,13 @@
#define SK_HERMES_MSG_MAX 32
#define SK_HERMES_MEMBER_MAX 64
/* SK_HERMES_CHUNK_MAX_PAYLOAD - one block (1024 bytes, 64x16, SXLIV.1),
* moved up here (was originally defined much further down, next to
* SkHermesChunkHeader) so SkHermesMessage's own payload_buf field below
* can be sized against it. See this file's chunking section, further
* down, for the full sizing rationale -- not restated here. */
#define SK_HERMES_CHUNK_MAX_PAYLOAD 1024
/*
* SK_HERMES_Q_SLOT - per-message admission heat, task 2.2 (item 28).
* messaging.4th:44-46 derives Q.SLOT as the reservoir remaining after
@@ -98,11 +105,35 @@
* reserved sentinels MSG-NACKED(253)/MSG-DELIVERED(255)).
* @field from Sending VM (mirrors MSG-FROM@).
* @field to Target VM (mirrors MSG-TO@).
* @field payload_addr Out-of-line payload address (mirrors MSG-PADDR@).
* @field payload_addr Payload address a reader dereferences (mirrors
* MSG-PADDR@). CORRECTED 2026-09-22 (real defect,
* FABRIC-3.6.md task 3.8's own findings log, "a real
* payload-aliasing defect"): this used to be the
* caller's own out-of-line pointer, stored as-is --
* two sends before either drained meant both
* messages pointed at the same caller-owned buffer,
* whichever send wrote last silently winning. Now
* always points at this SAME message's own
* `payload_buf` below, which sk_hermes_send_one()
* copies into at send time -- every message owns its
* payload bytes, independent of whatever the caller
* does with its own buffer afterward. Every existing
* reader (sk_hermes_drain_checkpoint(),
* sk_hermes_reassemble()) is unaffected: `payload_addr`
* still points at valid payload bytes of the same
* length, just message-owned instead of caller-owned.
* FABRIC-3.5.md SXLIII.6.1 (item 44, still open): a
* payload above INPUT_BUFFER_SIZE-1 (1024) bytes cannot
* be drained in one interpret call -- bound or chunk it
* before Phase 3, not here.
* @field payload_buf Backing storage for `payload_addr` above -- see its
* own doc comment. Sized to SK_HERMES_CHUNK_MAX_PAYLOAD,
* the uniform bound sk_hermes_send_one()/
* sk_hermes_publish() already enforce on `payload_len`
* (defined just above, moved up in this same fix so it
* is in scope here). No reader should touch this field
* directly -- go through `payload_addr`, which is what
* every existing reader already does.
* @field payload_len Payload length in bytes (mirrors MSG-PLEN@).
* @field stadium_cell Index into the Stadium cell array this message's
* heat currently occupies, or a sentinel meaning "none"
@@ -126,6 +157,7 @@ typedef struct {
VMUuid from;
VMUuid to;
void *payload_addr;
uint8_t payload_buf[SK_HERMES_CHUNK_MAX_PAYLOAD];
uint32_t payload_len;
int32_t stadium_cell;
uint32_t seq;
@@ -411,8 +443,12 @@ int sk_hermes_queues_boot_init(void);
* response encode context (e.g. task 3.6's GRANT
* uses this to tell the requester which private
* channel was just created).
* @param payload_addr Out-of-line payload address, passed through
* unchanged.
* @param payload_addr Caller-owned buffer, read and copied into the
* message's own storage before this function
* returns (2026-09-22 fix, this struct's own
* payload_addr field doc comment has the full
* account) -- the caller does not need to keep
* it alive past this call.
* @param payload_len Refused (-1) if it exceeds
* SK_HERMES_CHUNK_MAX_PAYLOAD.
* @return 0 on success, -1 on any refusal (bound, reservoir, arena, or
@@ -450,8 +486,10 @@ int sk_hermes_send_one(VMUuid from, VMUuid to, uint32_t type, uint32_t channel_i
* channel from sk_hermes_channel_create()). Refused
* (-1) if invalid/not in use.
* @param type Message type code, passed through unchanged.
* @param payload_addr Out-of-line payload address, passed through
* unchanged.
* @param payload_addr Caller-owned buffer, read once per subscriber via
* sk_hermes_send_one() (its own doc comment has the
* full account) -- the caller does not need to keep
* it alive past this call.
* @param payload_len Payload length in bytes. Refused (-1, no
* allocation attempted) if it exceeds
* SK_HERMES_CHUNK_MAX_PAYLOAD (task 3.5, added here
@@ -585,10 +623,11 @@ int sk_hermes_drain_checkpoint(VM *vm);
* (3.8+, the same boundary chunk-aware drain is already deferred past).
*/
/* One block = 1024 bytes (64x16, SXLIV.1) -- the uniform bound every
* message's payload_len must satisfy, chunked or not (see this
* section's own sizing note above). */
#define SK_HERMES_CHUNK_MAX_PAYLOAD 1024
/* SK_HERMES_CHUNK_MAX_PAYLOAD (1024 bytes, 64x16, SXLIV.1) -- the
* uniform bound every message's payload_len must satisfy, chunked or
* not (see this section's own sizing note above) -- is defined earlier
* in this file now, next to SkHermesMessage's own payload_buf field,
* which needs it in scope. Not redefined here. */
/* SkHermesChunkHeader - prepended to a chunk carrier's own payload,
* ahead of up to SK_HERMES_CHUNK_MAX_SLICE bytes of real content.
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
+42 -25
View File
@@ -362,12 +362,23 @@ static void sk_word_blk_attach_ack(VM *vm) {
* own ATTACH-ACK-BUF (artemis:init.4th) is a raw CMOVE'd byte buffer
* with no such guarantee, so this copies into its own NUL-terminated
* buffer rather than passing ATTACH-ACK-BUF's address through
* unchanged. Static, not stack-local: sk_hermes_send_one() stores
* payload_addr out-of-line and the caller must keep it alive until
* drained (kernel_hermes.h's own documented contract) -- same single-
* buffer-reuse shape ATTACH-ACK-BUF itself already had (a second attach
* before the first drains overwrites it), not a new hazard this
* introduces. */
* unchanged.
*
* CORRECTED 2026-09-22 (real defect, this task's own findings log):
* this comment used to claim "static, not stack-local" was load-bearing
* because sk_hermes_send_one() stored payload_addr out-of-line, the
* caller having to keep it alive until drained -- and that a second
* attach before the first drains overwriting this buffer was "the same
* shape ATTACH-ACK-BUF itself already had, not a new hazard." That
* claim was wrong: it WAS a new, real payload-aliasing defect (two
* sends before either drains both pointing at this same buffer,
* whichever wrote last silently winning), confirmed live and fixed at
* the source (kernel_hermes.c's sk_hermes_send_one() now copies into
* the message's own storage immediately, kernel_hermes.h's own doc
* comment on SkHermesMessage has the full account). This buffer no
* longer needs to survive past the KH-BLK-ATTACH-SEND call that sends
* it -- staying `static` here is now just a convenience (avoids an
* 80-byte stack frame), not a correctness requirement. */
#define SK_KH_BLK_ATTACH_BUF_SIZE 80
static char g_kh_blk_attach_buf[SK_KH_BLK_ATTACH_BUF_SIZE];
@@ -1387,25 +1398,31 @@ static int sk_repl_line_calls_use(const char *input)
return (len == 3) || (input[len - 4] == ' ' || input[len - 4] == '\t');
}
/* FABRIC-3.6.md task 3.9 -- static, not stack-local, for the same reason
* g_kh_blk_attach_buf (task 3.8) is: sk_hermes_send_one() stores
* payload_addr out-of-line and the caller must keep it alive until
* drained. Sized to the largest single payload kernel-Hermes will ever
* accept (SK_HERMES_CHUNK_MAX_PAYLOAD, task 3.5's one-block bound) --
* console lines up to INPUT_BUFFER_SIZE-1 (1024) content bytes are
* silently capped to SK_HERMES_CHUNK_MAX_PAYLOAD-1 (1023) to leave room
* for the NUL terminator sk_hermes_drain_checkpoint() requires; chunking
* a console line across multiple messages is out of this task's scope
* (task 3.5 built the primitives, not a sender for this use) and the cap
* is not expected to be reached by ordinary interactive typing. Only one
* console-proxy session exists today (console.c's own singleton, per
* memory project_multiseat_console_idea) so the single shared buffer
* carries the same payload-aliasing defect g_kh_blk_attach_buf has
* (FABRIC-3.6.md task 3.8's write-up wrongly called this shape benign;
* corrected in the findings log, entry "found while starting task 3.9",
* once task 3.8's own defect was found). Fix deferred: Captain Bob
* ruled 2026-09-22 to land task 3.9's send-side cutover first and fix
* the aliasing separately. */
/* FABRIC-3.6.md task 3.9. Sized to the largest single payload
* kernel-Hermes will ever accept (SK_HERMES_CHUNK_MAX_PAYLOAD, task
* 3.5's one-block bound) -- console lines up to INPUT_BUFFER_SIZE-1
* (1024) content bytes are silently capped to SK_HERMES_CHUNK_MAX_
* PAYLOAD-1 (1023) to leave room for the NUL terminator
* sk_hermes_drain_checkpoint() requires; chunking a console line across
* multiple messages is out of this task's scope (task 3.5 built the
* primitives, not a sender for this use) and the cap is not expected to
* be reached by ordinary interactive typing.
*
* CORRECTED 2026-09-22 (real defect, found while starting this task,
* fixed at the source): this comment originally justified `static, not
* stack-local` on the claim that sk_hermes_send_one() stored
* payload_addr out-of-line and the caller had to keep it alive until
* drained -- which was true, and which g_kh_blk_attach_buf (task 3.8)
* turned out to make into a genuine payload-aliasing defect (two sends
* before either drains, whichever wrote last silently winning). Fixed
* at the source (kernel_hermes.c's sk_hermes_send_one() now copies into
* the message's own storage immediately, kernel_hermes.h's own doc
* comment on SkHermesMessage has the full account) -- this buffer no
* longer needs to survive past the send call, for this or any future
* console-proxy session (console.c's own singleton today, per memory
* project_multiseat_console_idea). Staying `static` is now just a
* convenience (avoids a 1024-byte stack frame), not a correctness
* requirement. */
static char g_kh_console_cmd_buf[SK_HERMES_CHUNK_MAX_PAYLOAD];
static void sk_repl_dispatch_line(VM *vm, const char *input)
+17 -1
View File
@@ -496,10 +496,26 @@ int sk_hermes_send_one(VMUuid from, VMUuid to, uint32_t type, uint32_t channel_i
if (sk_hermes_alloc(from, &msg) != 0) return -1;
/* Real defect, found 2026-09-22 (FABRIC-3.6.md task 3.8's own
* findings log): this used to store the caller's own payload_addr
* as-is, a pointer this function does not own. Two sends before
* either drains meant both messages pointed at the same
* caller-owned buffer -- whichever send wrote last silently won.
* Copy into this message's own payload_buf instead (kernel_hermes.h,
* sized to SK_HERMES_CHUNK_MAX_PAYLOAD, the bound already enforced
* above) and point payload_addr at that -- every message now owns
* its payload bytes, independent of whatever the caller does with
* its own buffer afterward. payload_len == 0 is a real, valid case
* (an empty payload) -- memcpy of 0 bytes is well-defined even with
* a NULL payload_addr -- guarded anyway (C's memcpy() is technically
* undefined for a NULL argument even at length 0), matching this
* project's own STRICT_PTR discipline elsewhere. */
if (payload_len > 0) memcpy(msg->payload_buf, payload_addr, payload_len);
msg->payload_addr = msg->payload_buf;
msg->type = type;
msg->from = from;
msg->to = to;
msg->payload_addr = payload_addr;
msg->payload_len = payload_len;
msg->channel = channel_id;