kdmsg: bound circuit nesting to prevent kernel stack overflow - #41
Open
dreamfast wants to merge 1 commit into
Open
kdmsg: bound circuit nesting to prevent kernel stack overflow#41dreamfast wants to merge 1 commit into
dreamfast wants to merge 1 commit into
Conversation
kdmsg_simulate_failure() and kdmsg_state_dying() recurse through a state's subq child tree with no depth limit. A DMSG peer builds an arbitrarily deep parent->child chain via CREATE messages, and teardown (DELETE on the root, or connection close) drives recursion that overflows the 16 KB LWKT kernel thread stack and double-faults. Add a depth field to struct kdmsg_state and reject CREATEs whose parent is already at DMSG_MAX_CIRCUIT_DEPTH (8), setting depth in both the receive and transmit CREATE paths. The cap is 8 rather than a larger value because kdmsg_state_abort() re-enters the receive path per level, so each nesting level costs a ~5-function cycle (~485 bytes); 33 levels empirically still overflow the stack, while 8 keeps the worst case (~4.4 KB) within it with a >3x margin.
Contributor
|
Can you explain when this path can happen without triggering via a custom script ? |
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.
kdmsg: bound circuit nesting to prevent kernel stack overflow
kdmsg_simulate_failure() and kdmsg_state_dying() in
sys/kern/kern_dmsg.crecurse without a depth limit through a state'ssubqchild tree (the recursive call is inside theTAILQ_FOREACHoversubq). A DMSG peer builds an arbitrarily deep parent→child chain by sending CREATE messages whosecircuitfield names the previous state as parent — the child is linked intopstate->subqon the receive path — then triggers teardown with a DELETE on the chain root or by closing the connection. The resulting depth-first recursion overflows the 16 KB LWKT kernel thread stack and double-faults the kernel. CRC is not verified on the receive path, so any peer that can reach a DMSG link can forge the triggering messages: the userland hammer2 cluster relay over the network (LNK_AUTH is unimplemented), or locally viaDIOCRECLUSTERon a disk device node.Fix
Add an
int depthfield tostruct kdmsg_stateand reject CREATEs whose parent is already atDMSG_MAX_CIRCUIT_DEPTH(8), in the receive CREATE path ofkdmsg_state_msgrx.depthis set in both the receive and transmit CREATE paths so a transmit-created state used as a circuit parent carries an accurate value. This bounds the recursive teardown to ~9 levels. The cap is deliberately 8 rather than a larger value:kdmsg_state_abort()re-enters the full receive path for each level, so each nesting level costs a ~5-function call cycle (~485 bytes) — a 33-deep chain was found to still overflow the 16 KB stack, while 8 keeps the worst case (~4.4 KB) within it with a >3x margin. Typical DMSG/HAMMER2 circuit nesting is 1–3.Before / after
Reproducer
Build and run as root/operator (a reachable disk node is needed for
DIOCRECLUSTER):