Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys Fix deadlock between llentry timers and destruction of l...
details: https://anonhg.NetBSD.org/src/rev/e0c593c45f0b
branches: trunk
changeset: 819878:e0c593c45f0b
user: ozaki-r <ozaki-r%NetBSD.org@localhost>
date: Wed Dec 21 08:47:02 2016 +0000
description:
Fix deadlock between llentry timers and destruction of llentry
llentry timer (of nd6) holds both llentry's lock and softnet_lock.
A caller also holds them and calls callout_halt to wait for the
timer to quit. However we can pass only one lock to callout_halt,
so passing either of them can cause a deadlock. Fix it by avoid
calling callout_halt without holding llentry's lock.
BTW in the first place we cannot pass llentry's lock to callout_halt
because it's a rwlock...
diffstat:
sys/net/if_llatbl.c | 29 ++++++++++++++++++++++++++---
sys/net/if_llatbl.h | 3 ++-
sys/netinet6/in6.c | 32 +++++++++++++++++++++++---------
sys/netinet6/nd6.c | 42 ++++++++++++++++--------------------------
4 files changed, 67 insertions(+), 39 deletions(-)
diffs (220 lines):
diff -r 0acd46177744 -r e0c593c45f0b sys/net/if_llatbl.c
--- a/sys/net/if_llatbl.c Wed Dec 21 07:02:16 2016 +0000
+++ b/sys/net/if_llatbl.c Wed Dec 21 08:47:02 2016 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if_llatbl.c,v 1.15 2016/10/11 12:32:30 roy Exp $ */
+/* $NetBSD: if_llatbl.c,v 1.16 2016/12/21 08:47:02 ozaki-r Exp $ */
/*
* Copyright (c) 2004 Luigi Rizzo, Alessandro Cerri. All rights reserved.
* Copyright (c) 2004-2008 Qing Li. All rights reserved.
@@ -424,8 +424,24 @@
IF_AFDATA_WUNLOCK(llt->llt_ifp);
LIST_FOREACH_SAFE(lle, &dchain, lle_chain, next) {
- if (callout_halt(&lle->la_timer, &lle->lle_lock))
- LLE_REMREF(lle);
+ /*
+ * We need to release the lock here to lle_timer proceeds;
+ * lle_timer should stop immediately if LLE_LINKED isn't set.
+ * Note that we cannot pass lle->lle_lock to callout_halt
+ * because it's a rwlock.
+ */
+ LLE_ADDREF(lle);
+ LLE_WUNLOCK(lle);
+#ifdef NET_MPSAFE
+ callout_halt(&lle->la_timer, NULL);
+#else
+ if (mutex_owned(softnet_lock))
+ callout_halt(&lle->la_timer, softnet_lock);
+ else
+ callout_halt(&lle->la_timer, NULL);
+#endif
+ LLE_WLOCK(lle);
+ LLE_REMREF(lle);
llentry_free(lle);
}
@@ -557,6 +573,13 @@
}
void
+lltable_free_entry(struct lltable *llt, struct llentry *lle)
+{
+
+ llt->llt_free_entry(llt, lle);
+}
+
+void
lltable_fill_sa_entry(const struct llentry *lle, struct sockaddr *sa)
{
struct lltable *llt;
diff -r 0acd46177744 -r e0c593c45f0b sys/net/if_llatbl.h
--- a/sys/net/if_llatbl.h Wed Dec 21 07:02:16 2016 +0000
+++ b/sys/net/if_llatbl.h Wed Dec 21 08:47:02 2016 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if_llatbl.h,v 1.9 2016/04/04 07:37:07 ozaki-r Exp $ */
+/* $NetBSD: if_llatbl.h,v 1.10 2016/12/21 08:47:02 ozaki-r Exp $ */
/*
* Copyright (c) 2004 Luigi Rizzo, Alessandro Cerri. All rights reserved.
* Copyright (c) 2004-2008 Qing Li. All rights reserved.
@@ -273,6 +273,7 @@
const void *paddr);
void lltable_link_entry(struct lltable *llt, struct llentry *lle);
void lltable_unlink_entry(struct lltable *llt, struct llentry *lle);
+void lltable_free_entry(struct lltable *llt, struct llentry *lle);
void lltable_fill_sa_entry(const struct llentry *lle, struct sockaddr *sa);
struct ifnet *lltable_get_ifp(const struct lltable *llt);
int lltable_get_af(const struct lltable *llt);
diff -r 0acd46177744 -r e0c593c45f0b sys/netinet6/in6.c
--- a/sys/netinet6/in6.c Wed Dec 21 07:02:16 2016 +0000
+++ b/sys/netinet6/in6.c Wed Dec 21 08:47:02 2016 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: in6.c,v 1.225 2016/12/19 07:51:34 ozaki-r Exp $ */
+/* $NetBSD: in6.c,v 1.226 2016/12/21 08:47:02 ozaki-r Exp $ */
/* $KAME: in6.c,v 1.198 2001/07/18 09:12:38 itojun Exp $ */
/*
@@ -62,7 +62,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: in6.c,v 1.225 2016/12/19 07:51:34 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: in6.c,v 1.226 2016/12/21 08:47:02 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -2362,28 +2362,42 @@
static void
in6_lltable_free_entry(struct lltable *llt, struct llentry *lle)
{
- struct ifnet *ifp __diagused;
+ struct ifnet *ifp = llt->llt_ifp;
+ IF_AFDATA_WLOCK_ASSERT(ifp);
LLE_WLOCK_ASSERT(lle);
- KASSERT(llt != NULL);
/* Unlink entry from table */
if ((lle->la_flags & LLE_LINKED) != 0) {
- ifp = llt->llt_ifp;
- IF_AFDATA_WLOCK_ASSERT(ifp);
lltable_unlink_entry(llt, lle);
+ KASSERT((lle->la_flags & LLE_LINKED) == 0);
}
+ /*
+ * We need to release the lock here to lle_timer proceeds;
+ * lle_timer should stop immediately if LLE_LINKED isn't set.
+ * Note that we cannot pass lle->lle_lock to callout_halt
+ * because it's a rwlock.
+ */
+ LLE_ADDREF(lle);
+ LLE_WUNLOCK(lle);
+ IF_AFDATA_WUNLOCK(ifp);
#ifdef NET_MPSAFE
callout_halt(&lle->lle_timer, NULL);
#else
- KASSERT(mutex_owned(softnet_lock));
- callout_halt(&lle->lle_timer, softnet_lock);
+ if (mutex_owned(softnet_lock))
+ callout_halt(&lle->lle_timer, softnet_lock);
+ else
+ callout_halt(&lle->lle_timer, NULL);
#endif
+ LLE_WLOCK(lle);
LLE_REMREF(lle);
- llentry_free(lle);
+ lltable_drop_entry_queue(lle);
+ LLE_FREE_LOCKED(lle);
+
+ IF_AFDATA_WLOCK(ifp);
}
static int
diff -r 0acd46177744 -r e0c593c45f0b sys/netinet6/nd6.c
--- a/sys/netinet6/nd6.c Wed Dec 21 07:02:16 2016 +0000
+++ b/sys/netinet6/nd6.c Wed Dec 21 08:47:02 2016 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: nd6.c,v 1.221 2016/12/21 04:08:47 ozaki-r Exp $ */
+/* $NetBSD: nd6.c,v 1.222 2016/12/21 08:47:02 ozaki-r Exp $ */
/* $KAME: nd6.c,v 1.279 2002/06/08 11:16:51 itojun Exp $ */
/*
@@ -31,7 +31,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nd6.c,v 1.221 2016/12/21 04:08:47 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nd6.c,v 1.222 2016/12/21 08:47:02 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_net_mpsafe.h"
@@ -401,22 +401,18 @@
CTASSERT(sizeof(time_t) > sizeof(int));
LLE_WLOCK_ASSERT(ln);
- if (xtick < 0) {
- ln->ln_expire = 0;
- ln->ln_ntick = 0;
- callout_halt(&ln->ln_timer_ch, &ln->lle_lock);
+ KASSERT(xtick >= 0);
+
+ ln->ln_expire = time_uptime + xtick / hz;
+ LLE_ADDREF(ln);
+ if (xtick > INT_MAX) {
+ ln->ln_ntick = xtick - INT_MAX;
+ callout_reset(&ln->ln_timer_ch, INT_MAX,
+ nd6_llinfo_timer, ln);
} else {
- ln->ln_expire = time_uptime + xtick / hz;
- LLE_ADDREF(ln);
- if (xtick > INT_MAX) {
- ln->ln_ntick = xtick - INT_MAX;
- callout_reset(&ln->ln_timer_ch, INT_MAX,
- nd6_llinfo_timer, ln);
- } else {
- ln->ln_ntick = 0;
- callout_reset(&ln->ln_timer_ch, xtick,
- nd6_llinfo_timer, ln);
- }
+ ln->ln_ntick = 0;
+ callout_reset(&ln->ln_timer_ch, xtick,
+ nd6_llinfo_timer, ln);
}
}
@@ -456,6 +452,8 @@
const struct in6_addr *daddr6 = NULL;
LLE_WLOCK(ln);
+ if ((ln->la_flags & LLE_LINKED) == 0)
+ goto out;
if (ln->ln_ntick > 0) {
nd6_llinfo_settimer(ln, ln->ln_ntick);
goto out;
@@ -1208,9 +1206,6 @@
* even though it is not harmful, it was not really necessary.
*/
- /* cancel timer */
- nd6_llinfo_settimer(ln, -1);
-
if (!ip6_forwarding) {
ND6_WLOCK();
dr = nd6_defrouter_lookup(in6, ifp);
@@ -1309,12 +1304,7 @@
IF_AFDATA_LOCK(ifp);
LLE_WLOCK(ln);
- /* Guard against race with other llentry_free(). */
- if (ln->la_flags & LLE_LINKED) {
- LLE_REMREF(ln);
- llentry_free(ln);
- } else
- LLE_FREE_LOCKED(ln);
+ lltable_free_entry(LLTABLE6(ifp), ln);
IF_AFDATA_UNLOCK(ifp);
}
Home |
Main Index |
Thread Index |
Old Index