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>