net80211 race conditions seen in -HEAD

[PseudoCylon]

On Wed, Jan 25, 2012 at 2:47 PM, Adrian Chadd <adrian at 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