Date: Thu, 2 Jan 2014 22:17:38 +0000 From: "Gumpula, Suresh" <Suresh.Gumpula@netapp.com> To: "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org> Cc: "Gumpula, Suresh" <Suresh.Gumpula@netapp.com> Subject: Reference count race window Message-ID: <D29CB80EBA4DEA4D91181928AAF51538438C0D8B@SACEXCMBX04-PRD.hq.netapp.com>
next in thread | raw e-mail | index | archive | help
Hi,
I am Suresh from NetAPP and I have questions/queries related to the refe=
rence count usage in the BSD kernel. We are seeing some corruptions/use aft=
er free
issues and while debugging we found that the corruption pattern is a ucr=
ed/crgroups structure and started looking at ucred reference count implemen=
ation.
This is my understanding of ref count race window, please correct me if I =
am wrong.
It seems there is a timing window exposed by the FreeBSD reference count us=
age/implementation. Let's start with the definitions of the acquire and rel=
ease routines
in freebsd/sys/sys/refcount.h
static __inline void
refcount_acquire(volatile u_int *count)
{
atomic_add_acq_int(count, 1);
}
static __inline int
refcount_release(volatile u_int *count)
{
u_int old;
/* XXX: Should this have a rel membar? */
old =3D atomic_fetchadd_int(count, -1);
KASSERT(old > 0, ("negative refcount %p", count));
return (old =3D=3D 1);
}
As implemented, a call to refcount_acquire atomically increments the refer=
ence count while refcount_release decrements
the reference count and returns true if this release dropped the reference =
count to zero.
Consider the following sequence of events in the absence of other external =
synchronization:
* Object foo has a refcount of 1
* Thread a on processor m calls refcount_release on foo.
* Very soon after (in CPU terms) thread b on processor n calls refcount_acq=
uire on foo.
* atomic_fetchadd_int operating in thread a stalls the atomic_add_acq_int i=
n thread b,
decrementing foo's refcount to zero and setting old to 1. refcount_releas=
e returns true.
* atomic_add_acq_int in thread b increments the reference count to 1!
* thread a, seeing refcount_release return success, frees foo.
* thread b, believing it has a reference count on foo, continues to use it.
* The major hole here is that refcount_acquire is a void function. If it al=
so returned status,
calling software could determine that it had a valid reference and take =
appropriate action if it failed to acquire.
One such implementation might look like:
static __inline int
refcount_acquire(volatile u_int *count)
{
u_int old;
old =3D atomic_fetchadd_int(count, 1);
return (old !=3D 0);
}
This change would require modification of all calls to refcount_acquire and=
determining appropriate action in the case of a non-success return.
Without changing the return-value semantics of refcount_acquire, we have in=
troduced a panic if we detected a race as below.
static __inline void
refcount_acquire(volatile u_int *count)
{
u_int old;
old =3D atomic_fetchadd_int(count, 1);
if (old =3D=3D 0) {
panic("refcount_acquire race condition detected!\n");
}
}
After this change , we have seen this panic in one of our systems. Could so=
meone look at my understanding and give me some ways to narrow down this pr=
oblem.
As I mentioned earlier, one option is to change refcount_acquire to be non =
void and change all the callers, but it seems there are many paths to be ch=
anged on failure case.
Thank you
Suresh
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?D29CB80EBA4DEA4D91181928AAF51538438C0D8B>