Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys Get rid of IFNET_LOCK for if_mcast_op to avoid a deadlock
details: https://anonhg.NetBSD.org/src/rev/ddef31c11994
branches: trunk
changeset: 451309:ddef31c11994
user: ozaki-r <ozaki-r%NetBSD.org@localhost>
date: Wed May 15 02:59:18 2019 +0000
description:
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
diffstat:
sys/net/if_vlan.c | 10 ++--------
sys/netinet/in_pcb.c | 6 ++----
sys/netinet/ip_output.c | 12 ++----------
sys/netinet6/in6_pcb.c | 8 ++------
sys/netinet6/ip6_output.c | 17 ++---------------
5 files changed, 10 insertions(+), 43 deletions(-)
diffs (228 lines):
diff -r 126bc6375f8a -r ddef31c11994 sys/net/if_vlan.c
--- a/sys/net/if_vlan.c Wed May 15 02:56:47 2019 +0000
+++ b/sys/net/if_vlan.c Wed May 15 02:59:18 2019 +0000
@@ -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 @@
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 @@
/* 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 @@
}
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);
}
diff -r 126bc6375f8a -r ddef31c11994 sys/netinet/in_pcb.c
--- a/sys/netinet/in_pcb.c Wed May 15 02:56:47 2019 +0000
+++ b/sys/netinet/in_pcb.c Wed May 15 02:59:18 2019 +0000
@@ -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 @@
}
/* 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);
diff -r 126bc6375f8a -r ddef31c11994 sys/netinet/ip_output.c
--- a/sys/netinet/ip_output.c Wed May 15 02:56:47 2019 +0000
+++ b/sys/netinet/ip_output.c Wed May 15 02:59:18 2019 +0000
@@ -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 @@
* 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 @@
* 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 @@
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));
diff -r 126bc6375f8a -r ddef31c11994 sys/netinet6/in6_pcb.c
--- a/sys/netinet6/in6_pcb.c Wed May 15 02:56:47 2019 +0000
+++ b/sys/netinet6/in6_pcb.c Wed May 15 02:59:18 2019 +0000
@@ -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 @@
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);
diff -r 126bc6375f8a -r ddef31c11994 sys/netinet6/ip6_output.c
--- a/sys/netinet6/ip6_output.c Wed May 15 02:56:47 2019 +0000
+++ b/sys/netinet6/ip6_output.c Wed May 15 02:59:18 2019 +0000
@@ -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 @@
* 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 @@
}
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 @@
* 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 @@
/* 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);
}
Home |
Main Index |
Thread Index |
Old Index