Module Name: src
Committed By: ozaki-r
Date: Wed May 15 02:59:19 UTC 2019
Modified Files:
src/sys/net: if_vlan.c
src/sys/netinet: in_pcb.c ip_output.c
src/sys/netinet6: in6_pcb.c ip6_output.c
Log Message:
Get rid of IFNET_LOCK for if_mcast_op to avoid a deadlock
The IFNET_LOCK was added to avoid data races on if_flags for IFF_ALLMULTI.
Unfortunatetly it caused a deadlock instead. A known scenario causing a
deadlock is to occur the following two operations concurrently: (a) a removal of
an IP adddres assigned to an interface and (b) a manipulation of multicast
groups to the interface. The resource dependency graph is like this:
softnet_lock => IFNET_LOCK => psref_target_destroy => softint => softnet_lock
Thanks to the previous commit that avoids data races on if_flags for
IFF_ALLMULTI by another approach, we can remove IFNET_LOCK and defuse the
deadlock.
PR kern/54189
To generate a diff of this commit:
cvs rdiff -u -r1.135 -r1.136 src/sys/net/if_vlan.c
cvs rdiff -u -r1.182 -r1.183 src/sys/netinet/in_pcb.c
cvs rdiff -u -r1.311 -r1.312 src/sys/netinet/ip_output.c
cvs rdiff -u -r1.165 -r1.166 src/sys/netinet6/in6_pcb.c
cvs rdiff -u -r1.219 -r1.220 src/sys/netinet6/ip6_output.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/net/if_vlan.c
diff -u src/sys/net/if_vlan.c:1.135 src/sys/net/if_vlan.c:1.136
--- src/sys/net/if_vlan.c:1.135 Fri Apr 26 11:51:56 2019
+++ src/sys/net/if_vlan.c Wed May 15 02:59:18 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: if_vlan.c,v 1.135 2019/04/26 11:51:56 pgoyette Exp $ */
+/* $NetBSD: if_vlan.c,v 1.136 2019/05/15 02:59:18 ozaki-r Exp $ */
/*
* Copyright (c) 2000, 2001 The NetBSD Foundation, Inc.
@@ -78,7 +78,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_vlan.c,v 1.135 2019/04/26 11:51:56 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_vlan.c,v 1.136 2019/05/15 02:59:18 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -1197,9 +1197,7 @@ vlan_ether_addmulti(struct ifvlan *ifv,
mib = ifv->ifv_mib;
KERNEL_LOCK_UNLESS_IFP_MPSAFE(mib->ifvm_p);
- IFNET_LOCK(mib->ifvm_p);
error = if_mcast_op(mib->ifvm_p, SIOCADDMULTI, sa);
- IFNET_UNLOCK(mib->ifvm_p);
KERNEL_UNLOCK_UNLESS_IFP_MPSAFE(mib->ifvm_p);
if (error != 0)
@@ -1255,9 +1253,7 @@ vlan_ether_delmulti(struct ifvlan *ifv,
/* We no longer use this multicast address. Tell parent so. */
mib = ifv->ifv_mib;
- IFNET_LOCK(mib->ifvm_p);
error = if_mcast_op(mib->ifvm_p, SIOCDELMULTI, sa);
- IFNET_UNLOCK(mib->ifvm_p);
if (error == 0) {
/* And forget about this address. */
@@ -1287,10 +1283,8 @@ vlan_ether_purgemulti(struct ifvlan *ifv
}
while ((mc = LIST_FIRST(&ifv->ifv_mc_listhead)) != NULL) {
- IFNET_LOCK(mib->ifvm_p);
(void)if_mcast_op(mib->ifvm_p, SIOCDELMULTI,
sstocsa(&mc->mc_addr));
- IFNET_UNLOCK(mib->ifvm_p);
LIST_REMOVE(mc, mc_entries);
free(mc, M_DEVBUF);
}
Index: src/sys/netinet/in_pcb.c
diff -u src/sys/netinet/in_pcb.c:1.182 src/sys/netinet/in_pcb.c:1.183
--- src/sys/netinet/in_pcb.c:1.182 Tue Feb 27 14:44:10 2018
+++ src/sys/netinet/in_pcb.c Wed May 15 02:59:18 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: in_pcb.c,v 1.182 2018/02/27 14:44:10 maxv Exp $ */
+/* $NetBSD: in_pcb.c,v 1.183 2019/05/15 02:59:18 ozaki-r Exp $ */
/*
* Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@@ -93,7 +93,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: in_pcb.c,v 1.182 2018/02/27 14:44:10 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: in_pcb.c,v 1.183 2019/05/15 02:59:18 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -779,9 +779,7 @@ in_pcbpurgeif0(struct inpcbtable *table,
}
/* IFNET_LOCK must be taken after solock */
- IFNET_LOCK(ifp);
in_purgeifmcast(inp->inp_moptions, ifp);
- IFNET_UNLOCK(ifp);
if (need_unlock)
inp_unlock(inp);
Index: src/sys/netinet/ip_output.c
diff -u src/sys/netinet/ip_output.c:1.311 src/sys/netinet/ip_output.c:1.312
--- src/sys/netinet/ip_output.c:1.311 Mon May 13 07:47:59 2019
+++ src/sys/netinet/ip_output.c Wed May 15 02:59:18 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: ip_output.c,v 1.311 2019/05/13 07:47:59 ozaki-r Exp $ */
+/* $NetBSD: ip_output.c,v 1.312 2019/05/15 02:59:18 ozaki-r Exp $ */
/*
* Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@@ -91,7 +91,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ip_output.c,v 1.311 2019/05/13 07:47:59 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ip_output.c,v 1.312 2019/05/15 02:59:18 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -1837,9 +1837,7 @@ ip_add_membership(struct ip_moptions *im
* Everything looks good; add a new record to the multicast
* address list for the given interface.
*/
- IFNET_LOCK(ifp);
imo->imo_membership[i] = in_addmulti(&ia, ifp);
- IFNET_UNLOCK(ifp);
if (imo->imo_membership[i] == NULL) {
error = ENOBUFS;
goto out;
@@ -1899,10 +1897,7 @@ ip_drop_membership(struct ip_moptions *i
* Give up the multicast address record to which the
* membership points.
*/
- struct ifnet *inm_ifp = imo->imo_membership[i]->inm_ifp;
- IFNET_LOCK(inm_ifp);
in_delmulti(imo->imo_membership[i]);
- IFNET_UNLOCK(inm_ifp);
/*
* Remove the gap in the membership array.
@@ -2097,11 +2092,8 @@ ip_freemoptions(struct ip_moptions *imo)
if (imo != NULL) {
for (i = 0; i < imo->imo_num_memberships; ++i) {
struct in_multi *inm = imo->imo_membership[i];
- struct ifnet *ifp = inm->inm_ifp;
- IFNET_LOCK(ifp);
in_delmulti(inm);
/* ifp should not leave thanks to solock */
- IFNET_UNLOCK(ifp);
}
kmem_intr_free(imo, sizeof(*imo));
Index: src/sys/netinet6/in6_pcb.c
diff -u src/sys/netinet6/in6_pcb.c:1.165 src/sys/netinet6/in6_pcb.c:1.166
--- src/sys/netinet6/in6_pcb.c:1.165 Tue Feb 27 14:44:10 2018
+++ src/sys/netinet6/in6_pcb.c Wed May 15 02:59:18 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: in6_pcb.c,v 1.165 2018/02/27 14:44:10 maxv Exp $ */
+/* $NetBSD: in6_pcb.c,v 1.166 2019/05/15 02:59:18 ozaki-r Exp $ */
/* $KAME: in6_pcb.c,v 1.84 2001/02/08 18:02:08 itojun Exp $ */
/*
@@ -62,7 +62,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: in6_pcb.c,v 1.165 2018/02/27 14:44:10 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: in6_pcb.c,v 1.166 2019/05/15 02:59:18 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -877,17 +877,13 @@ in6_pcbpurgeif0(struct inpcbtable *table
i6mm_chain, nimm) {
if (imm->i6mm_maddr->in6m_ifp == ifp) {
LIST_REMOVE(imm, i6mm_chain);
- IFNET_LOCK(ifp);
in6_leavegroup(imm);
- IFNET_UNLOCK(ifp);
}
}
}
/* IFNET_LOCK must be taken after solock */
- IFNET_LOCK(ifp);
in_purgeifmcast(in6p->in6p_v4moptions, ifp);
- IFNET_UNLOCK(ifp);
if (need_unlock)
in6p_unlock(in6p);
Index: src/sys/netinet6/ip6_output.c
diff -u src/sys/netinet6/ip6_output.c:1.219 src/sys/netinet6/ip6_output.c:1.220
--- src/sys/netinet6/ip6_output.c:1.219 Mon May 13 07:47:59 2019
+++ src/sys/netinet6/ip6_output.c Wed May 15 02:59:18 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: ip6_output.c,v 1.219 2019/05/13 07:47:59 ozaki-r Exp $ */
+/* $NetBSD: ip6_output.c,v 1.220 2019/05/15 02:59:18 ozaki-r Exp $ */
/* $KAME: ip6_output.c,v 1.172 2001/03/25 09:55:56 itojun Exp $ */
/*
@@ -62,7 +62,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ip6_output.c,v 1.219 2019/05/13 07:47:59 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ip6_output.c,v 1.220 2019/05/15 02:59:18 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -2535,9 +2535,7 @@ ip6_setmoptions(const struct sockopt *so
* Everything looks good; add a new record to the multicast
* address list for the given interface.
*/
- IFNET_LOCK(ifp);
imm = in6_joingroup(ifp, &ia, &error, 0);
- IFNET_UNLOCK(ifp);
if (imm == NULL)
goto put_break;
LIST_INSERT_HEAD(&im6o->im6o_memberships, imm, i6mm_chain);
@@ -2548,7 +2546,6 @@ ip6_setmoptions(const struct sockopt *so
}
case IPV6_LEAVE_GROUP: {
- struct ifnet *in6m_ifp;
/*
* Drop a multicast group membership.
* Group must be a valid IP6 multicast address.
@@ -2630,11 +2627,8 @@ ip6_setmoptions(const struct sockopt *so
* membership points.
*/
LIST_REMOVE(imm, i6mm_chain);
- in6m_ifp = imm->i6mm_maddr->in6m_ifp;
- IFNET_LOCK(in6m_ifp);
in6_leavegroup(imm);
/* in6m_ifp should not leave thanks to in6p_lock */
- IFNET_UNLOCK(in6m_ifp);
break;
}
@@ -2715,15 +2709,8 @@ ip6_freemoptions(struct ip6_moptions *im
/* The owner of im6o (in6p) should be protected by solock */
LIST_FOREACH_SAFE(imm, &im6o->im6o_memberships, i6mm_chain, nimm) {
- struct ifnet *ifp;
-
LIST_REMOVE(imm, i6mm_chain);
-
- ifp = imm->i6mm_maddr->in6m_ifp;
- IFNET_LOCK(ifp);
in6_leavegroup(imm);
- /* ifp should not leave thanks to solock */
- IFNET_UNLOCK(ifp);
}
free(im6o, M_IPMOPTS);
}
