During clone/destroy stress tests on pseudo-interfaces I found a double
free easily reproducible with dhclient(8) running on top of a trunk(4).
The problem comes from trunk_ether_delmulti() which is almost identical
to carp's version except that it always free "mc".
So when you do "# ifconfig trunk0 destroy" the kernel first brings the
interface down and generates a routing message. dhclient(8) receives
this message and removes the address it's configured on trunk0 and with
it the "default multicast group". If dhclient(8) lose the race, it will
fail to remove the multicast address but it will free "mc"!
Then ifconfig's thread which was asleep in trunk_ether_purgemulti() ->
trunk_ioctl_allports() wakes up and tries to free "mc" a second time:
uvm_fault(0xd5647bd0, 0x0, 0, 1) -> e
kernel: page fault trap, code=0
Stopped at trunk_ether_purgemulti+0xc2: movl 0x104(%edx),%eax
ddb> tr
trunk_ether_purgemulti(d131b800,d131b800,0,d041d091,0) at trunk_ether_purgemult
i+0xc2
trunk_clone_destroy(d131b800,0,f3766dec,d1064e94,f3766ef4) at trunk_clone_destr
oy+0x16
ifioctl(d54d10f0,80206979,f3766e84,d54a42dc,2) at ifioctl+0x232
sys_ioctl(d54a42dc,f3766f60,f3766f80,d0566f17,d54a42dc) at sys_ioctl+0x257
syscall() at syscall+0x247
Here's a simple fix that also reduces the differences with carp's
version. ok?
Index: net/if_trunk.c
===================================================================
RCS file: /cvs/src/sys/net/if_trunk.c,v
retrieving revision 1.101
diff -u -p -r1.101 if_trunk.c
--- net/if_trunk.c 9 Jun 2015 14:50:14 -0000 1.101
+++ net/if_trunk.c 10 Jun 2015 15:30:10 -0000
@@ -857,18 +857,20 @@ trunk_ether_delmulti(struct trunk_softc
if ((error = ether_delmulti(ifr, &tr->tr_ac)) != ENETRESET)
return (error);
- if ((error = trunk_ioctl_allports(tr, SIOCDELMULTI,
- (caddr_t)ifr)) != 0) {
+ /* We no longer use this multicast address. Tell parent so. */
+ error = trunk_ioctl_allports(tr, SIOCDELMULTI, (caddr_t)ifr);
+ if (error == 0) {
+ SLIST_REMOVE(&tr->tr_mc_head, mc, trunk_mc, mc_entries);
+ free(mc, M_DEVBUF, sizeof(*mc));
+ } else {
/* XXX At least one port failed to remove the address */
if (tr->tr_ifflags & IFF_DEBUG) {
printf("%s: failed to remove multicast address "
- "on all ports\n", tr->tr_ifname);
+ "on all ports (%d)\n", tr->tr_ifname, error);
}
+ (void)ether_addmulti(ifr, &tr->tr_ac);
}
- SLIST_REMOVE(&tr->tr_mc_head, mc, trunk_mc, mc_entries);
- free(mc, M_DEVBUF, 0);
-
return (0);
}
@@ -886,7 +888,7 @@ trunk_ether_purgemulti(struct trunk_soft
trunk_ioctl_allports(tr, SIOCDELMULTI, (caddr_t)ifr);
SLIST_REMOVE(&tr->tr_mc_head, mc, trunk_mc, mc_entries);
- free(mc, M_DEVBUF, 0);
+ free(mc, M_DEVBUF, sizeof(*mc));
}
}
