[17.0-stable] usbmanager: stop self-parent assigngrp from OOMing pillar - #6253
Open
eriknordmark wants to merge 1 commit into
Open
Conversation
eriknordmark
marked this pull request as ready for review
July 31, 2026 20:13
eriknordmark
force-pushed
the
usbmanager-self-parent-recursion-17.0-stable
branch
from
July 31, 2026 21:03
e25b021 to
59f3e31
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 17.0-stable #6253 +/- ##
===============================================
+ Coverage 20.61% 21.03% +0.42%
===============================================
Files 506 516 +10
Lines 92880 94590 +1710
===============================================
+ Hits 19148 19900 +752
- Misses 72136 72915 +779
- Partials 1596 1775 +179 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
rene
approved these changes
Aug 1, 2026
A PhysicalIO whose parentassigngrp equals its own assigngrp built a self-referential node in usbmanager's ioBundleTree (children[grp] points at the group itself), because addIOBundle's circular-dependency check cannot catch a self-parent: a group is never among its own descendants before it is inserted. groupDependendentsImpl then recursed on that self-loop with no visited guard, growing the goroutine stack without bound until the pillar memory cgroup OOM-killed zedbox and the watchdog rebooted the device in a loop. Reject a self-parent bundle in addIOBundle, and guard both tree walks (groupDependendentsImpl and groupParents) against cycles with a visited set so a malformed model can no longer wedge usbmanager. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: eriknordmark <erik@zededa.com> (cherry picked from commit 00a7df8)
eriknordmark
force-pushed
the
usbmanager-self-parent-recursion-17.0-stable
branch
from
August 3, 2026 18:19
59f3e31 to
d9d0a80
Compare
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.
Description
Backport of #6184 to
17.0-stable.A device model can declare a PhysicalIO whose
parentassigngrpequals its ownassigngrp. When usbmanager processes such a bundle it builds aself-referential node in its internal ioBundle tree (the group becomes its own
child), and the dependents walk then recurses on that self-loop with no visited
guard. The goroutine stack grows without bound until the pillar memory cgroup
OOM-kills zedbox, and the watchdog reboots the device — repeatedly, since the
offending config is re-applied on every boot, leaving the device stuck in a
reboot loop and unmanageable.
The existing circular-dependency check in
addIOBundlecannot catch aself-parent, because a group is never among its own descendants before it is
inserted. This change rejects a self-parent bundle in
addIOBundle, and guardsboth tree walks (
groupDependendentsImplandgroupParents) with a visitedset so any cyclic
parentassigngrpgraph that reaches them terminates.Cherry-picked with
git cherry-pick -xfrom00a7df8768e80a0726996e1dc84fe1434a2cae62, the merge of #6184 onmaster. Itapplies with no adaptation:
pkg/pillar/cmd/usbmanager/ioBundleTree.goand itstest file have not been touched since the file was added in
31b9c8eab(Dec 2023), so both are byte-identical to master on this branch and the
resulting diff matches #6184 exactly.
How to test and validate this PR
Covered by unit tests in
pkg/pillar/cmd/usbmanager/ioBundleTree_test.go(
TestSelfParentAssigngrpRejected,TestTreeWalksGuardCycles).Both were run against this branch's
pkg/pillarbefore and after the fix.Without it,
TestSelfParentAssigngrpRejectedreports that the self-parentbundle was added, and
TestTreeWalksGuardCyclesdoes not terminate — the runeither trips the test's 2 s guard or dies with
fatal error: stack overflow.With the fix, the full
./cmd/usbmanager/package passes, along withgo vetand
gofmt.End to end: push a device model with a PhysicalIO whose
parentassigngrpequals its
assigngrp. On an unfixed build the device OOM-reboots within~2 minutes and loops; with this fix the model is reported as an error
(
IOBundle cannot be its own parent) and the device stays up with steadymemory.
Changelog notes
Fixed a device reboot loop that could occur when the device model contained an
I/O adapter whose parent assignment group referred to itself.
PR Backports
All four current LTS branches carry the affected code byte-identical to master,
so all of them are affected. This PR is one of a set of four:
Checklist
configuration surface changes
identical change was verified on amd64 in usbmanager: stop self-parent assigngrp from OOMing pillar #6184, and the unit tests above
were run against this branch
For backport PRs (remove it if it's not a backport):
And the last but not least:
check them.
🤖 Generated with Claude Code