From owner-freebsd-net@FreeBSD.ORG Mon Sep 21 15:25:22 2009 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1C85D10656A6 for ; Mon, 21 Sep 2009 15:25:22 +0000 (UTC) (envelope-from emaste@freebsd.org) Received: from mail2.sandvine.com (Mail1.sandvine.com [64.7.137.134]) by mx1.freebsd.org (Postfix) with ESMTP id D2C688FC26 for ; Mon, 21 Sep 2009 15:25:21 +0000 (UTC) Received: from labgw2.phaedrus.sandvine.com ([192.168.3.11]) by mail2.sandvine.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 21 Sep 2009 11:13:18 -0400 Received: by labgw2.phaedrus.sandvine.com (Postfix, from userid 12627) id C2D2011653; Mon, 21 Sep 2009 11:13:18 -0400 (EDT) Date: Mon, 21 Sep 2009 11:13:18 -0400 From: Ed Maste To: Andrew Brampton Message-ID: <20090921151318.GA27605@sandvine.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.1i X-OriginalArrivalTime: 21 Sep 2009 15:13:18.0861 (UTC) FILETIME=[0CF61BD0:01CA3ACE] Cc: freebsd-net@freebsd.org Subject: Re: Is this a race in mbuf's refcounting? X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Sep 2009 15:25:22 -0000 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