Page MenuHomeFreeBSD

icmp6: fix use-after-reference-release
ClosedPublic

Authored by kp on May 22 2025, 12:39 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Sep 27, 4:13 AM
Unknown Object (File)
Thu, Sep 25, 10:02 PM
Unknown Object (File)
Thu, Sep 25, 3:46 AM
Unknown Object (File)
Wed, Sep 24, 12:21 AM
Unknown Object (File)
Tue, Sep 23, 11:52 PM
Unknown Object (File)
Tue, Sep 23, 2:08 AM
Unknown Object (File)
Sun, Sep 21, 8:03 PM
Unknown Object (File)
Wed, Sep 17, 3:34 PM

Details

Summary

We release the reference to the in6_ifaddr but retain a pointer to it.
Copy the address itself, rather than keeping the pointer to fix this.

Sponsored by: Rubicon Communications, LLC ("Netgate")

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kp requested review of this revision.May 22 2025, 12:39 PM

I think this change is not necessary, but the comment should be revised instead. Is there any issue caused by referencing a released ifa ?

sys/netinet6/icmp6.c
2465

Well, icmp6_redirect_output() is within net epoch section, the following ifa_free() will defer the deallocating, so it is still safe to reference the released ifa. Then this comment is not accurate.

sys/netinet6/icmp6.c
2465

That's a good catch. You're correct that this isn't actually going to cause problems, but I'm still inclined to make the change anyway, because it's highly counterintuitive (and might break if we ever change how ifa_free() works).

Copying the address is clearly safe, and shouldn't cause anyone else to go chasing phantoms. It's a small amount of extra work, but this is essentially an error path (and is rate limited on line 2434).

This revision was not accepted when it landed; it landed in state Needs Review.May 27 2025, 12:03 PM
This revision was automatically updated to reflect the committed changes.