Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys Fix reference leaks of llentry
details: https://anonhg.NetBSD.org/src/rev/f37be11258eb
branches: trunk
changeset: 830962:f37be11258eb
user: ozaki-r <ozaki-r%NetBSD.org@localhost>
date: Tue Mar 06 07:24:01 2018 +0000
description:
Fix reference leaks of llentry
callout_reset and callout_halt can cancel a pending callout without telling us.
Detect a cancel and remove a reference by using callout_pending and
callout_stop (it's a bit tricy though, we can detect it).
While here, we can remove remaining abuses of mutex_owned for softnet_lock.
diffstat:
sys/net/if_llatbl.c | 38 +++++++++++++++-----------------------
sys/netinet/if_arp.c | 44 +++++++++++++++++++++++---------------------
sys/netinet/in.c | 35 ++---------------------------------
sys/netinet6/in6.c | 42 +++---------------------------------------
sys/netinet6/nd6.c | 38 ++++++++++++++++----------------------
5 files changed, 59 insertions(+), 138 deletions(-)
diffs (truncated from 338 to 300 lines):
diff -r 0fe1e7ca1ebf -r f37be11258eb sys/net/if_llatbl.c
--- a/sys/net/if_llatbl.c Tue Mar 06 07:20:41 2018 +0000
+++ b/sys/net/if_llatbl.c Tue Mar 06 07:24:01 2018 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if_llatbl.c,v 1.23 2018/02/14 14:15:53 maxv Exp $ */
+/* $NetBSD: if_llatbl.c,v 1.24 2018/03/06 07:24:01 ozaki-r Exp $ */
/*
* Copyright (c) 2004 Luigi Rizzo, Alessandro Cerri. All rights reserved.
* Copyright (c) 2004-2008 Qing Li. All rights reserved.
@@ -358,6 +358,18 @@
llt->llt_unlink_entry(lle);
}
+ /*
+ * Stop a pending callout if one exists. If we cancel one, we have to
+ * remove a reference to avoid a leak. callout_pending is required to
+ * to exclude the case that the callout has never been scheduled.
+ */
+ /* XXX once softnet_lock goes away, we should use callout_halt */
+ if (callout_pending(&lle->la_timer)) {
+ bool expired = callout_stop(&lle->la_timer);
+ if (!expired)
+ LLE_REMREF(lle);
+ }
+
pkts_dropped = lltable_drop_entry_queue(lle);
LLE_FREE_LOCKED(lle);
@@ -428,28 +440,8 @@
llentries_unlink(llt, &dchain);
IF_AFDATA_WUNLOCK(llt->llt_ifp);
- LIST_FOREACH_SAFE(lle, &dchain, lle_chain, next) {
- /*
- * 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);
- }
-
+ LIST_FOREACH_SAFE(lle, &dchain, lle_chain, next)
+ (void)llentry_free(lle);
}
/*
diff -r 0fe1e7ca1ebf -r f37be11258eb sys/netinet/if_arp.c
--- a/sys/netinet/if_arp.c Tue Mar 06 07:20:41 2018 +0000
+++ b/sys/netinet/if_arp.c Tue Mar 06 07:24:01 2018 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if_arp.c,v 1.269 2018/03/06 07:19:03 ozaki-r Exp $ */
+/* $NetBSD: if_arp.c,v 1.270 2018/03/06 07:24:01 ozaki-r Exp $ */
/*
* Copyright (c) 1998, 2000, 2008 The NetBSD Foundation, Inc.
@@ -68,7 +68,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.269 2018/03/06 07:19:03 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.270 2018/03/06 07:24:01 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_ddb.h"
@@ -318,29 +318,18 @@
KASSERT((lle->la_flags & LLE_STATIC) == 0);
LLE_WLOCK(lle);
- if (callout_pending(&lle->la_timer)) {
- /*
- * Here we are a bit odd in the treatment of
- * active/pending. If the pending bit is set, it got
- * rescheduled before I ran. The active
- * bit we ignore, since if it was stopped
- * in ll_tablefree() and was currently running
- * it would have return 0 so the code would
- * not have deleted it since the callout could
- * not be stopped so we want to go through
- * with the delete here now. If the callout
- * was restarted, the pending bit will be back on and
- * we just want to bail since the callout_reset would
- * return 1 and our reference would have been removed
- * by arpresolve() below.
- */
- LLE_WUNLOCK(lle);
+
+ /*
+ * This shortcut is required to avoid trying to touch ifp that may be
+ * being destroyed.
+ */
+ if ((lle->la_flags & LLE_LINKED) == 0) {
+ LLE_FREE_LOCKED(lle);
return;
}
+
ifp = lle->lle_tbl->llt_ifp;
- callout_stop(&lle->la_timer);
-
/* XXX: LOR avoidance. We still have ref on lle. */
LLE_WUNLOCK(lle);
@@ -369,6 +358,19 @@
LLE_WLOCK_ASSERT(la);
KASSERT((la->la_flags & LLE_STATIC) == 0);
+ /*
+ * We have to take care of a reference leak which occurs if
+ * callout_reset overwrites a pending callout schedule. Unfortunately
+ * we don't have a mean to know the overwrite, so we need to know it
+ * using callout_stop. We need to call callout_pending first to exclude
+ * the case that the callout has never been scheduled.
+ */
+ if (callout_pending(&la->la_timer)) {
+ bool expired = callout_stop(&la->la_timer);
+ if (!expired)
+ /* A pending callout schedule is canceled. */
+ LLE_REMREF(la);
+ }
LLE_ADDREF(la);
callout_reset(&la->la_timer, hz * sec, arptimer, la);
}
diff -r 0fe1e7ca1ebf -r f37be11258eb sys/netinet/in.c
--- a/sys/netinet/in.c Tue Mar 06 07:20:41 2018 +0000
+++ b/sys/netinet/in.c Tue Mar 06 07:24:01 2018 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: in.c,v 1.220 2018/03/06 07:20:41 ozaki-r Exp $ */
+/* $NetBSD: in.c,v 1.221 2018/03/06 07:24:01 ozaki-r Exp $ */
/*
* Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@@ -91,7 +91,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: in.c,v 1.220 2018/03/06 07:20:41 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: in.c,v 1.221 2018/03/06 07:24:01 ozaki-r Exp $");
#include "arp.h"
@@ -1992,44 +1992,13 @@
static void
in_lltable_free_entry(struct lltable *llt, struct llentry *lle)
{
- struct ifnet *ifp __diagused;
size_t pkts_dropped;
- bool locked = false;
LLE_WLOCK_ASSERT(lle);
KASSERT(llt != NULL);
- /* Unlink entry from table if not already */
- if ((lle->la_flags & LLE_LINKED) != 0) {
- ifp = llt->llt_ifp;
- IF_AFDATA_WLOCK_ASSERT(ifp);
- lltable_unlink_entry(llt, lle);
- locked = true;
- }
-
- /*
- * 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 (locked)
- IF_AFDATA_WUNLOCK(ifp);
-
- /* cancel timer */
- callout_halt(&lle->lle_timer, NULL);
-
- LLE_WLOCK(lle);
- LLE_REMREF(lle);
-
- /* Drop hold queue */
pkts_dropped = llentry_free(lle);
arp_stat_add(ARP_STAT_DFRDROPPED, (uint64_t)pkts_dropped);
-
- if (locked)
- IF_AFDATA_WLOCK(ifp);
}
static int
diff -r 0fe1e7ca1ebf -r f37be11258eb sys/netinet6/in6.c
--- a/sys/netinet6/in6.c Tue Mar 06 07:20:41 2018 +0000
+++ b/sys/netinet6/in6.c Tue Mar 06 07:24:01 2018 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: in6.c,v 1.261 2018/03/06 07:20:41 ozaki-r Exp $ */
+/* $NetBSD: in6.c,v 1.262 2018/03/06 07:24:01 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.261 2018/03/06 07:20:41 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: in6.c,v 1.262 2018/03/06 07:24:01 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -2446,45 +2446,9 @@
static void
in6_lltable_free_entry(struct lltable *llt, struct llentry *lle)
{
- struct ifnet *ifp = llt->llt_ifp;
- bool locked = false;
LLE_WLOCK_ASSERT(lle);
-
- /* Unlink entry from table */
- if ((lle->la_flags & LLE_LINKED) != 0) {
- IF_AFDATA_WLOCK_ASSERT(ifp);
- lltable_unlink_entry(llt, lle);
- KASSERT((lle->la_flags & LLE_LINKED) == 0);
- locked = true;
- }
- /*
- * 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 (locked)
- IF_AFDATA_WUNLOCK(ifp);
-
-#ifdef NET_MPSAFE
- callout_halt(&lle->lle_timer, NULL);
-#else
- 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);
-
- lltable_drop_entry_queue(lle);
- LLE_FREE_LOCKED(lle);
-
- if (locked)
- IF_AFDATA_WLOCK(ifp);
+ (void) llentry_free(lle);
}
static int
diff -r 0fe1e7ca1ebf -r f37be11258eb sys/netinet6/nd6.c
--- a/sys/netinet6/nd6.c Tue Mar 06 07:20:41 2018 +0000
+++ b/sys/netinet6/nd6.c Tue Mar 06 07:24:01 2018 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: nd6.c,v 1.245 2018/01/29 19:51:15 christos Exp $ */
+/* $NetBSD: nd6.c,v 1.246 2018/03/06 07:24:01 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.245 2018/01/29 19:51:15 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nd6.c,v 1.246 2018/03/06 07:24:01 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_net_mpsafe.h"
@@ -402,6 +402,19 @@
KASSERT(xtick >= 0);
+ /*
+ * We have to take care of a reference leak which occurs if
+ * callout_reset overwrites a pending callout schedule. Unfortunately
+ * we don't have a mean to know the overwrite, so we need to know it
+ * using callout_stop. We need to call callout_pending first to exclude
+ * the case that the callout has never been scheduled.
+ */
+ if (callout_pending(&ln->la_timer)) {
+ bool expired = callout_stop(&ln->la_timer);
+ if (!expired)
+ LLE_REMREF(ln);
+ }
+
ln->ln_expire = time_uptime + xtick / hz;
LLE_ADDREF(ln);
Home |
Main Index |
Thread Index |
Old Index