Skip to content

Conversation

@rsmarples
Copy link
Member

As such, we cannot access dhcpcd_ctx so we can't use the free route list.
Fixes #571.

As such, we cannot access dhcpcd_ctx so we can't use the free
route list.
Fixes #571.
@coderabbitai
Copy link

coderabbitai bot commented Dec 28, 2025

Walkthrough

The changes fix a NULL pointer dereference vulnerability in route cleanup code. When rt_headclear processes a route node with a NULL interface pointer, it now safely passes NULL context to rt_headclear0 instead of dereferencing the NULL pointer, and rt_headclear0 now conditionally guards its assertion against NULL contexts.

Changes

Cohort / File(s) Summary
NULL pointer dereference fix in route cleanup
src/route.c
Modified rt_headclear to check if rt->rt_ifp is NULL before dereferencing it for ctx; updated rt_headclear0 to conditionally assert ctx is non-NULL only under RT_FREE_ROUTE_TABLE when ctx is actually non-NULL

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'route: Routes may not have an interface assinged' directly addresses the core change—handling routes without an interface pointer.
Description check ✅ Passed The description explains that routes may lack an associated interface, preventing ctx access, and references issue #571 which documents the NULL-pointer dereference.
Linked Issues check ✅ Passed The code changes directly fix the NULL-pointer dereference in rt_headclear by guarding against NULL ctx and conditionally passing NULL ctx when rt_ifp is NULL.
Out of Scope Changes check ✅ Passed The changes to rt_headclear0 and rt_headclear are directly focused on preventing NULL-pointer dereference and do not introduce unrelated modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-route-headclear

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 117742d and 83eafa0.

📒 Files selected for processing (1)
  • src/route.c
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
  • GitHub Check: netbsd (--disable-ipv6)
  • GitHub Check: netbsd (--disable-ipv4ll, -DSMALL)
  • GitHub Check: netbsd (--disable-ipv6, -DSMALL)
  • GitHub Check: netbsd (--disable-dhcp6, -DSMALL)
  • GitHub Check: netbsd (--disable-arp, -DSMALL)
  • GitHub Check: netbsd (--disable-dhcp6)
  • GitHub Check: netbsd (-DSMALL)
  • GitHub Check: netbsd (--disable-ipv4)
  • GitHub Check: netbsd (--disable-ipv4ll)
  • GitHub Check: netbsd
  • GitHub Check: netbsd (--disable-ipv4, -DSMALL)
  • GitHub Check: netbsd (--disable-arp)
  • GitHub Check: freebsd
  • GitHub Check: openbsd
🔇 Additional comments (2)
src/route.c (2)

317-318: LGTM! Properly guards assertion against NULL context.

The conditional check correctly prevents the assertion from executing when ctx is NULL, which occurs when routes lack an associated interface. Since rt_free handles NULL rt_ifp by directly freeing the route (bypassing the free route table), this assertion isn't needed in that case.


338-338: LGTM! Correctly prevents NULL pointer dereference.

The ternary operator safely checks whether rt->rt_ifp is NULL before dereferencing it to access ctx. This directly fixes the NULL pointer dereference vulnerability reported in issue #571, where routes without an associated interface would cause a crash during cleanup.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dhcpcd/src/route.c:338 SEGV by a READ memory access in rt_headclear

2 participants