From owner-freebsd-net@FreeBSD.ORG Sat Mar 10 00:59:33 2012 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 9C4EB106566B for ; Sat, 10 Mar 2012 00:59:33 +0000 (UTC) (envelope-from prabhakar.lakhera@gmail.com) Received: from mail-yx0-f182.google.com (mail-yx0-f182.google.com [209.85.213.182]) by mx1.freebsd.org (Postfix) with ESMTP id 5C2DB8FC25 for ; Sat, 10 Mar 2012 00:59:33 +0000 (UTC) Received: by yenl9 with SMTP id l9so1552540yen.13 for ; Fri, 09 Mar 2012 16:59:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:date:message-id:subject:from:to:content-type; bh=t23+TuW2yxlKvnFP4PvarBxCNG828M/6HjeYhc++lzw=; b=x0rov2uGbZllML0YhKSvD/wZ9/FjTxSyx6+cc1SckW7tBMsgaF2px2FdcmjM4Yh3r6 96iNgREWIJXl36s7sPdP4Sxrby+xLcAEKw44RAPE9TtoPuGz2wiySjmjb/d7cMY2/xoN 7IPUxWF8l2eUfSTtS9FmkTiemHRol+FqFefoOonMsXjIQNCwP/N2eq6K0I9Efg0RWZk6 WRc8xAvDVQb9Pp7r2mQ+B65ySflLH1/dVBGuahFeg2BYZImyi2y3cQx/j0R/26UB/l/m VfnyY0PPV3ZwvmOaNhnURG21vuwVJgHdDRoWmOAq0opjmAYJW03pQMjde8P2DGSSRVFw FjJg== MIME-Version: 1.0 Received: by 10.236.118.131 with SMTP id l3mr334033yhh.82.1331341172674; Fri, 09 Mar 2012 16:59:32 -0800 (PST) Received: by 10.101.61.11 with HTTP; Fri, 9 Mar 2012 16:59:32 -0800 (PST) Date: Fri, 9 Mar 2012 16:59:32 -0800 Message-ID: From: prabhakar lakhera To: freebsd-net@freebsd.org Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.5 Subject: Race condition in IPv6 DAD detection code? 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: Sat, 10 Mar 2012 00:59:33 -0000 Hi, I had sent a mail sometime back regarding some queries/concern regarding v6 DAD detection code. Here's what my concern is, drawing tables for the whole sequence in hopes of getting some replies: *CPU1* *CPU2* In nd6_dad_na_input... struct dadq *dp; if (ifa =3D=3D NULL) panic("ifa =3D=3D NULL in nd6_dad_na_input"); dp =3D nd6_dad_find(ifa); if (dp) nd6_dad_timer.. last attempt and NA counter not yet incremented by the thread on CPU1, therefore address considered to be not duplicated. if (dp->dad_ns_ocount < dp->dad_count) { =85.. } else { /* * We have transmitted sufficient number of DAD packets. * See what we've got. */ int duplicate; duplicate =3D 0; if (dp->dad_na_icount) { /* * the check is in nd6_dad_na_input(), * but just in case */ duplicate++; } if (dp->dad_ns_icount) { /* We've seen NS, means DAD has failed. */ duplicate++; } if (duplicate) { =DF----- Duplicate 0 here /* (*dp) will be freed in nd6_dad_duplicated() */ dp =3D NULL; nd6_dad_duplicated(ifa); } else { In nd6_dad_na_input... dp->dad_na_icount++; /* remove the address. */ nd6_dad_duplicated(ifa); Inside nd6_dad_duplicated=85 nd6_dad_duplicated(struct ifaddr *ifa) { struct in6_ifaddr *ia =3D (struct in6_ifaddr *)ifa; struct ifnet *ifp; struct dadq *dp; char ip6buf[INET6_ADDRSTRLEN]; dp =3D nd6_dad_find(ifa); if (dp =3D=3D NULL) { log(LOG_ERR, "nd6_dad_duplicated: DAD structure not found\n"); return; } =85. --=E0 dp is not NULL as has not yet been removed from queue. Nd6_dad_timer executing else=85. /* * We are done with DAD. No NA came, no NS came. * No duplicate address found. */ ia->ia6_flags &=3D ~IN6_IFF_TENTATIVE; nd6log((LOG_DEBUG, "%s: DAD complete for %s - no duplicates found\n", if_name(ifa->ifa_ifp), ip6_sprintf(ip6buf, &ia->ia_addr.sin6_addr))); TAILQ_REMOVE(&V_dadq, (struct dadq *)dp, dad_list); free(dp, M_IP6NDP); dp =3D NULL; ifa_free(ifa); } In nd6_dad_duplicated(struct ifaddr *ifa) =85 *Working on freed pointer??? * * log(LOG_ERR, "%s: DAD detected duplicate IPv6 address %s: "* * "NS in/out=3D%d/%d, NA in=3D%d\n",* * if_name(ifa->ifa_ifp), ip6_sprintf(ip6buf, &ia->ia_addr.sin6_addr),* * dp->dad_ns_icount, dp->dad_ns_ocount, dp->dad_na_icount);* On Wed, Mar 7, 2012 at 5:51 PM, prabhakar lakhera < prabhakar.lakhera@gmail.com> wrote: > Hi, > > I was puzzled to look at DAD detection code in FreeBSD. We check for > counters for any received NA/NS for DAD in nd6_dad_timer: > > if (dp->dad_na_icount) { > 1326 /* > 1327 * the check is in nd6_dad_na_input(), > 1328 * but just in case > 1329 */ > 1330 duplicate++; > 1331 } > 1332 > 1333 if (dp->dad_ns_icount) { > 1334 /* We've seen NS, means DAD has failed. */ > 1335 duplicate++; > 1336 } > 1337 > 1338 if (duplicate) { > 1339 /* (*dp) will be freed in > nd6_dad_duplicated() */ > 1340 dp =3D NULL; > 1341 nd6_dad_duplicated(ifa); > > the function later calls nd6_dad_duplicated to perform the remaining work > if the address is detected duplicate. > > nd6_dad_duplicated also gets called from nd6_dad_na_input > and nd6_dad_ns_input, both the functions are the only places which > increment the input NA/NS counters respectively. > > 1505 static void > 1506 nd6_dad_na_input(struct ifaddr *ifa) > 1507 { > 1508 struct dadq *dp; > 1509 > 1510 if (ifa =3D=3D NULL) > 1511 panic("ifa =3D=3D NULL in nd6_dad_na_input"); > 1512 > 1513 dp =3D nd6_dad_find(ifa); > 1514 if (dp) > 1515 dp->dad_na_icount++; > 1516 > 1517 /* remove the address. */ > 1518 nd6_dad_duplicated(ifa); > 1519 } > > nd6_dad_duplicated stops the timer among other things. > > Why nd6_dad_timer need check on these counters if we stop the timer on DA= D > failure anyways? > Ok.. may be just an optimization which just "hopes" that the counters hav= e > been updated but the nd6_dad_*_input has not yet called nd6_dad_duplicate= d. > > Can the this timer and na packet processing ever run in parallel, I don;t > see dp being protected by any locks, nor does it seem that it's been > reference counted. > Any explanation will be highly appreciated. > > Best, > > Prabhakar > > > > > >