Date: Tue, 22 Sep 2009 00:05:14 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Andrew Brampton <brampton+freebsd-net@gmail.com> Cc: freebsd-net@FreeBSD.org Subject: Re: Is this a race in mbuf's refcounting? Message-ID: <20090921235604.U12163@delplex.bde.org> In-Reply-To: <d41814900909210543p46894d83u6d814353ea1ee130@mail.gmail.com> References: <d41814900909210543p46894d83u6d814353ea1ee130@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 21 Sep 2009, Andrew Brampton wrote:
> I've been reading the FreeBSD source code to understand how mbufs are
> reference counted. However, there are a few bits of code that I'm
> wondering if they would fail under the exactly right timing. Take for
> example in uipc_mbuf.c:
>
> 286 static void
> 287 mb_dupcl(struct mbuf *n, struct mbuf *m)
> 288 {
> ...
> 293 if (*(m->m_ext.ref_cnt) == 1)
> 294 *(m->m_ext.ref_cnt) += 1;
> 295 else
> 296 atomic_add_int(m->m_ext.ref_cnt, 1);
> ...
> 305 }
>
> Now, the way I understand this code is, if ref_cnt is 1, then it is
> not shared. In that case non-atomically increment ref_cnt. However, if
> ref_cnt was something else, then it is shared so update the value in
> an atomic way. This seems valid, however what happens if two threads
> call mb_dupcl at the same time with a non-shared m. Could they both
> evaluate the if on line 293 at the same time, and then both
> non-atomically increment ref_cnt?
>
> If this could happen then we have a lost update and our reference
> counting is broken. I've also noticed that in other places similar
> optimisations are made to avoid the atomic operation.
>
> So is this a problem?
I don't see how it can work.
Also, if the count was 1, then it should become 2, but there is nothing to
flush the store to memory. This seems to mainly enlarge the race window
for the previous problem.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090921235604.U12163>