Hi,
To fix the KASSERT(la != NULL) we have to protect the rt_llinfo
with a mutex. The idea is to keep rt_llinfo and RTF_LLINFO consistent.
Also do not put the mutex in the fast path.
ok?
bluhm
Index: netinet/if_ether.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/if_ether.c,v
retrieving revision 1.250
diff -u -p -r1.250 if_ether.c
--- netinet/if_ether.c 27 Jun 2022 20:47:10 -0000 1.250
+++ netinet/if_ether.c 28 Jun 2022 14:00:12 -0000
@@ -101,6 +101,8 @@ void arpreply(struct ifnet *, struct mbu
unsigned int);
struct niqueue arpinq = NIQUEUE_INITIALIZER(50, NETISR_ARP);
+
+/* llinfo_arp live time, rt_llinfo and RTF_LLINFO are protected by arp_mtx */
struct mutex arp_mtx = MUTEX_INITIALIZER(IPL_SOFTNET);
LIST_HEAD(, llinfo_arp) arp_list; /* [mN] list of all llinfo_arp structures */
@@ -155,7 +157,7 @@ void
arp_rtrequest(struct ifnet *ifp, int req, struct rtentry *rt)
{
struct sockaddr *gate = rt->rt_gateway;
- struct llinfo_arp *la = (struct llinfo_arp *)rt->rt_llinfo;
+ struct llinfo_arp *la;
time_t uptime;
NET_ASSERT_LOCKED();
@@ -171,7 +173,7 @@ arp_rtrequest(struct ifnet *ifp, int req
rt->rt_expire = 0;
break;
}
- if ((rt->rt_flags & RTF_LOCAL) && !la)
+ if ((rt->rt_flags & RTF_LOCAL) && rt->rt_llinfo == NULL)
rt->rt_expire = 0;
/*
* Announce a new entry if requested or warn the user
@@ -192,44 +194,54 @@ arp_rtrequest(struct ifnet *ifp, int req
}
satosdl(gate)->sdl_type = ifp->if_type;
satosdl(gate)->sdl_index = ifp->if_index;
- if (la != NULL)
- break; /* This happens on a route change */
/*
* Case 2: This route may come from cloning, or a manual route
* add with a LL address.
*/
la = pool_get(&arp_pool, PR_NOWAIT | PR_ZERO);
- rt->rt_llinfo = (caddr_t)la;
if (la == NULL) {
log(LOG_DEBUG, "%s: pool get failed\n", __func__);
break;
}
+ mtx_enter(&arp_mtx);
+ if (rt->rt_llinfo != NULL) {
+ /* we lost the race, another thread has entered it */
+ mtx_leave(&arp_mtx);
+ printf("%s: llinfo exists\n", __func__);
+ pool_put(&arp_pool, la);
+ break;
+ }
mq_init(&la->la_mq, LA_HOLD_QUEUE, IPL_SOFTNET);
+ rt->rt_llinfo = (caddr_t)la;
la->la_rt = rt;
rt->rt_flags |= RTF_LLINFO;
+ LIST_INSERT_HEAD(&arp_list, la, la_list);
if ((rt->rt_flags & RTF_LOCAL) == 0)
rt->rt_expire = uptime;
- mtx_enter(&arp_mtx);
- LIST_INSERT_HEAD(&arp_list, la, la_list);
mtx_leave(&arp_mtx);
+
break;
case RTM_DELETE:
- if (la == NULL)
- break;
mtx_enter(&arp_mtx);
+ la = (struct llinfo_arp *)rt->rt_llinfo;
+ if (la == NULL) {
+ /* we lost the race, another thread has removed it */
+ mtx_leave(&arp_mtx);
+ printf("%s: llinfo missing\n", __func__);
+ break;
+ }
LIST_REMOVE(la, la_list);
- mtx_leave(&arp_mtx);
rt->rt_llinfo = NULL;
rt->rt_flags &= ~RTF_LLINFO;
atomic_sub_int(&la_hold_total, mq_purge(&la->la_mq));
+ mtx_leave(&arp_mtx);
+
pool_put(&arp_pool, la);
break;
case RTM_INVALIDATE:
- if (la == NULL)
- break;
if (!ISSET(rt->rt_flags, RTF_LOCAL))
arpinvalidate(rt);
break;
@@ -363,8 +375,6 @@ arpresolve(struct ifnet *ifp, struct rte
goto bad;
}
- la = (struct llinfo_arp *)rt->rt_llinfo;
- KASSERT(la != NULL);
/*
* Check the address family and length is valid, the address
@@ -372,13 +382,27 @@ arpresolve(struct ifnet *ifp, struct rte
*/
if ((rt->rt_expire == 0 || rt->rt_expire > uptime) &&
sdl->sdl_family == AF_LINK && sdl->sdl_alen != 0) {
+ int refresh = 0;
+
memcpy(desten, LLADDR(sdl), sdl->sdl_alen);
/* refresh ARP entry when timeout gets close */
if (rt->rt_expire != 0 &&
- rt->rt_expire - arpt_keep / 8 < uptime &&
- la->la_refreshed + 30 < uptime) {
- la->la_refreshed = uptime;
+ rt->rt_expire - arpt_keep / 8 < uptime) {
+
+ mtx_enter(&arp_mtx);
+ if (ISSET(rt->rt_flags, RTF_LLINFO)) {
+ la = (struct llinfo_arp *)rt->rt_llinfo;
+ KASSERT(la != NULL);
+
+ if (la->la_refreshed + 30 < uptime) {
+ la->la_refreshed = uptime;
+ refresh = 1;
+ }
+ }
+ mtx_leave(&arp_mtx);
+ }
+ if (refresh) {
arprequest(ifp,
&satosin(rt->rt_ifa->ifa_addr)->sin_addr.s_addr,
&satosin(dst)->sin_addr.s_addr,
@@ -724,12 +748,19 @@ arpcache(struct ifnet *ifp, struct ether
void
arpinvalidate(struct rtentry *rt)
{
- struct llinfo_arp *la = (struct llinfo_arp *)rt->rt_llinfo;
+ struct llinfo_arp *la;
struct sockaddr_dl *sdl = satosdl(rt->rt_gateway);
+ mtx_enter(&arp_mtx);
+ la = (struct llinfo_arp *)rt->rt_llinfo;
+ if (la == NULL) {
+ mtx_leave(&arp_mtx);
+ return;
+ }
atomic_sub_int(&la_hold_total, mq_purge(&la->la_mq));
sdl->sdl_alen = 0;
la->la_asked = 0;
+ mtx_leave(&arp_mtx);
}
/*
