Date: Mon, 21 Sep 2009 11:13:18 -0400 From: Ed Maste <emaste@freebsd.org> 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: <20090921151318.GA27605@sandvine.com> 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, Sep 21, 2009 at 01:43:33PM +0100, 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?
Your analysis is correct; this issue also has a PR, kern/137145.
http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/137145
As you point out it requires that two threads have a reference to the
same non-shared mbuf. I had a quick look and didn't find any case of
this in the vanilla FreeBSD tree; if I didn't miss anything it'll
affect only 3rd party src.
We'll need to have a look at this after 8.0 is done.
-Ed
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090921151318.GA27605>