Skip site navigation (1)Skip section navigation (2)
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>