Use struct refcnt for interface address reference counting.

There was a crash due to use after free of the ifa although it is
ref counted.  As ifa_refcnt was a simple integer increment, there
may be a path where multiple CPUs access it concurrently.  So change
to struct refcnt which is MP safe and provides dt(4) leak debugging.
Link level address for IPsec enc(4) and various MPLS interfaces is
special.  There ifa is part of struct sc.  Use refcount anyway and
add a panic to detect use after free.
bug report stsp@; OK mvs@

8 files changed, 45 insertions, 25 deletions

diff --git a/sys/net/if_enc.c b/sys/net/if_enc.c
index 4645f3af89f..e835a8b31e3 100644
--- a/sys/net/if_enc.c
+++ b/sys/net/if_enc.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_enc.c,v 1.78 2020/12/28 14:28:50 kn Exp $	*/
+/*	$OpenBSD: if_enc.c,v 1.79 2022/08/29 07:51:45 bluhm Exp $	*/
 
 /*
  * Copyright (c) 2010 Reyk Floeter <reyk@vantronix.net>
@@ -100,6 +100,7 @@ enc_clone_create(struct if_clone *ifc, int unit)
 	 * and empty ifa of type AF_LINK for this purpose.
 	 */
 	if_alloc_sadl(ifp);
+	refcnt_init_trace(&sc->sc_ifa.ifa_refcnt, DT_REFCNT_IDX_IFADDR);
 	sc->sc_ifa.ifa_ifp = ifp;
 	sc->sc_ifa.ifa_addr = sdltosa(ifp->if_sadl);
 	sc->sc_ifa.ifa_netmask = NULL;
@@ -152,6 +153,10 @@ enc_clone_destroy(struct ifnet *ifp)
 	NET_UNLOCK();
 
 	if_detach(ifp);
+	if (refcnt_rele(&sc->sc_ifa.ifa_refcnt) == 0) {
+		panic("%s: ifa refcnt has %u refs", __func__,
+		    sc->sc_ifa.ifa_refcnt.r_refs);
+	}
 	free(sc, M_DEVBUF, sizeof(*sc));
 
 	return (0);
