Conversation
The shared-page CompletionRing IPC path (syscall 0x1039 and everything downstream) had zero userspace callers, so registration never ran and every consumer arm was unreachable — confirmed during the MAZ-179 investigation (audit trail in that ticket's findings). Live delivery is the WaitSoftIRQ slot rings and io_uring CQs, both untouched. Removal also deletes completionRingPush's latent bounds-check-vs-index modulus defect and un-shadows the block-IRQ fallback's internal topHalfBlockRing arm. Per Ian, the SysRegisterCompletionRing constant is fully removed: slot 57 keeps the conventional freed-slot gap comment and behaves like every other freed slot (syscallPanic; no freed slot returns ENOSYS). Refs: MAZ-180 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe change removes legacy block and input completion-ring registration, notification, polling, and cleanup paths. Block IRQ fallback events now enter ChangesCompletion-ring removal
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
Claude code review (inline, effort: high)Verdict: clean ✅ — 0 confirmed, 0 plausible findings. Reviewed the full diff (whitespace-blind) across correctness, reuse/simplification, and efficiency:
Gates: full suite ✅ (2×), both-arch builds ✅, CC gate pass (only overlapped 🔴 is 🤖 Generated with Claude Code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@kmazarin/kmazarin/block_top_half.go`:
- Around line 129-136: The block soft-IRQ fallback around topHalfBlockRing must
only use a slot owned by the established block-device owner. Update
RegisterSoftIRQSlotKsyscall and its cleanup to reject or atomically replace
competing registrations, and clear blockDeviceOwnerPID together with irqToSlot;
ensure the fallback cannot queue to or wake a caller-owned slot without
validated ownership.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d00c785-7bb8-4c49-937a-4f50481a4498
📒 Files selected for processing (12)
kmazarin/kmazarin/block_top_half.gokmazarin/kmazarin/bottom_half.gokmazarin/kmazarin/input_focus.gokmazarin/kmazarin/soft_irq_slots.gokmazarin/kmazarin/threads.gokmazarin/ksyscall/bridge_asm.gokmazarin/ksyscall/mazzy.gokmazarin/ksyscall/softirq.gokmazarin/ksyscall/wait_softirq_asm.gomazarin/sys/softirq.goshared/hid/hid.goshared/mazzy/mazzy.go
💤 Files with no reviewable changes (7)
- kmazarin/ksyscall/softirq.go
- mazarin/sys/softirq.go
- kmazarin/kmazarin/soft_irq_slots.go
- kmazarin/kmazarin/bottom_half.go
- kmazarin/kmazarin/threads.go
- kmazarin/ksyscall/bridge_asm.go
- kmazarin/ksyscall/wait_softirq_asm.go
Comment on lines
+129
to
+136
| // No active CQ, or CQ full: fall back to the internal soft-IRQ ring. | ||
| atomic.AddUint32(&dbgBlockCQEMissed, 1) | ||
| ev := hid.HIDEvent{ | ||
| Type: tag, | ||
| Code: status, | ||
| Value: info.UsedLen, | ||
| } | ||
| crKVA := blockCompletionRingKVA | ||
| if crKVA != 0 { | ||
| if !completionRingPush(crKVA, ev) { | ||
| atomic.AddUint32(&dbgBlockRingFull, 1) | ||
| } | ||
| } else { | ||
| ringPush(&topHalfBlockRing, ev) | ||
| } | ||
| ringPush(&topHalfBlockRing, ev) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline kmazarin/kmazarin/soft_irq_slots.go --items all --type function \
--match 'RegisterSoftIRQSlotKsyscall|DrainSoftIRQSlotEvents|WakeSlotForIRQ'
rg -n -C 8 \
'blockDeviceOwnerPID|irqToSlot\[|BlockVirtualIRQ|topHalfBlockRing|RegisterSoftIRQSlotKsyscall|DrainSoftIRQSlotEvents' \
kmazarinRepository: iansmith/mazarin
Length of output: 30585
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== soft_irq_slots relevant sections =="
sed -n '45,60p' kmazarin/kmazarin/soft_irq_slots.go
sed -n '107,180p' kmazarin/kmazarin/soft_irq_slots.go
sed -n '260,320p' kmazarin/kmazarin/soft_irq_slots.go
sed -n '457,502p' kmazarin/kmazarin/soft_irq_slots.go
echo "== block completion top half =="
sed -n '100,175p' kmazarin/kmazarin/block_top_half.go
echo "== block owner / SetBlockDeviceOwnerPID call sites =="
rg -n -C 5 'SetBlockDeviceOwnerPID|GetBlockDeviceOwnerPID|blockDeviceOwnerPID|WakeIOUringFromIRQ|WakeSlotForIRQ' kmazarin device kmazarin > /tmp/bg_search.txt
sed -n '1,80p' /tmp/bg_search.txt | sed -n '1,140p' /tmp/bg_search.txt
echo "== structural read-only verifier for block soft-irq ownership properties =="
python3 - <<'PY'
from pathlib import Path
import re
soft = Path('kmazarin/kmazarin/soft_irq_slots.go').read_text()
block = Path('kmazarin/kmazarin/block_top_half.go').read_text()
def fn_body(name):
m = re.search(r'func\s+' + re.escape(name) + r'\b[\s\S]*?\n\}', soft)
return m.group(0) if m else ''
r_body = fn_body('RegisterSoftIRQSlotKsyscall')
c_body = fn_body('CleanupSoftIRQSlotsForShepherd')
b_body = block[block.find('// No active CQ'):block.find('} else {', block.find('// No active CQ'))]
checks = {
'block_registration_assigns_ring': 'ring = &topHalfBlockRing' in r_body,
'block_registration_sets_owner': 'blockDeviceOwnerPID = shepherdID' in r_body,
'block_registration_maps_irq': r_body.count('irqToSlot[irqNum] = slotNum') == 1,
'block_registration_no_owner_guard': 'blockDeviceOwnerPID == shepherdID' not in r_body and 'blockDeviceOwnerPID' not in r_body.split('} else if irqNum == hid.BlockVirtualIRQ',1)[1].split('}')[0],
'cleanup_clears_block_owner': c_body.count('if slot.intKind == hid.DiskInterrupt') == 1 and c_body.find('blockDeviceOwnerPID = -1') > c_body.find('if slot.intKind == hid.DiskInterrupt'),
'fallback_pushes_to_topHalfBlockRing': 'ringPush(&topHalfBlockRing, ev)' in b_body,
'fallback_wakes_block_virtual_irq': 'WakeSlotForIRQ(hid.BlockVirtualIRQ)' in block,
}
for k,v in checks.items():
print(f'{k}={v}')
print('owner_writer_count=', len(re.findall(r'(?m)^\tblockDeviceOwnerPID\s*=', soft)))
print('owner_setter_count=', len(re.findall(r'(?m)^\tblockDeviceOwnerPID\s*=', open('kmazarin/kmazarin/soft_irq_slots.go').read()) +
len(re.findall(r'(?m)^\tblockDeviceOwnerPID\s*=', open('kmazarin/kmazarin/shared/hid/hid.go').read()) if Path('kmazarin/kmazarin/shared/hid/hid.go').exists() else [])))
PYRepository: iansmith/mazarin
Length of output: 8911
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== soft_irq_slots relevant sections =="
sed -n '45,60p' kmazarin/kmazarin/soft_irq_slots.go
sed -n '107,180p' kmazarin/kmazarin/soft_irq_slots.go
sed -n '260,320p' kmazarin/kmazarin/soft_irq_slots.go
sed -n '457,502p' kmazarin/kmazarin/soft_irq_slots.go
echo "== block completion top half =="
sed -n '100,175p' kmazarin/kmazarin/block_top_half.go
echo "== block owner / SetBlockDeviceOwnerPID call sites =="
rg -n -C 5 'SetBlockDeviceOwnerPID|GetBlockDeviceOwnerPID|blockDeviceOwnerPID|WakeIOUringFromIRQ|WakeSlotForIRQ' kmazarin device kmazarin > /tmp/bg_search.txt
sed -n '1,140p' /tmp/bg_search.txt
echo "== structural read-only verifier for block soft-irq ownership properties =="
python3 - <<'PY'
from pathlib import Path
import re
soft = Path('kmazarin/kmazarin/soft_irq_slots.go').read_text()
block = Path('kmazarin/kmazarin/block_top_half.go').read_text()
def fn_body(name):
m = re.search(r'func\s+' + re.escape(name) + r'\b[\s\S]*?\n\}', soft)
return m.group(0) if m else ''
r_body = fn_body('RegisterSoftIRQSlotKsyscall')
c_body = fn_body('CleanupSoftIRQSlotsForShepherd')
b_body = block[block.find('// No active CQ'):block.find('} else {', block.find('// No active CQ'))]
checks = {
'block_registration_assigns_ring': 'ring = &topHalfBlockRing' in r_body,
'block_registration_sets_owner': 'blockDeviceOwnerPID = shepherdID' in r_body,
'block_registration_maps_irq': r_body.count('irqToSlot[irqNum] = slotNum') == 1,
'block_registration_no_owner_guard': 'blockDeviceOwnerPID == shepherdID' not in r_body and 'blockDeviceOwnerPID' not in r_body.split('} else if irqNum == hid.BlockVirtualIRQ',1)[1].split('}')[0],
'cleanup_clears_block_owner': c_body.count('if slot.intKind == hid.DiskInterrupt') == 1 and c_body.find('blockDeviceOwnerPID = -1') > c_body.find('if slot.intKind == hid.DiskInterrupt'),
'fallback_pushes_to_topHalfBlockRing': 'ringPush(&topHalfBlockRing, ev)' in b_body,
'fallback_wakes_block_virtual_irq': 'WakeSlotForIRQ(hid.BlockVirtualIRQ)' in block,
}
for k,v in checks.items():
print(f'{k}={v}')
print('owner_writer_count=', len(re.findall(r'(?m)^\tblockDeviceOwnerPID\s*=', soft)))
print('owner_setter_count=', len(re.findall(r'(?m)^\tblockDeviceOwnerPID\s*=', open('kmazarin/kmazarin/soft_irq_slots.go').read()) +
len(re.findall(r'(?m)^\tblockDeviceOwnerPID\s*=', open('kmazarin/kmazarin/shared/hid/hid.go').read()) if Path('kmazarin/kmazarin/shared/hid/hid.go').exists() else [])))
PYRepository: iansmith/mazarin
Length of output: 8911
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files under kmazarin =="
git ls-files kmazarin 'kmazarin/**' | sed 's#^`#-` #'
echo "== exact owner writes and call sites =="
rg -n '\bblockDeviceOwnerPID\s*=|SetBlockDeviceOwnerPID\(|GetBlockDeviceOwnerPID\(' . --glob '*.go' --glob '!vendor/**'
echo "== softirq syscall and registration context =="
sed -n '120,150p' kmazarin/kmazarin/ksyscall/softirq.go
sed -n '185,225p' kmazarin/kmazarin/soft_irq_slots.go
sed -n '290,315p' kmazarin/kmazarin/soft_irq_slots.go
sed -n '457,502p' kmazarin/kmazarin/soft_irq_slots.goRepository: iansmith/mazarin
Length of output: 14465
Security And Privacy (CWE-862): Missing Authorization
Reachability: External · Exploitability: Moderate
Authorize the block soft-IRQ consumer before this fallback.
SyscallRegisterSoftIRQ accepts a caller-selected hid.BlockVirtualIRQ. RegisterSoftIRQSlotKsyscall assigns that caller's slot to topHalfBlockRing, sets blockDeviceOwnerPID, and stores irqToSlot[irqNum] without an owner guard. This fallback then queues block completions to that ring and wakes the mapped caller-owned slot. Keep ownership only with the established block-device owner; reject or atomically replace competing registrations; clear ownership plus the IRQ mapping together during cleanup.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@kmazarin/kmazarin/block_top_half.go` around lines 129 - 136, The block
soft-IRQ fallback around topHalfBlockRing must only use a slot owned by the
established block-device owner. Update RegisterSoftIRQSlotKsyscall and its
cleanup to reject or atomically replace competing registrations, and clear
blockDeviceOwnerPID together with irqToSlot; ensure the fallback cannot queue to
or wake a caller-owned slot without validated ownership.
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
1 participant
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.
Summary
Removes the dead legacy shared-page CompletionRing IPC path — syscall 0x1039
SysRegisterCompletionRingand everything downstream. The registration syscall had zero userspace callers (verified by grep sweep over all.go/.splus a no-filter sweep, on master8822835e), soblockCompletionRingKVA/wmInputRingKVAwere always 0 and every consumer arm was unreachable. Confirmed dead during the MAZ-179 investigation; audit trail in that ticket.Live delivery paths are untouched: WaitSoftIRQ slot rings (
softIRQRing/ringPush/RingDrain) and io_uring CQs.Bonus defect deleted:
completionRingPushbounds-checkedtail-head >= hid.CompletionRingSizebut indexedEvents[tail%ring.Capacity]— two different moduli.What was removed
mazarin/sys/softirq.go):RegisterCompletionRing,PollCompletionRingkmazarin/ksyscall/):SyscallRegisterCompletionRing, dispatch entry 57, the twoRegister*CompletionRinglinkname decls, caller-lessgetBlockRingFulllinknamekmazarin/kmazarin/): both CompletionRing sections insoft_irq_slots.go(registration/cleanup/accessors + 6 state vars), the two shepherd-deathCleanup*CompletionRingcalls inthreads.go,completionRingPush+dbgBlockRingFull/GetBlockRingFullinbottom_half.go, the never-takencrKVA != 0fallback arm inblock_top_half.go(now unconditionallyringPush(&topHalfBlockRing, ev)), andinput_focus.go's caller-lessrouteInputEvent,wakeInputConsumersshim, and WM wake chain (wakeWMViaMailbox/wakeWMViaUringFn/wakeWMViaUringImpl/hidNotifyMsg)hid.CompletionRingstruct +CompletionRingSize;SysRegisterCompletionRingconstant (per Ian — slot 57 keeps the conventional freed-slot gap comment; unmapped slots panic like every other freed slot, no freed slot returns ENOSYS)Kept deliberately:
KernelWriteToRingFromIRQ(io_uring core), WM-role claiming (SysRequestWindowManager/windowManagerSID),blockDeviceOwnerPID,dbgBlockCQEMissed(live in the surviving fallback).Most of the +122 is gofmt column realignment in the two syscall-number tables;
git diff -wshows ~30 real non-deletion lines.Verification
slot 57 freedgap comments$GO tool task(ARM64) ✅ ·$GO tool task all(x86_64) ✅ ·$GO tool task test✅Complexity notes
CC gate: the only 🔴 overlapping this diff is
blockTopHalf(CC 20), a pre-existing violation this branch reduces (22 on master; deleting the fallback branch removed two decision points). The other 22 🔴 in touched files are pre-existing with no hunk overlap (deletion-only diff) — not blocking per repo precedent (MAZ-165/167/175). NLOC:threads.go2090 (pre-existing, warn-only).Ticket: https://linear.app/mazarin/issue/MAZ-180/remove-dead-legacy-completionring-machinery
🤖 Generated with Claude Code
Summary by CodeRabbit