On Tue, Aug 24, 2021 at 08:07:15PM -0400, Daniel Jakots wrote:
> Hi,
>
> Most of the time when I do `ifconfig wgX destroy` the machine stops
> responding. This has been forever. I had one a few days so I finally got
> a look at the console. This happened on a snapshot from 2021-08-14 but
> as I said, it's not a regression afaict.
>
Hello Daniel,
I see this assertion wrong. if_get(9) returns NULL here if the interface
is going to die and already unlinked from the stack, but the routing
table entries are not yet removed by in{,6}_ifdetach(). So I propose to
return ESRCH and let concurrent if_detach() to follow it's way.
Also the following "ifp != NULL" assertions in rtm_output() are wrong
and kernel lock serialization doesn't work here because we are grabbing
netlock before if_get(9). We have the same case here: the returned NULL
means the requested interface is going to die and there is no reason to
perform ifp->if_rtrequest().
Does the diff below helps? The panic should gone, but I'm interesting
about the routing table consistency.
Index: sys/net/rtsock.c
===================================================================
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.319
diff -u -p -r1.319 rtsock.c
--- sys/net/rtsock.c 23 Jun 2021 16:10:45 -0000 1.319
+++ sys/net/rtsock.c 25 Aug 2021 09:26:19 -0000
@@ -970,9 +970,13 @@ rtm_output(struct rt_msghdr *rtm, struct
break;
}
- /* Detaching an interface requires the KERNEL_LOCK(). */
ifp = if_get(rt->rt_ifidx);
- KASSERT(ifp != NULL);
+ if (ifp == NULL) {
+ rtfree(rt);
+ rt = NULL;
+ error = ESRCH;
+ break;
+ }
/*
* Invalidate the cache of automagically created and
@@ -1084,10 +1088,11 @@ rtm_output(struct rt_msghdr *rtm, struct
ifa = info->rti_ifa;
if (rt->rt_ifa != ifa) {
ifp = if_get(rt->rt_ifidx);
- KASSERT(ifp != NULL);
- ifp->if_rtrequest(ifp, RTM_DELETE, rt);
- ifafree(rt->rt_ifa);
- if_put(ifp);
+ if (ifp != NULL) {
+ ifp->if_rtrequest(ifp, RTM_DELETE, rt);
+ ifafree(rt->rt_ifa);
+ if_put(ifp);
+ }
ifa->ifa_refcnt++;
rt->rt_ifa = ifa;
@@ -1153,9 +1158,10 @@ change:
rtm_setmetrics(rtm->rtm_inits, &rtm->rtm_rmx, &rt->rt_rmx);
ifp = if_get(rt->rt_ifidx);
- KASSERT(ifp != NULL);
- ifp->if_rtrequest(ifp, RTM_ADD, rt);
- if_put(ifp);
+ if (ifp != NULL) {
+ ifp->if_rtrequest(ifp, RTM_ADD, rt);
+ if_put(ifp);
+ }
if (info->rti_info[RTAX_LABEL] != NULL) {
char *rtlabel = ((struct sockaddr_rtlabel *)
