Page MenuHomeFreeBSD

Plug some ifaddr refcount leaks.
ClosedPublic

Authored by markj on Dec 23 2019, 7:09 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jan 22, 4:23 PM
Unknown Object (File)
Sun, Jan 22, 4:22 PM
Unknown Object (File)
Sun, Jan 22, 4:22 PM
Unknown Object (File)
Sun, Jan 22, 4:15 PM
Unknown Object (File)
Dec 31 2022, 2:02 AM
Unknown Object (File)
Dec 24 2022, 12:06 PM
Subscribers

Details

Summary
  • Don't take a ref in rt_exportinfo(). This was quite wrong in that we took the ref even when NHR_REF was not set.
  • Don't unconditionally take a ref in rtrequest1_fib(). rt_getifa_fib() will acquire a reference, in which case we would previously acquire two references.
  • Stop taking a reference in rtinit1() before calling rtrequest1_fib(). rtrequest1_fib() will acquire a reference for the RTM_ADD case.
Test Plan

This fixes a memory leak observed when removing an interface address.

I would appreciate any testing that others can provide.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj added a reviewer: network.
markj added reviewers: ae, melifaro.

LGTM.

Any chance you have some dtrace / kgdb scripts verifying refcounting that can be added to the test suite?

This revision is now accepted and ready to land.Dec 25 2019, 10:07 AM

LGTM.

Any chance you have some dtrace / kgdb scripts verifying refcounting that can be added to the test suite?

I'm afraid I don't. I added some dtrace probes to ifa_ref() and friends to help debug the issue and looked at the vmstat -m stats to verify the change, but it was all ad-hoc.

sys/net/route.c
909 ↗(On Diff #65930)

I guess it would be formally correct to take a ref in the NHR_REF case, though no existing callers care. I will do that and make sure we release the reference.

This revision was automatically updated to reflect the committed changes.