diff --git a/sys/net/if_mpe.c b/sys/net/if_mpe.c
index 8232b2f8209..7b84d24d5d7 100644
--- a/sys/net/if_mpe.c
+++ b/sys/net/if_mpe.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: if_mpe.c,v 1.101 2021/11/08 04:50:54 dlg Exp $ */
+/* $OpenBSD: if_mpe.c,v 1.102 2022/08/29 07:51:45 bluhm Exp $ */
 
 /*
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@spootnik.org>
@@ -128,6 +128,7 @@ mpe_clone_create(struct if_clone *ifc, int unit)
 	sc->sc_txhprio = 0;
 	sc->sc_rxhprio = IF_HDRPRIO_PACKET;
 	sc->sc_rdomain = 0;
+	refcnt_init_trace(&sc->sc_ifa.ifa_refcnt, DT_REFCNT_IDX_IFADDR);
 	sc->sc_ifa.ifa_ifp = ifp;
 	sc->sc_ifa.ifa_addr = sdltosa(ifp->if_sadl);
 	sc->sc_smpls.smpls_len = sizeof(sc->sc_smpls);
@@ -154,6 +155,10 @@ mpe_clone_destroy(struct ifnet *ifp)
 	ifq_barrier(&ifp->if_snd);
 
 	if_detach(ifp);
+	if (refcnt_rele(&sc->sc_ifa.ifa_refcnt) == 0) {
+		panic("%s: ifa refcnt has %u refs", __func__,
+		    sc->sc_ifa.ifa_refcnt.r_refs);
+	}
 	free(sc, M_DEVBUF, sizeof *sc);
 	return (0);
 }
diff --git a/sys/net/if_mpip.c b/sys/net/if_mpip.c
index a8daeeea314..4fefa7580f2 100644
--- a/sys/net/if_mpip.c
+++ b/sys/net/if_mpip.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_mpip.c,v 1.15 2021/03/26 19:00:21 kn Exp $ */
+/*	$OpenBSD: if_mpip.c,v 1.16 2022/08/29 07:51:45 bluhm Exp $ */
 
 /*
  * Copyright (c) 2015 Rafael Zalamena <rzalamena@openbsd.org>
@@ -128,6 +128,7 @@ mpip_clone_create(struct if_clone *ifc, int unit)
 	bpfattach(&ifp->if_bpf, ifp, DLT_LOOP, sizeof(uint32_t));
 #endif
 
+	refcnt_init_trace(&sc->sc_ifa.ifa_refcnt, DT_REFCNT_IDX_IFADDR);
 	sc->sc_ifa.ifa_ifp = ifp;
 	sc->sc_ifa.ifa_addr = sdltosa(ifp->if_sadl);
 
@@ -152,7 +153,10 @@ mpip_clone_destroy(struct ifnet *ifp)
 	ifq_barrier(&ifp->if_snd);
 
 	if_detach(ifp);
-
+	if (refcnt_rele(&sc->sc_ifa.ifa_refcnt) == 0) {
+		panic("%s: ifa refcnt has %u refs", __func__,
+		    sc->sc_ifa.ifa_refcnt.r_refs);
+	}
 	free(sc->sc_neighbor, M_DEVBUF, sizeof(*sc->sc_neighbor));
 	free(sc, M_DEVBUF, sizeof(*sc));
 
diff --git a/sys/net/if_mpw.c b/sys/net/if_mpw.c
index f17693dc766..2106c3e6819 100644
--- a/sys/net/if_mpw.c
+++ b/sys/net/if_mpw.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_mpw.c,v 1.62 2021/03/26 19:00:21 kn Exp $ */
+/*	$OpenBSD: if_mpw.c,v 1.63 2022/08/29 07:51:45 bluhm Exp $ */
 
 /*
  * Copyright (c) 2015 Rafael Zalamena <rzalamena@openbsd.org>
@@ -122,6 +122,7 @@ mpw_clone_create(struct if_clone *ifc, int unit)
 	sc->sc_txhprio = 0;
 	sc->sc_rxhprio = IF_HDRPRIO_PACKET;
 	sc->sc_rdomain = 0;
+	refcnt_init_trace(&sc->sc_ifa.ifa_refcnt, DT_REFCNT_IDX_IFADDR);
 	sc->sc_ifa.ifa_ifp = ifp;
 	sc->sc_ifa.ifa_addr = sdltosa(ifp->if_sadl);
 	sc->sc_smpls.smpls_len = sizeof(sc->sc_smpls);
@@ -149,7 +150,10 @@ mpw_clone_destroy(struct ifnet *ifp)
 
 	ether_ifdetach(ifp);
 	if_detach(ifp);
-
+	if (refcnt_rele(&sc->sc_ifa.ifa_refcnt) == 0) {
+		panic("%s: ifa refcnt has %u refs", __func__,
+		    sc->sc_ifa.ifa_refcnt.r_refs);
+	}
 	free(sc->sc_neighbor, M_DEVBUF, sizeof(*sc->sc_neighbor));
 	free(sc, M_DEVBUF, sizeof(*sc));
 
diff --git a/sys/net/if_pppx.c b/sys/net/if_pppx.c
index fd099d81ff0..953524b3a31 100644
--- a/sys/net/if_pppx.c
+++ b/sys/net/if_pppx.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pppx.c,v 1.121 2022/07/25 08:29:26 mvs Exp $ */
+/*	$OpenBSD: if_pppx.c,v 1.122 2022/08/29 07:51:45 bluhm Exp $ */
 
 /*
  * Copyright (c) 2010 Claudio Jeker <claudio@openbsd.org>
@@ -659,6 +659,7 @@ pppx_add_session(struct pppx_dev *pxd, struct pipex_session_req *req)
 	ifaddr.sin_addr = req->pr_ip_srcaddr;
 
 	ia = malloc(sizeof (*ia), M_IFADDR, M_WAITOK | M_ZERO);
+	refcnt_init_trace(&ia->ia_ifa.ifa_refcnt, DT_REFCNT_IDX_IFADDR);
 
 	ia->ia_addr.sin_family = AF_INET;
 	ia->ia_addr.sin_len = sizeof(struct sockaddr_in);
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 400afd319b0..394398c5c5c 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_var.h,v 1.114 2021/02/20 04:55:52 dlg Exp $	*/
+/*	$OpenBSD: if_var.h,v 1.115 2022/08/29 07:51:45 bluhm Exp $	*/
 /*	$NetBSD: if.h,v 1.23 1996/05/07 02:40:27 thorpej Exp $	*/
 
 /*
@@ -242,7 +242,7 @@ struct ifaddr {
 	struct	ifnet *ifa_ifp;		/* back-pointer to interface */
 	TAILQ_ENTRY(ifaddr) ifa_list;	/* list of addresses for interface */
 	u_int	ifa_flags;		/* interface flags, see below */
-	u_int	ifa_refcnt;		/* number of `rt_ifa` references */
+	struct	refcnt ifa_refcnt;	/* number of `rt_ifa` references */
 	int	ifa_metric;		/* cost of going out this interface */
 };
 
