Date: Mon, 9 Feb 2015 19:28:12 +0000 (UTC) From: Randall Stewart <rrs@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r278472 - in head/sys: netinet netinet6 Message-ID: <201502091928.t19JSC5P066293@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: rrs Date: Mon Feb 9 19:28:11 2015 New Revision: 278472 URL: https://svnweb.freebsd.org/changeset/base/278472 Log: This fixes a bug in the way that the LLE timers for nd6 and arp were being used. They basically would pass in the mutex to the callout_init. Because they used this method to the callout system, it was possible to "stop" the callout. When flushing the table and you stopped the running callout, the callout_stop code would return 1 indicating that it was going to stop the callout (that was about to run on the callout_wheel blocked by the function calling the stop). Now when 1 was returned, it would lower the reference count one extra time for the stopped timer, then a few lines later delete the memory. Of course the callout_wheel was stuck in the lock code and would then crash since it was accessing freed memory. By using callout_init(c, 1) we always get a 0 back and the reference counting bug does not rear its head. We do have to make a few adjustments to the callouts themselves though to make sure it does the proper thing if rescheduled as well as gets the lock. Commented upon by hiren and sbruno See Phabricator D1777 for more details. Commented upon by hiren and sbruno Reviewed by: adrian, jhb and bz Sponsored by: Netflix Inc. Modified: head/sys/netinet/if_ether.c head/sys/netinet/in.c head/sys/netinet6/in6.c head/sys/netinet6/nd6.c Modified: head/sys/netinet/if_ether.c ============================================================================== --- head/sys/netinet/if_ether.c Mon Feb 9 19:21:54 2015 (r278471) +++ head/sys/netinet/if_ether.c Mon Feb 9 19:28:11 2015 (r278472) @@ -166,10 +166,28 @@ arptimer(void *arg) struct ifnet *ifp; if (lle->la_flags & LLE_STATIC) { - LLE_WUNLOCK(lle); return; } - + LLE_WLOCK(lle); + if (callout_pending(&lle->la_timer)) { + /* + * Here we are a bit odd here 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); + return; + } ifp = lle->lle_tbl->llt_ifp; CURVNET_SET(ifp->if_vnet); Modified: head/sys/netinet/in.c ============================================================================== --- head/sys/netinet/in.c Mon Feb 9 19:21:54 2015 (r278471) +++ head/sys/netinet/in.c Mon Feb 9 19:28:11 2015 (r278472) @@ -962,8 +962,7 @@ in_lltable_new(const struct sockaddr *l3 lle->base.lle_refcnt = 1; lle->base.lle_free = in_lltable_free; LLE_LOCK_INIT(&lle->base); - callout_init_rw(&lle->base.la_timer, &lle->base.lle_lock, - CALLOUT_RETURNUNLOCKED); + callout_init(&lle->base.la_timer, 1); return (&lle->base); } Modified: head/sys/netinet6/in6.c ============================================================================== --- head/sys/netinet6/in6.c Mon Feb 9 19:21:54 2015 (r278471) +++ head/sys/netinet6/in6.c Mon Feb 9 19:28:11 2015 (r278472) @@ -2047,8 +2047,7 @@ in6_lltable_new(const struct sockaddr *l lle->base.lle_refcnt = 1; lle->base.lle_free = in6_lltable_free; LLE_LOCK_INIT(&lle->base); - callout_init_rw(&lle->base.ln_timer_ch, &lle->base.lle_lock, - CALLOUT_RETURNUNLOCKED); + callout_init(&lle->base.ln_timer_ch, 1); return (&lle->base); } Modified: head/sys/netinet6/nd6.c ============================================================================== --- head/sys/netinet6/nd6.c Mon Feb 9 19:21:54 2015 (r278471) +++ head/sys/netinet6/nd6.c Mon Feb 9 19:28:11 2015 (r278472) @@ -473,9 +473,28 @@ nd6_llinfo_timer(void *arg) KASSERT(arg != NULL, ("%s: arg NULL", __func__)); ln = (struct llentry *)arg; - LLE_WLOCK_ASSERT(ln); + LLE_WLOCK(ln); + if (callout_pending(&ln->la_timer)) { + /* + * Here we are a bit odd here 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 nd6_llinfo_settimer_locked above since canceled + * would have been 1. + */ + LLE_WUNLOCK(ln); + return; + } ifp = ln->lle_tbl->llt_ifp; - CURVNET_SET(ifp->if_vnet); if (ln->ln_ntick > 0) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201502091928.t19JSC5P066293>