DHCPv6: When deprecating addresses, restart on prefix deletions

[rsmarples]

As that might invalidate the next address to iterate on.

Reported-by: CuB3y0nd root@cubeyond.net

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7b188c9c-81c6-4167-89a4-42fef89502b8

📥 Commits

Reviewing files that changed from the base of the PR and between 117ee18 and a467b5f.

📒 Files selected for processing (1)

• src/dhcp6.c

🚧 Files skipped from review as they are similar to previous changes (1)

• src/dhcp6.c

---

Walkthrough

In src/dhcp6.c, dhcp6_deprecatedele() is changed from void to bool, gains a self-pointer guard (dda != ia) before freeing a delegated address, and returns a freed flag. dhcp6_deprecateaddrs() captures this return value in again and, after deleting the current ia, conditionally restarts the TAILQ loop via goto again to avoid using an invalidated next pointer.

Changes

Delegated-prefix deprecation loop fix

Layer / File(s)	Summary
dhcp6_deprecatedele: void→bool with self-pointer guard src/dhcp6.c	Return type changed to bool; a freed local is introduced; the removal/free path is skipped when dda == ia (guard against freeing the current element alias); freed is returned.
dhcp6_deprecateaddrs: capture return and restart loop src/dhcp6.c	The call to dhcp6_deprecatedele(ia) now stores its result in again; after ia is deleted and freed, goto again is executed conditionally on again being true, preventing use of an invalidated TAILQ next pointer.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: restarting address deprecation loops when prefix deletions occur, which is the core fix implemented in the changeset.
Description check	✅ Passed	The description is related to the changeset, explaining why the restart is needed (invalidation of iteration pointers) and crediting the reporter, though it lacks detail about implementation specifics.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch deprecate_dele

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/dhcp6.c (1)

2636-2641: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

again is read uninitialized and is sticky across iterations.

again (line 2615) is assigned only when ia->flags & IPV6_AF_PFXDELEGATION is set, but it is unconditionally read at line 2655 (if (again) goto again;).

• For the first ia that lacks IPV6_AF_PFXDELEGATION, again is read uninitialized → undefined behavior.
• Once it becomes true, it is never cleared, so subsequent iterations (including those without IPV6_AF_PFXDELEGATION) will spuriously goto again, restarting the loop and re-walking the list unnecessarily.

Reset again at the start of each iteration before the conditional assignment.

🐛 Proposed fix: reset per-iteration

 `#ifndef` SMALL
 		/* If we delegated from this prefix, deprecate or remove
 		 * the delegations. */
+		again = false;
 		if (ia->flags & IPV6_AF_PFXDELEGATION)
 			again = dhcp6_deprecatedele(ia);
 `#endif`

🤖 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 `@src/dhcp6.c` around lines 2636 - 2641, The `again` variable is only
conditionally assigned when the `IPV6_AF_PFXDELEGATION` flag is checked and
`dhcp6_deprecatedele(ia)` is called, but it is unconditionally read at the end
of the loop iteration in the `goto again` check, causing uninitialized variable
access and sticky state across iterations. Reset the `again` variable to false
at the beginning of each loop iteration that processes the `ia` items, before
the conditional check for `IPV6_AF_PFXDELEGATION`. This ensures the variable is
always initialized and does not carry over its value from previous iterations.

🤖 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.

Outside diff comments:
In `@src/dhcp6.c`:
- Around line 2636-2641: The `again` variable is only conditionally assigned
when the `IPV6_AF_PFXDELEGATION` flag is checked and `dhcp6_deprecatedele(ia)`
is called, but it is unconditionally read at the end of the loop iteration in
the `goto again` check, causing uninitialized variable access and sticky state
across iterations. Reset the `again` variable to false at the beginning of each
loop iteration that processes the `ia` items, before the conditional check for
`IPV6_AF_PFXDELEGATION`. This ensures the variable is always initialized and
does not carry over its value from previous iterations.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4842ca38-3277-42d6-bc58-5cc480fe8b51

📥 Commits

Reviewing files that changed from the base of the PR and between 2f00c7b and 117ee18.

📒 Files selected for processing (1)

• src/dhcp6.c