Date: Thu, 26 Jan 2012 00:08:26 -0700 From: PseudoCylon <moonlightakkiy@yahoo.ca> To: Adrian Chadd <adrian@freebsd.org> Cc: freebsd-wireless@freebsd.org Subject: Re: net80211 race conditions seen in -HEAD Message-ID: <CAFZ_MYKdcFm5ukJyK8-0YoyAKsu%2B8jwXpvvt9T7=oF0VVbuvmA@mail.gmail.com> In-Reply-To: <CAJ-Vmo=tv0oHrsG834YdS32j%2B6%2BAe-Co9142Diox6SS6usL24w@mail.gmail.com> References: <CAFZ_MY%2BifiXc3iPfDEuWNHyr7JvhuG55uzp3BTmCO2Ek2G1LOg@mail.gmail.com> <CAJ-VmomReMTTDQ3KYjbRTb4%2BLY%2BKVtkba_T0fwM49oHakW_XSg@mail.gmail.com> <CAJ-Vmo=tv0oHrsG834YdS32j%2B6%2BAe-Co9142Diox6SS6usL24w@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Jan 25, 2012 at 2:47 PM, Adrian Chadd <adrian@freebsd.org> wrote: > .. whilst the refcount is 1, so ieee80211_ref_node() may not increment the > counter before it's freed by another thread. > Further browsing the codes, I'd say here is the point of no return. http://fxr.watson.org/fxr/source/net80211/ieee80211_freebsd.c?im=bigexcerpts#L310 After atomic_cmpset_int() returned 1, increment ref cont won't stop freeing node no matter how we handle the ref count. It will continue freeing node, anyway. If we cannot stop freeing node, we should stop the thread using the node once freeing node process has started. How about make ieee80211_ref_node() return NULL when ni_refcnt == 0 and caller of ieee80211_ref_node() to exit? ieee80211_ref_node(ni) { #ifdef NO_LOCK /* * This loop simulates atomic_cmp_and_add() as commented at http://fxr.watson.org/fxr/source/net80211/ieee80211_freebsd.c?im=bigexcerpts#L308 * The current code work most of the time, so this will loop very rarely */ for (;;) { if ((cnt = atomic_load_int(&ni_refcnt)) == 0) return (NULL); /* caller should abort process */ if (atomic_cmp_set(&ni_refcnt, cnt, cnt + 1)) return (ni); } #else /* you may receive complimentary barrage of LOR */ LOCK(); if (ni_refcnt == 0) ni = NULL; else ni_refcnt++: UNLOCK(); return (ni); #endif } ieee80211_node_dectestref(ni) { #ifdef NO_LOCK for (;;) { if ((cnt = atomic_load_int(&ni_refcnt)) == 0) return (1); /* free node */ if (atomic_cmp_set(&ni_refcnt, cnt, cnt -1)) return (cnt <= 1); /* cnt - 1 == 0 free node */ } #else LOCK(); cnt = ni_refcnt > 0 ? cnt - 1 : 0; UNLOCK(); return (cnt); #endif } AK
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFZ_MYKdcFm5ukJyK8-0YoyAKsu%2B8jwXpvvt9T7=oF0VVbuvmA>