git: 2144431c1152 - main - Remove in_ifaddr_lock acquisiton to access in_ifaddrhead.
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 13 Oct 2021 17:10:11 UTC
The branch main has been updated by glebius:
URL: https://cgit.FreeBSD.org/src/commit/?id=2144431c11529d1107f4440a5fe57559fb20002c
commit 2144431c11529d1107f4440a5fe57559fb20002c
Author: Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2021-10-08 19:56:24 +0000
Commit: Gleb Smirnoff <glebius@FreeBSD.org>
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 <sys/protosw.h>
#include <sys/kernel.h>
#include <sys/lock.h>
-#include <sys/rmlock.h>
#include <sys/sysctl.h>
#include <sys/ktr.h>
#include <sys/condvar.h>
@@ -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 <sys/eventhandler.h>
#include <sys/domain.h>
#include <sys/protosw.h>
-#include <sys/rmlock.h>
#include <sys/smp.h>
#include <sys/socket.h>
#include <sys/socketvar.h>
@@ -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 <sys/priv.h>
#include <sys/proc.h>
#include <sys/protosw.h>
-#include <sys/rmlock.h>
#include <sys/sdt.h>
#include <sys/socket.h>
#include <sys/socketvar.h>
@@ -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 <sys/priv.h>
#include <sys/proc.h>
#include <sys/protosw.h>
-#include <sys/rmlock.h>
#include <sys/rwlock.h>
#include <sys/signalvar.h>
#include <sys/socket.h>
@@ -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;