network/tc: Fix stack overflow/concurrent modification when deleting TC qdisc trees (backport for v255)

[rocker-zhang]

User description

Backport of upstream commit ee8f605 to v255.

Original issue: systemd#32247

This fixes a stack overflow SIGSEGV (or hashmap assertion ABRT) when deleting deep TC qdisc trees, which caused mass machine disconnection in production when applying traffic control rules.

The fix:

• Split into two-phase mark-and-sweep: first recursively mark all nodes to delete, then single pass to drop all marked nodes
• Avoids mutual recursion that causes stack overflow on deep trees
• Avoids concurrent modification of hash set with multiple open iterators

Original patch by Daan De Meyer <daan.j.demeyer@gmail.com>.
Backport to v255 by Rocker Zhang <zhang.rocker.liyuan@gmail.com>.

Closes systemd#32247.

---

PR Type

Bug fix

---

Description

• Fix stack overflow and hashmap assertion when deleting deep TC qdisc trees

• Replace mutually recursive qdisc/tclass deletion with two-phase mark-and-sweep

• First recursively mark nodes, then drop all marked entries in single passes

• Backport of upstream fix ee8f605 for issue networkd segfault/stack overflow when dropping tclass/qdisc systemd/systemd#32247 to v255

---

Diagram Walkthrough

flowchart LR
  A["qdisc_drop / tclass_drop"] --> B["qdisc_mark_recursive / tclass_mark_recursive"]
  B -->|"mark all related nodes"| C["link_qdisc_drop_marked / link_tclass_drop_marked"]
  C -->|"single-pass sweep"| D["remove marked qdiscs and tclasses safely"]

Loading

File Walkthrough

	Relevant files
Bug fix	qdisc.c Two-phase mark-and-sweep qdisc deletion                                    src/network/tc/qdisc.c Refactor qdisc_drop() into two-phase mark-and-sweep: qdisc_mark_recursive() marks qdisc and child tclasses Add link_qdisc_drop_marked() that iterates once over link->qdiscs and frees marked entries qdisc_drop() now marks recursively, then calls link_tclass_drop_marked() before link_qdisc_drop_marked() and returns NULL Eliminates mutual recursion with tclass_drop() that caused stack overflow and unsafe set modification +39/-13  tclass.c Two-phase mark-and-sweep tclass deletion                                  src/network/tc/tclass.c Refactor tclass_drop() into two-phase mark-and-sweep: tclass_mark_recursive() marks tclass and child qdiscs Add link_tclass_drop_marked() that iterates once over link->tclasses and frees marked entries tclass_drop() now marks recursively, then calls link_qdisc_drop_marked() before link_tclass_drop_marked() and returns NULL Eliminates mutual recursion with qdisc_drop() that caused stack overflow and unsafe set modification +38/-13
Enhancement	qdisc.h Declare qdisc mark-and-sweep functions                                      src/network/tc/qdisc.h Declare new functions qdisc_mark_recursive() and link_qdisc_drop_marked() +2/-0      tclass.h Declare tclass mark-and-sweep functions                                    src/network/tc/tclass.h Declare new functions tclass_mark_recursive() and link_tclass_drop_marked() +2/-0

---

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0d6df438b9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Use iterative marking to prevent deep-tree stack overflow

When deleting a sufficiently deep alternating qdisc/class hierarchy, this call still mutually recurses through tclass_mark_recursive() and back into qdisc_mark_recursive(), consuming one or more stack frames per tree level and potentially crashing networkd before the sweep begins. The two-phase approach prevents concurrent set modification, but it does not address the stated stack-overflow scenario; the marking traversal needs an explicit work queue or stack.

Useful? React with 👍 / 👎.

[rocker-zhang]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

⚡ Recommended focus areas for review Possible Issue The new code calls qdisc_mark, qdisc_unmark, qdisc_is_marked (and the tclass_mark/tclass_unmark/tclass_is_marked equivalents in src/network/tc/tclass.c), but this PR does not define any of them, and to my knowledge stock v255 does not provide these helpers — upstream introduced the mark/unmark helpers (and the corresponding MARKED config-state bit) as part of the very commit being backported here. Unless the target branch already carries those definitions, this backport will fail to compile. Uncertainty: the helpers may exist elsewhere on this branch (e.g., via DEFINE_NETWORK_CONFIG_STATE_FUNCTIONS or a prior cherry-pick); please verify they are available before merging. void qdisc_mark_recursive(QDisc *qdisc) { TClass *tclass; assert(qdisc); assert(qdisc->link); if (qdisc_is_marked(qdisc)) return; qdisc_mark(qdisc);

[rocker-zhang]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Add missing link NULL assertion Unlike the symmetric qdisc_drop(), tclass_drop() dereferences tclass->link (in the link_qdisc_drop_marked() and link_tclass_drop_marked() calls) without first asserting it is non-NULL. Add assert(tclass->link) for parity with qdisc_drop(), so an invalid object fails fast and clearly at function entry rather than relying on the nested assert inside tclass_mark_recursive(). src/network/tc/tclass.c [295-298] TClass* tclass_drop(TClass *tclass) { assert(tclass); + assert(tclass->link); tclass_mark_recursive(tclass); Suggestion importance[1-10]: 3 __ Why: The suggestion is accurate: qdisc_drop() includes assert(qdisc->link) while tclass_drop() lacks the symmetric assert(tclass->link), and adding it improves consistency and fail-fast behavior. However, the impact is minor since tclass_mark_recursive() already asserts tclass->link immediately after, making this mostly a stylistic parity improvement.	Low