Date: Mon, 7 Mar 2022 17:45:53 GMT From: Ryan Stone <rstone@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 617729314af3 - stable/13 - Fix ifa refcount leak in ifa_ifwithnet() Message-ID: <202203071745.227HjrI7012429@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by rstone: URL: https://cgit.FreeBSD.org/src/commit/?id=617729314af33b28ffaf42261561ccce1ab817c8 commit 617729314af33b28ffaf42261561ccce1ab817c8 Author: Ryan Stone <rstone@FreeBSD.org> AuthorDate: 2021-02-11 16:17:58 +0000 Commit: Ryan Stone <rstone@FreeBSD.org> CommitDate: 2022-03-07 17:41:40 +0000 Fix ifa refcount leak in ifa_ifwithnet() In 4f6c66cc9c75c8, ifa_ifwithnet() was changed to no longer ifa_ref() the returned ifaddr, and instead the caller was required to stay in the net_epoch for as long as they wanted the ifaddr to remain valid. However, this missed the case where an AF_LINK lookup would call ifaddr_byindex(), which still does ifa_ref() the ifaddr. This would cause a refcount leak. Fix this by inlining the relevant parts of ifaddr_byindex() here, with the ifa_ref() call removed. This also avoids an unnecessary entry and exit from the net_epoch for this case. I've audited all in-tree consumers of ifa_ifwithnet() that could possibly perform an AF_LINK lookup and confirmed that none of them will expect the ifaddr to have a reference that they need to release. MFC after: 2 months Sponsored by: Dell Inc Differential Revision: https://reviews.freebsd.org/D28705 Reviewed by: melifaro (cherry picked from commit 5adea417d494b9c0e186cf2d06f98873d02d1834) --- sys/net/if.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/sys/net/if.c b/sys/net/if.c index 7d7206e7596c..d4871ccbc1f7 100644 --- a/sys/net/if.c +++ b/sys/net/if.c @@ -2032,6 +2032,7 @@ ifa_ifwithnet(const struct sockaddr *addr, int ignore_ptp, int fibnum) struct ifaddr *ifa_maybe = NULL; u_int af = addr->sa_family; const char *addr_data = addr->sa_data, *cplim; + const struct sockaddr_dl *sdl; NET_EPOCH_ASSERT(); /* @@ -2039,9 +2040,14 @@ ifa_ifwithnet(const struct sockaddr *addr, int ignore_ptp, int fibnum) * so do that if we can. */ if (af == AF_LINK) { - const struct sockaddr_dl *sdl = (const struct sockaddr_dl *)addr; - if (sdl->sdl_index && sdl->sdl_index <= V_if_index) - return (ifaddr_byindex(sdl->sdl_index)); + sdl = (const struct sockaddr_dl *)addr; + if (sdl->sdl_index && sdl->sdl_index <= V_if_index) { + ifp = ifnet_byindex(sdl->sdl_index); + if (ifp == NULL) + return (NULL); + + return (ifp->if_addr); + } } /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202203071745.227HjrI7012429>