Thanks to awolk@ hacking on USB wireless we found a lot of new races in
the network stack.
Passing and ``ifp'' pointer to a task is *not* safe. If the task
sleeps, then another thread my start executing if_detach() freeing the
memory associated to ``ifp''. And task do sleep, that's why we use
them in the first place.
So we should always pass and ifp index like I did with the
if_input_process() task. Diff attached fix awolk@'s first panic.
ok?
Index: net/if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.441
diff -u -p -r1.441 if.c
--- net/if.c 4 Sep 2016 10:32:01 -0000 1.441
+++ net/if.c 4 Sep 2016 11:03:37 -0000
@@ -400,6 +400,8 @@ if_map_dtor(void *null, void *m)
void
if_attachsetup(struct ifnet *ifp)
{
+ unsigned long ifidx;
+
TAILQ_INIT(&ifp->if_groups);
if_addgroup(ifp, IFG_ALL);
@@ -409,18 +411,19 @@ if_attachsetup(struct ifnet *ifp)
pfi_attach_ifnet(ifp);
#endif
- task_set(ifp->if_watchdogtask, if_watchdog_task, ifp);
timeout_set(ifp->if_slowtimo, if_slowtimo, ifp);
if_slowtimo(ifp);
- task_set(ifp->if_linkstatetask, if_linkstate, ifp);
-
if_idxmap_insert(ifp);
KASSERT(if_get(0) == NULL);
+ ifidx = ifp->if_index;
+
mq_init(&ifp->if_inputqueue, 8192, IPL_NET);
- task_set(ifp->if_inputtask, if_input_process,
- (void *)(unsigned long)ifp->if_index);
+ task_set(ifp->if_inputtask, if_input_process, (void *)ifidx);
+ task_set(ifp->if_watchdogtask, if_watchdog_task, (void *)ifidx);
+ task_set(ifp->if_linkstatetask, if_linkstate, (void *)ifidx);
+
/* Announce the interface. */
rt_ifannouncemsg(ifp, IFAN_ARRIVAL);
@@ -1482,11 +1485,16 @@ if_up(struct ifnet *ifp)
* a link-state transition.
*/
void
-if_linkstate(void *xifp)
+if_linkstate(void *xifidx)
{
- struct ifnet *ifp = xifp;
+ unsigned int ifidx = (unsigned long)xifidx;
+ struct ifnet *ifp;
int s;
+ ifp = if_get(ifidx);
+ if (ifp == NULL)
+ return;
+
s = splsoftnet();
rt_ifmsg(ifp);
#ifndef SMALL_KERNEL
@@ -1494,6 +1502,8 @@ if_linkstate(void *xifp)
#endif
dohooks(ifp->if_linkstatehooks, 0);
splx(s);
+
+ if_put(ifp);
}
/*
@@ -1525,15 +1535,22 @@ if_slowtimo(void *arg)
}
void
-if_watchdog_task(void *arg)
+if_watchdog_task(void *xifidx)
{
- struct ifnet *ifp = arg;
+ unsigned int ifidx = (unsigned long)xifidx;
+ struct ifnet *ifp;
int s;
+ ifp = if_get(ifidx);
+ if (ifp == NULL)
+ return;
+
s = splnet();
if (ifp->if_watchdog)
(*ifp->if_watchdog)(ifp);
splx(s);
+
+ if_put(ifp);
}
/*
