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)
Tue, May 7, 5:55 AM
Unknown Object (File)
Sun, Apr 21, 10:15 AM
Unknown Object (File)
Apr 4 2024, 10:28 PM
Unknown Object (File)
Apr 4 2024, 4:15 PM
Unknown Object (File)
Mar 6 2024, 8:47 PM
Unknown Object (File)
Feb 29 2024, 8:49 PM
Unknown Object (File)
Feb 17 2024, 8:26 PM
Unknown Object (File)
Dec 31 2023, 2:02 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?