@@ -333,6 +333,7 @@ int	p2p_bpf_mtap(caddr_t, const struct mbuf *, u_int);
 struct	ifaddr *ifa_ifwithaddr(struct sockaddr *, u_int);
 struct	ifaddr *ifa_ifwithdstaddr(struct sockaddr *, u_int);
 struct	ifaddr *ifaof_ifpforaddr(struct sockaddr *, struct ifnet *);
+struct	ifaddr *ifaref(struct ifaddr *);
 void	ifafree(struct ifaddr *);
 
 int	if_isconnected(const struct ifnet *, unsigned int);
diff --git a/sys/net/route.c b/sys/net/route.c
index 7d1aeeca26a..6f5cb089aec 100644
--- a/sys/net/route.c
+++ b/sys/net/route.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: route.c,v 1.413 2022/07/28 22:19:09 bluhm Exp $	*/
+/*	$OpenBSD: route.c,v 1.414 2022/08/29 07:51:45 bluhm Exp $	*/
 /*	$NetBSD: route.c,v 1.14 1996/02/13 22:00:46 christos Exp $	*/
 
 /*
@@ -146,7 +146,6 @@ extern unsigned int	rtmap_limit;
 
 struct cpumem *		rtcounters;
 int			rttrash;	/* routes not in table but not freed */
-int			ifatrash;	/* ifas not in ifp list but not free */
 
 struct pool	rtentry_pool;		/* pool for rtentry structures */
 struct pool	rttimer_pool;		/* pool for rttimer structures */
@@ -512,16 +511,19 @@ rtfree(struct rtentry *rt)
 	pool_put(&rtentry_pool, rt);
 }
 
+struct ifaddr *
+ifaref(struct ifaddr *ifa)
+{
+	refcnt_take(&ifa->ifa_refcnt);
+	return ifa;
+}
+
 void
 ifafree(struct ifaddr *ifa)
 {
-	if (ifa == NULL)
-		panic("ifafree");
-	if (ifa->ifa_refcnt == 0) {
-		ifatrash--;
-		free(ifa, M_IFADDR, 0);
-	} else
-		ifa->ifa_refcnt--;
+	if (refcnt_rele(&ifa->ifa_refcnt) == 0)
+		return;
+	free(ifa, M_IFADDR, 0);
 }
 
 /*
@@ -901,8 +903,7 @@ rtrequest(int req, struct rt_addrinfo *info, u_int8_t prio,
 			rt_mpls_clear(rt);
 #endif
 
-		ifa->ifa_refcnt++;
-		rt->rt_ifa = ifa;
+		rt->rt_ifa = ifaref(ifa);
 		rt->rt_ifidx = ifp->if_index;
 		/*
 		 * Copy metrics and a back pointer from the cloned
@@ -1857,8 +1858,8 @@ db_print_ifa(struct ifaddr *ifa)
 	db_print_sa(ifa->ifa_dstaddr);
 	db_printf("  ifa_mask=");
 	db_print_sa(ifa->ifa_netmask);
-	db_printf("  flags=0x%x, refcnt=%d, metric=%d\n",
-	    ifa->ifa_flags, ifa->ifa_refcnt, ifa->ifa_metric);
+	db_printf("  flags=0x%x, refcnt=%u, metric=%d\n",
+	    ifa->ifa_flags, ifa->ifa_refcnt.r_refs, ifa->ifa_metric);
 }
 
 /*
diff --git a/sys/net/rtsock.c b/sys/net/rtsock.c
index 5e54c1c94fd..c7476a5b359 100644
--- a/sys/net/rtsock.c
+++ b/sys/net/rtsock.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rtsock.c,v 1.346 2022/08/28 21:35:12 mvs Exp $	*/
+/*	$OpenBSD: rtsock.c,v 1.347 2022/08/29 07:51:45 bluhm Exp $	*/
 /*	$NetBSD: rtsock.c,v 1.18 1996/03/29 00:32:10 cgd Exp $	*/
 
 /*
@@ -1145,8 +1145,7 @@ rtm_output(struct rt_msghdr *rtm, struct rtentry **prt,
 				ifp->if_rtrequest(ifp, RTM_DELETE, rt);
 				ifafree(rt->rt_ifa);
 
-				ifa->ifa_refcnt++;
-				rt->rt_ifa = ifa;
+				rt->rt_ifa = ifaref(ifa);
 				rt->rt_ifidx = ifa->ifa_ifp->if_index;
 				/* recheck link state after ifp change */
 				rt_if_linkstate_change(rt, ifa->ifa_ifp,