Page MenuHomeFreeBSD

icmp6: fix use-after-reference-release
ClosedPublic

Authored by kp on Thu, May 22, 12:39 PM.
Tags
None
Referenced Files
F119292192: D50460.id155857.diff
Sat, Jun 7, 7:38 AM
Unknown Object (File)
Thu, Jun 5, 10:30 AM
Unknown Object (File)
Tue, Jun 3, 10:54 PM
Unknown Object (File)
Mon, Jun 2, 12:41 AM
Unknown Object (File)
Tue, May 27, 12:03 PM
Unknown Object (File)
Sat, May 24, 3:02 AM
Unknown Object (File)
Sat, May 24, 12:39 AM
Unknown Object (File)
Fri, May 23, 2:23 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 Skipped
Unit
Tests Skipped
Build Status
Buildable 64347
Build 61231: arc lint + arc unit

Event Timeline

kp requested review of this revision.Thu, May 22, 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.Tue, May 27, 12:03 PM
This revision was automatically updated to reflect the committed changes.