Page MenuHomeFreeBSD

Fix NULL deref in ip_output during route change
ClosedPublic

Authored by vangyzen on May 23 2023, 3:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 9, 3:17 AM
Unknown Object (File)
Wed, Nov 20, 5:59 AM
Unknown Object (File)
Oct 27 2024, 10:04 AM
Unknown Object (File)
Oct 7 2024, 6:54 AM
Unknown Object (File)
Oct 4 2024, 4:48 AM
Unknown Object (File)
Oct 3 2024, 3:59 PM
Unknown Object (File)
Oct 2 2024, 11:59 PM
Unknown Object (File)
Oct 1 2024, 12:01 AM

Details

Summary

When changing the interface address during a route change,
the rtentry's rt_ifa will be NULL briefly. Some parts of
ip_output do not handle that NULL. In such case, re-validate
the rtentry. That validation does not check the rt_ifa, but
it does lock the route, which will synchronize with
rtrequest1_fib_change.

I would prefer to leave the rt_ifa pointer intact during
the route change, but ip6_output is not fully protected
by the net_epoch, so that could allow a use-after-free.
ip6_output already handles a NULL rt_ifa.

This is a direct commit to stable/12 because later branches
have nexthop and do not appear to have this bug.

PR: 271573
Reported by: Gaurav.Gandhi@dell.com
Sponsored by: Dell EMC Isilon

Test Plan

See the PR.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 51640
Build 48531: arc lint + arc unit

Event Timeline

vangyzen edited the test plan for this revision. (Show Details)

I'm afraid I won't be able to review this in the foreseeable future. I think I completely forgot pre-nexthop routing logic.

This revision is now accepted and ready to land.May 30 2023, 3:04 PM

Looks like a sensible approach for 12.x

yuripv added inline comments.
sys/netinet/ip_output.c
398

Sorry for chiming in late. The only "goto again" below does some cleanup, is it not needed here?