From nobody Wed Oct 13 17:10:11 2021 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id D736F17FA165; Wed, 13 Oct 2021 17:10:11 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HTzYb5jdBz3Pyj; Wed, 13 Oct 2021 17:10:11 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id A39DF2B998; Wed, 13 Oct 2021 17:10:11 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 19DHABvw092103; Wed, 13 Oct 2021 17:10:11 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 19DHABY0092091; Wed, 13 Oct 2021 17:10:11 GMT (envelope-from git) Date: Wed, 13 Oct 2021 17:10:11 GMT Message-Id: <202110131710.19DHABY0092091@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Gleb Smirnoff Subject: git: 2144431c1152 - main - Remove in_ifaddr_lock acquisiton to access in_ifaddrhead. List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: glebius X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 2144431c11529d1107f4440a5fe57559fb20002c Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by glebius: URL: https://cgit.FreeBSD.org/src/commit/?id=2144431c11529d1107f4440a5fe57559fb20002c commit 2144431c11529d1107f4440a5fe57559fb20002c Author: Gleb Smirnoff AuthorDate: 2021-10-08 19:56:24 +0000 Commit: Gleb Smirnoff CommitDate: 2021-10-13 17:04:46 +0000 Remove in_ifaddr_lock acquisiton to access in_ifaddrhead. An IPv4 address is embedded into an ifaddr which is freed via epoch. And the in_ifaddrhead is already a CK list. Use the network epoch to protect against use after free. Next step would be to CK-ify the in_addr hash and get rid of the... Reviewed by: melifaro Differential Revision: https://reviews.freebsd.org/D32434 --- sys/net/if_stf.c | 4 ---- sys/netinet/if_ether.c | 6 +----- sys/netinet/igmp.c | 10 +++------- sys/netinet/in.c | 27 +++++++++++---------------- sys/netinet/in_mcast.c | 6 ++---- sys/netinet/in_pcb.c | 8 -------- sys/netinet/in_var.h | 7 ++----- sys/netinet/ip_output.c | 4 +--- sys/netinet/raw_ip.c | 14 +++----------- 9 files changed, 23 insertions(+), 63 deletions(-) diff --git a/sys/net/if_stf.c b/sys/net/if_stf.c index 40f8a6f3a30a..f7d6758d052c 100644 --- a/sys/net/if_stf.c +++ b/sys/net/if_stf.c @@ -530,7 +530,6 @@ isrfc1918addr(struct in_addr *in) static int stf_checkaddr4(struct stf_softc *sc, struct in_addr *in, struct ifnet *inifp) { - struct rm_priotracker in_ifa_tracker; struct in_ifaddr *ia4; /* @@ -554,16 +553,13 @@ stf_checkaddr4(struct stf_softc *sc, struct in_addr *in, struct ifnet *inifp) /* * reject packets with broadcast */ - IN_IFADDR_RLOCK(&in_ifa_tracker); CK_STAILQ_FOREACH(ia4, &V_in_ifaddrhead, ia_link) { if ((ia4->ia_ifa.ifa_ifp->if_flags & IFF_BROADCAST) == 0) continue; if (in->s_addr == ia4->ia_broadaddr.sin_addr.s_addr) { - IN_IFADDR_RUNLOCK(&in_ifa_tracker); return -1; } } - IN_IFADDR_RUNLOCK(&in_ifa_tracker); /* * perform ingress filter diff --git a/sys/netinet/if_ether.c b/sys/netinet/if_ether.c index 5400f35d953f..45ce04117948 100644 --- a/sys/netinet/if_ether.c +++ b/sys/netinet/if_ether.c @@ -906,13 +906,9 @@ in_arpinput(struct mbuf *m) /* * If bridging, fall back to using any inet address. */ - IN_IFADDR_RLOCK(&in_ifa_tracker); - if (!bridged || (ia = CK_STAILQ_FIRST(&V_in_ifaddrhead)) == NULL) { - IN_IFADDR_RUNLOCK(&in_ifa_tracker); + if (!bridged || (ia = CK_STAILQ_FIRST(&V_in_ifaddrhead)) == NULL) goto drop; - } ifa_ref(&ia->ia_ifa); - IN_IFADDR_RUNLOCK(&in_ifa_tracker); match: if (!enaddr) enaddr = (u_int8_t *)IF_LLADDR(ifp); diff --git a/sys/netinet/igmp.c b/sys/netinet/igmp.c index ef0da5e5cb46..e7636330d267 100644 --- a/sys/netinet/igmp.c +++ b/sys/netinet/igmp.c @@ -63,7 +63,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include #include @@ -1259,7 +1258,6 @@ static int igmp_input_v1_report(struct ifnet *ifp, /*const*/ struct ip *ip, /*const*/ struct igmp *igmp) { - struct rm_priotracker in_ifa_tracker; struct in_ifaddr *ia; struct in_multi *inm; @@ -1282,7 +1280,7 @@ igmp_input_v1_report(struct ifnet *ifp, /*const*/ struct ip *ip, * Replace 0.0.0.0 with the subnet address if told to do so. */ if (V_igmp_recvifkludge && in_nullhost(ip->ip_src)) { - IFP_TO_IA(ifp, ia, &in_ifa_tracker); + IFP_TO_IA(ifp, ia); if (ia != NULL) ip->ip_src.s_addr = htonl(ia->ia_subnet); } @@ -1368,7 +1366,6 @@ static int igmp_input_v2_report(struct ifnet *ifp, /*const*/ struct ip *ip, /*const*/ struct igmp *igmp) { - struct rm_priotracker in_ifa_tracker; struct in_ifaddr *ia; struct in_multi *inm; @@ -1377,7 +1374,7 @@ igmp_input_v2_report(struct ifnet *ifp, /*const*/ struct ip *ip, * leave requires knowing that we are the only member of a * group. */ - IFP_TO_IA(ifp, ia, &in_ifa_tracker); + IFP_TO_IA(ifp, ia); if (ia != NULL && in_hosteq(ip->ip_src, IA_SIN(ia)->sin_addr)) { return (0); } @@ -3535,7 +3532,6 @@ out: static struct mbuf * igmp_v3_encap_report(struct ifnet *ifp, struct mbuf *m) { - struct rm_priotracker in_ifa_tracker; struct igmp_report *igmp; struct ip *ip; int hdrlen, igmpreclen; @@ -3584,7 +3580,7 @@ igmp_v3_encap_report(struct ifnet *ifp, struct mbuf *m) if (m->m_flags & M_IGMP_LOOP) { struct in_ifaddr *ia; - IFP_TO_IA(ifp, ia, &in_ifa_tracker); + IFP_TO_IA(ifp, ia); if (ia != NULL) ip->ip_src = ia->ia_addr.sin_addr; } diff --git a/sys/netinet/in.c b/sys/netinet/in.c index b51f1111b88a..aa87546be2d4 100644 --- a/sys/netinet/in.c +++ b/sys/netinet/in.c @@ -107,18 +107,16 @@ SX_SYSINIT(in_control_sx, &in_control_sx, "in_control"); int in_localaddr(struct in_addr in) { - struct rm_priotracker in_ifa_tracker; u_long i = ntohl(in.s_addr); struct in_ifaddr *ia; - IN_IFADDR_RLOCK(&in_ifa_tracker); + NET_EPOCH_ASSERT(); + CK_STAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) { - if ((i & ia->ia_subnetmask) == ia->ia_subnet) { - IN_IFADDR_RUNLOCK(&in_ifa_tracker); + if ((i & ia->ia_subnetmask) == ia->ia_subnet) return (1); - } } - IN_IFADDR_RUNLOCK(&in_ifa_tracker); + return (0); } @@ -204,12 +202,10 @@ in_localip_more(struct in_ifaddr *original_ia) struct in_ifaddr * in_findlocal(uint32_t fibnum, bool loopback_ok) { - struct rm_priotracker in_ifa_tracker; struct in_ifaddr *ia = NULL, *ia_lo = NULL; NET_EPOCH_ASSERT(); - IN_IFADDR_RLOCK(&in_ifa_tracker); CK_STAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) { uint32_t ia_fib = ia->ia_ifa.ifa_ifp->if_fib; if (!V_rt_add_addr_allfibs && (fibnum != ia_fib)) @@ -220,7 +216,6 @@ in_findlocal(uint32_t fibnum, bool loopback_ok) if (loopback_ok) ia_lo = ia; } - IN_IFADDR_RUNLOCK(&in_ifa_tracker); if (ia == NULL) ia = ia_lo; @@ -938,15 +933,15 @@ in_handle_ifaddr_route(int cmd, struct in_ifaddr *ia) static bool in_hasrtprefix(struct in_ifaddr *target) { - struct rm_priotracker in_ifa_tracker; + struct epoch_tracker et; struct in_ifaddr *ia; struct in_addr prefix, mask, p, m; bool result = false; ia_getrtprefix(target, &prefix, &mask); - IN_IFADDR_RLOCK(&in_ifa_tracker); /* Look for an existing address with the same prefix, mask, and fib */ + NET_EPOCH_ENTER(et); CK_STAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) { ia_getrtprefix(ia, &p, &m); @@ -966,7 +961,7 @@ in_hasrtprefix(struct in_ifaddr *target) break; } } - IN_IFADDR_RUNLOCK(&in_ifa_tracker); + NET_EPOCH_EXIT(et); return (result); } @@ -1042,7 +1037,7 @@ in_scrubprefixlle(struct in_ifaddr *ia, int all, u_int flags) int in_scrubprefix(struct in_ifaddr *target, u_int flags) { - struct rm_priotracker in_ifa_tracker; + struct epoch_tracker et; struct in_ifaddr *ia; struct in_addr prefix, mask, p, m; int error = 0; @@ -1080,7 +1075,7 @@ in_scrubprefix(struct in_ifaddr *target, u_int flags) return (0); } - IN_IFADDR_RLOCK(&in_ifa_tracker); + NET_EPOCH_ENTER(et); CK_STAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) { ia_getrtprefix(ia, &p, &m); @@ -1098,7 +1093,7 @@ in_scrubprefix(struct in_ifaddr *target, u_int flags) */ if ((ia->ia_flags & IFA_ROUTE) == 0) { ifa_ref(&ia->ia_ifa); - IN_IFADDR_RUNLOCK(&in_ifa_tracker); + NET_EPOCH_EXIT(et); error = in_handle_ifaddr_route(RTM_DELETE, target); if (error == 0) target->ia_flags &= ~IFA_ROUTE; @@ -1118,7 +1113,7 @@ in_scrubprefix(struct in_ifaddr *target, u_int flags) return (error); } } - IN_IFADDR_RUNLOCK(&in_ifa_tracker); + NET_EPOCH_EXIT(et); /* * remove all L2 entries on the given prefix diff --git a/sys/netinet/in_mcast.c b/sys/netinet/in_mcast.c index f0827fcf3451..ad2d7af799a5 100644 --- a/sys/netinet/in_mcast.c +++ b/sys/netinet/in_mcast.c @@ -1752,7 +1752,6 @@ inp_get_source_filters(struct inpcb *inp, struct sockopt *sopt) int inp_getmoptions(struct inpcb *inp, struct sockopt *sopt) { - struct rm_priotracker in_ifa_tracker; struct ip_mreqn mreqn; struct ip_moptions *imo; struct ifnet *ifp; @@ -1795,7 +1794,7 @@ inp_getmoptions(struct inpcb *inp, struct sockopt *sopt) mreqn.imr_ifindex = ifp->if_index; NET_EPOCH_ENTER(et); - IFP_TO_IA(ifp, ia, &in_ifa_tracker); + IFP_TO_IA(ifp, ia); if (ia != NULL) mreqn.imr_address = IA_SIN(ia)->sin_addr; @@ -1887,6 +1886,7 @@ inp_lookup_mcast_ifp(const struct inpcb *inp, struct ifnet *ifp; struct nhop_object *nh; + NET_EPOCH_ASSERT(); KASSERT(inp != NULL, ("%s: inp must not be NULL", __func__)); KASSERT(gsin->sin_family == AF_INET, ("%s: not AF_INET", __func__)); KASSERT(IN_MULTICAST(ntohl(gsin->sin_addr.s_addr)), @@ -1909,7 +1909,6 @@ inp_lookup_mcast_ifp(const struct inpcb *inp, struct ifnet *mifp; mifp = NULL; - IN_IFADDR_RLOCK(&in_ifa_tracker); CK_STAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) { mifp = ia->ia_ifp; if (!(mifp->if_flags & IFF_LOOPBACK) && @@ -1919,7 +1918,6 @@ inp_lookup_mcast_ifp(const struct inpcb *inp, break; } } - IN_IFADDR_RUNLOCK(&in_ifa_tracker); } } diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c index 04d34b022772..189f73028198 100644 --- a/sys/netinet/in_pcb.c +++ b/sys/netinet/in_pcb.c @@ -58,7 +58,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include #include @@ -1363,7 +1362,6 @@ in_pcbconnect_setup(struct inpcb *inp, struct sockaddr *nam, in_addr_t *laddrp, u_short *lportp, in_addr_t *faddrp, u_short *fportp, struct inpcb **oinpp, struct ucred *cred) { - struct rm_priotracker in_ifa_tracker; struct sockaddr_in *sin = (struct sockaddr_in *)nam; struct in_ifaddr *ia; struct inpcb *oinp; @@ -1412,20 +1410,16 @@ in_pcbconnect_setup(struct inpcb *inp, struct sockaddr *nam, * choose the broadcast address for that interface. */ if (faddr.s_addr == INADDR_ANY) { - IN_IFADDR_RLOCK(&in_ifa_tracker); faddr = IA_SIN(CK_STAILQ_FIRST(&V_in_ifaddrhead))->sin_addr; - IN_IFADDR_RUNLOCK(&in_ifa_tracker); if (cred != NULL && (error = prison_get_ip4(cred, &faddr)) != 0) return (error); } else if (faddr.s_addr == (u_long)INADDR_BROADCAST) { - IN_IFADDR_RLOCK(&in_ifa_tracker); if (CK_STAILQ_FIRST(&V_in_ifaddrhead)->ia_ifp->if_flags & IFF_BROADCAST) faddr = satosin(&CK_STAILQ_FIRST( &V_in_ifaddrhead)->ia_broadaddr)->sin_addr; - IN_IFADDR_RUNLOCK(&in_ifa_tracker); } } if (laddr.s_addr == INADDR_ANY) { @@ -1443,7 +1437,6 @@ in_pcbconnect_setup(struct inpcb *inp, struct sockaddr *nam, imo = inp->inp_moptions; if (imo->imo_multicast_ifp != NULL) { ifp = imo->imo_multicast_ifp; - IN_IFADDR_RLOCK(&in_ifa_tracker); CK_STAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) { if ((ia->ia_ifp == ifp) && (cred == NULL || @@ -1457,7 +1450,6 @@ in_pcbconnect_setup(struct inpcb *inp, struct sockaddr *nam, laddr = ia->ia_addr.sin_addr; error = 0; } - IN_IFADDR_RUNLOCK(&in_ifa_tracker); } } if (error) diff --git a/sys/netinet/in_var.h b/sys/netinet/in_var.h index b42ca00d5ae7..c33098e2c79c 100644 --- a/sys/netinet/in_var.h +++ b/sys/netinet/in_var.h @@ -166,18 +166,15 @@ do { \ * Macro for finding the internet address structure (in_ifaddr) corresponding * to a given interface (ifnet structure). */ -#define IFP_TO_IA(ifp, ia, t) \ +#define IFP_TO_IA(ifp, ia) \ /* struct ifnet *ifp; */ \ /* struct in_ifaddr *ia; */ \ - /* struct rm_priotracker *t; */ \ do { \ NET_EPOCH_ASSERT(); \ - IN_IFADDR_RLOCK((t)); \ for ((ia) = CK_STAILQ_FIRST(&V_in_ifaddrhead); \ (ia) != NULL && (ia)->ia_ifp != (ifp); \ - (ia) = CK_STAILQ_NEXT((ia), ia_link)) \ + (ia) = CK_STAILQ_NEXT((ia), ia_link)) \ continue; \ - IN_IFADDR_RUNLOCK((t)); \ } while (0) /* diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c index ad41c9df0b8c..d850cf5b5167 100644 --- a/sys/netinet/ip_output.c +++ b/sys/netinet/ip_output.c @@ -53,7 +53,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include #include @@ -321,7 +320,6 @@ ip_output(struct mbuf *m, struct mbuf *opt, struct route *ro, int flags, struct ip_moptions *imo, struct inpcb *inp) { MROUTER_RLOCK_TRACKER; - struct rm_priotracker in_ifa_tracker; struct ip *ip; struct ifnet *ifp = NULL; /* keep compiler happy */ struct mbuf *m0; @@ -463,7 +461,7 @@ again: */ ifp = imo->imo_multicast_ifp; mtu = ifp->if_mtu; - IFP_TO_IA(ifp, ia, &in_ifa_tracker); + IFP_TO_IA(ifp, ia); isbroadcast = 0; /* fool gcc */ /* Interface may have no addresses. */ if (ia != NULL) diff --git a/sys/netinet/raw_ip.c b/sys/netinet/raw_ip.c index 00c95c451966..38ab5f4a8243 100644 --- a/sys/netinet/raw_ip.c +++ b/sys/netinet/raw_ip.c @@ -50,7 +50,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include #include @@ -817,20 +816,19 @@ rip_ctloutput(struct socket *so, struct sockopt *sopt) void rip_ctlinput(int cmd, struct sockaddr *sa, void *vip) { - struct rm_priotracker in_ifa_tracker; struct in_ifaddr *ia; struct ifnet *ifp; int err; int flags; + NET_EPOCH_ASSERT(); + switch (cmd) { case PRC_IFDOWN: - IN_IFADDR_RLOCK(&in_ifa_tracker); CK_STAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) { if (ia->ia_ifa.ifa_addr == sa && (ia->ia_flags & IFA_ROUTE)) { ifa_ref(&ia->ia_ifa); - IN_IFADDR_RUNLOCK(&in_ifa_tracker); /* * in_scrubprefix() kills the interface route. */ @@ -846,22 +844,16 @@ rip_ctlinput(int cmd, struct sockaddr *sa, void *vip) break; } } - if (ia == NULL) /* If ia matched, already unlocked. */ - IN_IFADDR_RUNLOCK(&in_ifa_tracker); break; case PRC_IFUP: - IN_IFADDR_RLOCK(&in_ifa_tracker); CK_STAILQ_FOREACH(ia, &V_in_ifaddrhead, ia_link) { if (ia->ia_ifa.ifa_addr == sa) break; } - if (ia == NULL || (ia->ia_flags & IFA_ROUTE)) { - IN_IFADDR_RUNLOCK(&in_ifa_tracker); + if (ia == NULL || (ia->ia_flags & IFA_ROUTE)) return; - } ifa_ref(&ia->ia_ifa); - IN_IFADDR_RUNLOCK(&in_ifa_tracker); flags = RTF_UP; ifp = ia->ia_ifa.ifa_ifp;