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)
Sun, Oct 19, 6:38 PM
Unknown Object (File)
Sat, Oct 18, 1:58 PM
Unknown Object (File)
Fri, Oct 17, 4:42 AM
Unknown Object (File)
Fri, Oct 17, 12:46 AM
Unknown Object (File)
Wed, Oct 15, 3:21 AM
Unknown Object (File)
Mon, Oct 6, 12:58 AM
Unknown Object (File)
Fri, Oct 3, 1:08 PM
Unknown Object (File)
Thu, Oct 2, 11:24 PM

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?