Date: Wed, 11 Sep 2013 07:24:46 +0000 (UTC) From: Devin Teske <dteske@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org Subject: svn commit: r255470 - in stable/8/sys: net netinet netinet6 Message-ID: <201309110724.r8B7OkdJ037593@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: dteske Date: Wed Sep 11 07:24:46 2013 New Revision: 255470 URL: http://svnweb.freebsd.org/changeset/base/255470 Log: Merge from stable/9 SVN r250927: MFC: r249628, r249742 - recover missing arp_ifinit() call. - plug static llentry leak (ipv4 & ipv6 were affected). PR: kern/172985 Also merge from stable/9 SVN r240313: Merge r238990 (manually resolving absence of r237263): Fix races between in_lltable_prefix_free(), lla_lookup(), llentry_free() and arptimer(): o Use callout_init_rw() for lle timeout, this allows us safely disestablish them. - This allows us to simplify the arptimer() and make it race safe. o Consistently use ifp->if_afdata_lock to lock access to linked lists in the lle hashes. o Introduce new lle flag LLE_LINKED, which marks an entry that is attached to the hash. - Use LLE_LINKED to avoid double unlinking via consequent calls to llentry_free(). - Mark lle with LLE_DELETED via |= operation istead of =, so that other flags won't be lost. o Make LLE_ADDREF(), LLE_REMREF() and LLE_FREE_LOCKED() more consistent and provide more informative KASSERTs. The patch is a collaborative work of all submitters and myself. PR: kern/165863 Submitted by: zont, rstone Submitted by: Eric van Gyzen <eric_van_gyzen dell.com> Modified: stable/8/sys/net/if_llatbl.c stable/8/sys/net/if_llatbl.h stable/8/sys/net/if_var.h stable/8/sys/net/if_vlan.c stable/8/sys/netinet/if_ether.c stable/8/sys/netinet/in.c stable/8/sys/netinet6/in6.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/net/ (props changed) stable/8/sys/netinet/ (props changed) stable/8/sys/netinet6/ (props changed) Modified: stable/8/sys/net/if_llatbl.c ============================================================================== --- stable/8/sys/net/if_llatbl.c Wed Sep 11 07:11:14 2013 (r255469) +++ stable/8/sys/net/if_llatbl.c Wed Sep 11 07:24:46 2013 (r255470) @@ -109,10 +109,19 @@ llentry_free(struct llentry *lle) size_t pkts_dropped; struct mbuf *next; - pkts_dropped = 0; + IF_AFDATA_WLOCK_ASSERT(lle->lle_tbl->llt_ifp); LLE_WLOCK_ASSERT(lle); + + /* XXX: guard against race with other llentry_free(). */ + if (!(lle->la_flags & LLE_LINKED)) { + LLE_FREE_LOCKED(lle); + return (0); + } + LIST_REMOVE(lle, lle_next); + lle->la_flags &= ~(LLE_VALID | LLE_LINKED); + pkts_dropped = 0; while ((lle->la_numheld > 0) && (lle->la_hold != NULL)) { next = lle->la_hold->m_nextpkt; m_freem(lle->la_hold); @@ -125,7 +134,6 @@ llentry_free(struct llentry *lle) ("%s: la_numheld %d > 0, pkts_droped %zd", __func__, lle->la_numheld, pkts_dropped)); - lle->la_flags &= ~LLE_VALID; LLE_FREE_LOCKED(lle); return (pkts_dropped); @@ -185,17 +193,16 @@ lltable_free(struct lltable *llt) SLIST_REMOVE(&V_lltables, llt, lltable, llt_link); LLTABLE_WUNLOCK(); - for (i=0; i < LLTBL_HASHTBL_SIZE; i++) { + IF_AFDATA_WLOCK(llt->llt_ifp); + for (i = 0; i < LLTBL_HASHTBL_SIZE; i++) { LIST_FOREACH_SAFE(lle, &llt->lle_head[i], lle_next, next) { - int canceled; - - canceled = callout_drain(&lle->la_timer); LLE_WLOCK(lle); - if (canceled) + if (callout_stop(&lle->la_timer)) LLE_REMREF(lle); llentry_free(lle); } } + IF_AFDATA_WUNLOCK(llt->llt_ifp); free(llt, M_LLTABLE); } Modified: stable/8/sys/net/if_llatbl.h ============================================================================== --- stable/8/sys/net/if_llatbl.h Wed Sep 11 07:11:14 2013 (r255469) +++ stable/8/sys/net/if_llatbl.h Wed Sep 11 07:24:46 2013 (r255470) @@ -97,26 +97,28 @@ struct llentry { #define LLE_ADDREF(lle) do { \ LLE_WLOCK_ASSERT(lle); \ KASSERT((lle)->lle_refcnt >= 0, \ - ("negative refcnt %d", (lle)->lle_refcnt)); \ + ("negative refcnt %d on lle %p", \ + (lle)->lle_refcnt, (lle))); \ (lle)->lle_refcnt++; \ } while (0) #define LLE_REMREF(lle) do { \ LLE_WLOCK_ASSERT(lle); \ - KASSERT((lle)->lle_refcnt > 1, \ - ("bogus refcnt %d", (lle)->lle_refcnt)); \ + KASSERT((lle)->lle_refcnt > 0, \ + ("bogus refcnt %d on lle %p", \ + (lle)->lle_refcnt, (lle))); \ (lle)->lle_refcnt--; \ } while (0) #define LLE_FREE_LOCKED(lle) do { \ - if ((lle)->lle_refcnt <= 1) \ + if ((lle)->lle_refcnt == 1) \ (lle)->lle_tbl->llt_free((lle)->lle_tbl, (lle));\ else { \ - (lle)->lle_refcnt--; \ + LLE_REMREF(lle); \ LLE_WUNLOCK(lle); \ } \ /* guard against invalid refs */ \ - lle = NULL; \ + (lle) = NULL; \ } while (0) #define LLE_FREE(lle) do { \ @@ -167,9 +169,10 @@ MALLOC_DECLARE(M_LLTABLE); #define LLE_VALID 0x0008 /* ll_addr is valid */ #define LLE_PROXY 0x0010 /* proxy entry ??? */ #define LLE_PUB 0x0020 /* publish entry ??? */ +#define LLE_LINKED 0x0040 /* linked to lookup structure */ +#define LLE_EXCLUSIVE 0x2000 /* return lle xlocked */ #define LLE_DELETE 0x4000 /* delete on a lookup - match LLE_IFADDR */ #define LLE_CREATE 0x8000 /* create on a lookup miss */ -#define LLE_EXCLUSIVE 0x2000 /* return lle xlocked */ #define LLATBL_HASH(key, mask) \ (((((((key >> 8) ^ key) >> 8) ^ key) >> 8) ^ key) & mask) Modified: stable/8/sys/net/if_var.h ============================================================================== --- stable/8/sys/net/if_var.h Wed Sep 11 07:11:14 2013 (r255469) +++ stable/8/sys/net/if_var.h Wed Sep 11 07:24:46 2013 (r255470) @@ -406,6 +406,8 @@ EVENTHANDLER_DECLARE(group_change_event, #define IF_AFDATA_DESTROY(ifp) rw_destroy(&(ifp)->if_afdata_lock) #define IF_AFDATA_LOCK_ASSERT(ifp) rw_assert(&(ifp)->if_afdata_lock, RA_LOCKED) +#define IF_AFDATA_RLOCK_ASSERT(ifp) rw_assert(&(ifp)->if_afdata_lock, RA_RLOCKED) +#define IF_AFDATA_WLOCK_ASSERT(ifp) rw_assert(&(ifp)->if_afdata_lock, RA_WLOCKED) #define IF_AFDATA_UNLOCK_ASSERT(ifp) rw_assert(&(ifp)->if_afdata_lock, RA_UNLOCKED) int if_handoff(struct ifqueue *ifq, struct mbuf *m, struct ifnet *ifp, Modified: stable/8/sys/net/if_vlan.c ============================================================================== --- stable/8/sys/net/if_vlan.c Wed Sep 11 07:11:14 2013 (r255469) +++ stable/8/sys/net/if_vlan.c Wed Sep 11 07:24:46 2013 (r255470) @@ -41,6 +41,7 @@ #include <sys/cdefs.h> __FBSDID("$FreeBSD$"); +#include "opt_inet.h" #include "opt_vlan.h" #include <sys/param.h> @@ -65,6 +66,11 @@ __FBSDID("$FreeBSD$"); #include <net/if_vlan_var.h> #include <net/vnet.h> +#ifdef INET +#include <netinet/in.h> +#include <netinet/if_ether.h> +#endif + #define VLANNAME "vlan" #define VLAN_DEF_HWIDTH 4 #define VLAN_IFFLAGS (IFF_BROADCAST | IFF_MULTICAST) Modified: stable/8/sys/netinet/if_ether.c ============================================================================== --- stable/8/sys/netinet/if_ether.c Wed Sep 11 07:11:14 2013 (r255469) +++ stable/8/sys/netinet/if_ether.c Wed Sep 11 07:24:46 2013 (r255470) @@ -167,38 +167,30 @@ arp_ifscrub(struct ifnet *ifp, uint32_t static void arptimer(void *arg) { + struct llentry *lle = (struct llentry *)arg; struct ifnet *ifp; - struct llentry *lle; - int pkts_dropped; + size_t pkts_dropped; + + if (lle->la_flags & LLE_STATIC) { + LLE_WUNLOCK(lle); + return; + } - KASSERT(arg != NULL, ("%s: arg NULL", __func__)); - lle = (struct llentry *)arg; ifp = lle->lle_tbl->llt_ifp; CURVNET_SET(ifp->if_vnet); + + callout_stop(&lle->la_timer); + + /* XXX: LOR avoidance. We still have ref on lle. */ + LLE_WUNLOCK(lle); IF_AFDATA_LOCK(ifp); LLE_WLOCK(lle); - if (lle->la_flags & LLE_STATIC) - LLE_WUNLOCK(lle); - else { - if (!callout_pending(&lle->la_timer) && - callout_active(&lle->la_timer)) { - callout_stop(&lle->la_timer); - LLE_REMREF(lle); - pkts_dropped = llentry_free(lle); - ARPSTAT_ADD(dropped, pkts_dropped); - ARPSTAT_INC(timeouts); - } else { -#ifdef DIAGNOSTIC - struct sockaddr *l3addr = L3_ADDR(lle); - log(LOG_INFO, - "arptimer issue: %p, IPv4 address: \"%s\"\n", lle, - inet_ntoa( - ((const struct sockaddr_in *)l3addr)->sin_addr)); -#endif - LLE_WUNLOCK(lle); - } - } + + LLE_REMREF(lle); + pkts_dropped = llentry_free(lle); IF_AFDATA_UNLOCK(ifp); + ARPSTAT_ADD(dropped, pkts_dropped); + ARPSTAT_INC(timeouts); CURVNET_RESTORE(); } Modified: stable/8/sys/netinet/in.c ============================================================================== --- stable/8/sys/netinet/in.c Wed Sep 11 07:11:14 2013 (r255469) +++ stable/8/sys/netinet/in.c Wed Sep 11 07:24:46 2013 (r255470) @@ -1350,7 +1350,6 @@ in_lltable_new(const struct sockaddr *l3 if (lle == NULL) /* NB: caller generates msg */ return NULL; - callout_init(&lle->base.la_timer, CALLOUT_MPSAFE); /* * For IPv4 this will trigger "arpresolve" to generate * an ARP request. @@ -1359,7 +1358,10 @@ in_lltable_new(const struct sockaddr *l3 lle->l3_addr4 = *(const struct sockaddr_in *)l3addr; lle->base.lle_refcnt = 1; LLE_LOCK_INIT(&lle->base); - return &lle->base; + callout_init_rw(&lle->base.la_timer, &lle->base.lle_lock, + CALLOUT_RETURNUNLOCKED); + + return (&lle->base); } /* @@ -1392,7 +1394,8 @@ in_lltable_prefix_free(struct lltable *l register int i; size_t pkts_dropped; - for (i=0; i < LLTBL_HASHTBL_SIZE; i++) { + IF_AFDATA_WLOCK(llt->llt_ifp); + for (i = 0; i < LLTBL_HASHTBL_SIZE; i++) { LIST_FOREACH_SAFE(lle, &llt->lle_head[i], lle_next, next) { /* @@ -1402,17 +1405,15 @@ in_lltable_prefix_free(struct lltable *l if (IN_ARE_MASKED_ADDR_EQUAL((struct sockaddr_in *)L3_ADDR(lle), pfx, msk) && ((flags & LLE_STATIC) || !(lle->la_flags & LLE_STATIC))) { - int canceled; - - canceled = callout_drain(&lle->la_timer); LLE_WLOCK(lle); - if (canceled) + if (callout_stop(&lle->la_timer)) LLE_REMREF(lle); pkts_dropped = llentry_free(lle); ARPSTAT_ADD(dropped, pkts_dropped); } } } + IF_AFDATA_WUNLOCK(llt->llt_ifp); } @@ -1545,15 +1546,20 @@ in_lltable_lookup(struct lltable *llt, u lle->lle_tbl = llt; lle->lle_head = lleh; + lle->la_flags |= LLE_LINKED; LIST_INSERT_HEAD(lleh, lle, lle_next); } else if (flags & LLE_DELETE) { if (!(lle->la_flags & LLE_IFADDR) || (flags & LLE_IFADDR)) { LLE_WLOCK(lle); - lle->la_flags = LLE_DELETED; - LLE_WUNLOCK(lle); + lle->la_flags |= LLE_DELETED; #ifdef DIAGNOSTIC - log(LOG_INFO, "ifaddr cache = %p is deleted\n", lle); + log(LOG_INFO, "ifaddr cache = %p is deleted\n", lle); #endif + if ((lle->la_flags & + (LLE_STATIC | LLE_IFADDR)) == LLE_STATIC) + llentry_free(lle); + else + LLE_WUNLOCK(lle); } lle = (void *)-1; Modified: stable/8/sys/netinet6/in6.c ============================================================================== --- stable/8/sys/netinet6/in6.c Wed Sep 11 07:11:14 2013 (r255469) +++ stable/8/sys/netinet6/in6.c Wed Sep 11 07:24:46 2013 (r255470) @@ -1377,10 +1377,10 @@ in6_purgeaddr(struct ifaddr *ifa) nd6_dad_stop(ifa); /* Remove local address entry from lltable. */ - IF_AFDATA_LOCK(ifp); - lla_lookup(LLTABLE6(ifp), (LLE_DELETE | LLE_IFADDR), - (struct sockaddr *)&ia->ia_addr); - IF_AFDATA_UNLOCK(ifp); + memcpy(&addr, &ia->ia_addr, sizeof(ia->ia_addr)); + memcpy(&mask, &ia->ia_prefixmask, sizeof(ia->ia_prefixmask)); + lltable_prefix_free(AF_INET6, (struct sockaddr *)&addr, + (struct sockaddr *)&mask, LLE_STATIC); /* * initialize for rtmsg generation @@ -1393,8 +1393,6 @@ in6_purgeaddr(struct ifaddr *ifa) /* */ bzero(&rt0, sizeof(rt0)); rt0.rt_gateway = (struct sockaddr *)&gateway; - memcpy(&mask, &ia->ia_prefixmask, sizeof(ia->ia_prefixmask)); - memcpy(&addr, &ia->ia_addr, sizeof(ia->ia_addr)); rt_mask(&rt0) = (struct sockaddr *)&mask; rt_key(&rt0) = (struct sockaddr *)&addr; rt0.rt_flags = RTF_HOST | RTF_STATIC; @@ -2392,23 +2390,22 @@ in6_lltable_prefix_free(struct lltable * * (flags & LLE_STATIC) means deleting all entries * including static ND6 entries */ - for (i=0; i < LLTBL_HASHTBL_SIZE; i++) { + IF_AFDATA_WLOCK(llt->llt_ifp); + for (i = 0; i < LLTBL_HASHTBL_SIZE; i++) { LIST_FOREACH_SAFE(lle, &llt->lle_head[i], lle_next, next) { if (IN6_ARE_MASKED_ADDR_EQUAL( - &((struct sockaddr_in6 *)L3_ADDR(lle))->sin6_addr, - &pfx->sin6_addr, - &msk->sin6_addr) && - ((flags & LLE_STATIC) || !(lle->la_flags & LLE_STATIC))) { - int canceled; - - canceled = callout_drain(&lle->la_timer); + &satosin6(L3_ADDR(lle))->sin6_addr, + &pfx->sin6_addr, &msk->sin6_addr) && + ((flags & LLE_STATIC) || + !(lle->la_flags & LLE_STATIC))) { LLE_WLOCK(lle); - if (canceled) + if (callout_stop(&lle->la_timer)) LLE_REMREF(lle); llentry_free(lle); } } } + IF_AFDATA_WUNLOCK(llt->llt_ifp); } static int @@ -2500,15 +2497,20 @@ in6_lltable_lookup(struct lltable *llt, lle->lle_tbl = llt; lle->lle_head = lleh; + lle->la_flags |= LLE_LINKED; LIST_INSERT_HEAD(lleh, lle, lle_next); } else if (flags & LLE_DELETE) { if (!(lle->la_flags & LLE_IFADDR) || (flags & LLE_IFADDR)) { LLE_WLOCK(lle); - lle->la_flags = LLE_DELETED; - LLE_WUNLOCK(lle); + lle->la_flags |= LLE_DELETED; #ifdef DIAGNOSTIC - log(LOG_INFO, "ifaddr cache = %p is deleted\n", lle); -#endif + log(LOG_INFO, "ifaddr cache = %p is deleted\n", lle); +#endif + if ((lle->la_flags & + (LLE_STATIC | LLE_IFADDR)) == LLE_STATIC) + llentry_free(lle); + else + LLE_WUNLOCK(lle); } lle = (void *)-1; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201309110724.r8B7OkdJ037593>