From owner-freebsd-net@FreeBSD.ORG Fri Jun 20 22:20:47 2014 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id EEC0DAAD; Fri, 20 Jun 2014 22:20:47 +0000 (UTC) Received: from mail-we0-x232.google.com (mail-we0-x232.google.com [IPv6:2a00:1450:400c:c03::232]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 658C52139; Fri, 20 Jun 2014 22:20:47 +0000 (UTC) Received: by mail-we0-f178.google.com with SMTP id x48so4364008wes.37 for ; Fri, 20 Jun 2014 15:20:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=sgnT9InKFw7pFdhjwBPkpNZmeAknx1QVulPmFxayTmY=; b=GndWXiAtc2mRH49E3tirKn/FUlyD0QDqhKM6w50zxo/FbXwAV+staqwInA6O5OSbOR Gn1Be/BwYPl4RJyTbEc/bLCpEJ2lzMMcJrT7fd1oH+wgdH5W7brG5JtwOfDCdfpa8Gj9 ZBuLgh1Q3w5Gek+BrlxQn7lfq+qo9mSUR1tfAPtE4PiCm7+FJ/NWibrqU/59uSL1VQbg UH45Amk+iTsJcbd44iEAMu0TdLlkZ4YLIGfJzgz9U5c3tLVzbCyIgqyUQQo3vSPO/BrP fp2Of/ZVfQSUV/N+rXz2m70NjnLIXBEvrlgtxs3k+llJEZG7B1HVgV0QZuIVUAwrSr7u 49OA== MIME-Version: 1.0 X-Received: by 10.194.157.195 with SMTP id wo3mr6659570wjb.130.1403302845657; Fri, 20 Jun 2014 15:20:45 -0700 (PDT) Sender: asomers@gmail.com Received: by 10.194.168.202 with HTTP; Fri, 20 Jun 2014 15:20:45 -0700 (PDT) In-Reply-To: <53A48849.8080504@chelsio.com> References: <53A48849.8080504@chelsio.com> Date: Fri, 20 Jun 2014 16:20:45 -0600 X-Google-Sender-Auth: oqAFFhIa-WT2C8IEa4gj0Tylpl0 Message-ID: Subject: Re: ifaddr refcount problem From: Alan Somers To: Navdeep Parhar Content-Type: text/plain; charset=UTF-8 Cc: "freebsd-net@freebsd.org" X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Jun 2014 22:20:48 -0000 On Fri, Jun 20, 2014 at 1:15 PM, Navdeep Parhar wrote: > Revision 264905 and 266860 that followed it seem to leak ifaddr > references. ifa_ifwithdstaddr and ifa_ifwithnet both install a > reference on the ifaddr returned to the caller but ip_output does not > release it, eventually leading to a panic when the refcount wraps over > to 0 and the ifaddr is freed while it is still on various lists. Are you referring to the ifa_ref() calls at lines 1674, 1745, 1756, and 1795 of sys/net/if.c? Those long predate 264905. Why do you think that 264905 introduced the bug? If anything, I guess that it was introduced by change 262747, which removed ifa_free() from the done: block of ip_output(). > > I'm using this patch for now. Thoughts? > > Regards, > Navdeep > > > diff -r 6dfcecd314af sys/netinet/ip_output.c > --- a/sys/netinet/ip_output.c Fri Jun 20 10:33:22 2014 -0700 > +++ b/sys/netinet/ip_output.c Fri Jun 20 12:07:12 2014 -0700 > @@ -243,6 +243,7 @@ again: > ifp = ia->ia_ifp; > ip->ip_ttl = 1; > isbroadcast = 1; > + ifa_free((void *)ia); > } else if (flags & IP_ROUTETOIF) { > if ((ia = ifatoia(ifa_ifwithdstaddr(sintosa(dst)))) == NULL && > (ia = ifatoia(ifa_ifwithnet(sintosa(dst), 0))) == NULL) { > @@ -253,6 +254,7 @@ again: > ifp = ia->ia_ifp; > ip->ip_ttl = 1; > isbroadcast = in_broadcast(dst->sin_addr, ifp); > + ifa_free((void *)ia); > } else if (IN_MULTICAST(ntohl(ip->ip_dst.s_addr)) && > imo != NULL && imo->imo_multicast_ifp != NULL) { > /* I don't think this is quite right. ip_output() references ia at lines 366, 433, 630-637, 674, and 675. All of those references have NULL checks, so your patch won't cause a use-after-free bug, but I think that the patch might cause statistics collection to fail, and it might cause the IP source address to not be set. Do you have a test case that can reproduce the panic? -Alan