From owner-freebsd-net@FreeBSD.ORG Mon Jun 23 16:45:01 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 313D0355; Mon, 23 Jun 2014 16:45:01 +0000 (UTC) Received: from mail-wg0-x229.google.com (mail-wg0-x229.google.com [IPv6:2a00:1450:400c:c00::229]) (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 9D1452B74; Mon, 23 Jun 2014 16:45:00 +0000 (UTC) Received: by mail-wg0-f41.google.com with SMTP id a1so6536748wgh.0 for ; Mon, 23 Jun 2014 09:44:59 -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=awJJd701eNs67cWd2qVDQ/kb4bFI/Au6Bw7SiqMJW74=; b=cROcxyaElJkVX0tP9Mkz9t7SJihbFl7NAp3ODZoxhqW6kzWLzwRsJS3R71H89TuqFP Y2FJ2bCiayjnNRYcs4mKSJCZPBwi0XGJ9mFaNy7HQdJSNO3TQWLQ8ibDf/JiBElZvucl /pg/O4cTlzLNPODKDj7a6nCWN5bRHklh5hSomkByD0Qyvb04j+/QnTB4bzpmrN8WiHMD HRQzsVoCSyxGYnLaACBzfQ3DcX8m4QtoE8CNJcTBdgjkeTDGMbR+JbSPZZL984i5Kgf/ PJflAon1Ui6uR/BeCBPLq3ZWo8RCTydHCZ2mbVQe0kY7B1hf8E85nxRTn/j5xXARS6B9 8Yxg== MIME-Version: 1.0 X-Received: by 10.180.149.175 with SMTP id ub15mr27471666wib.53.1403541898246; Mon, 23 Jun 2014 09:44:58 -0700 (PDT) Sender: asomers@gmail.com Received: by 10.194.168.202 with HTTP; Mon, 23 Jun 2014 09:44:58 -0700 (PDT) In-Reply-To: <20140623085229.GQ28199@FreeBSD.org> References: <53A48849.8080504@chelsio.com> <20140623085229.GQ28199@FreeBSD.org> Date: Mon, 23 Jun 2014 10:44:58 -0600 X-Google-Sender-Auth: vBzQ-1o3lL9B1mCKnrF8NZ9R8oI Message-ID: Subject: Re: ifaddr refcount problem From: Alan Somers To: Gleb Smirnoff Content-Type: text/plain; charset=UTF-8 Cc: "freebsd-net@freebsd.org" , Navdeep Parhar 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: Mon, 23 Jun 2014 16:45:01 -0000 On Mon, Jun 23, 2014 at 2:52 AM, Gleb Smirnoff wrote: > Navdeep, > > On Fri, Jun 20, 2014 at 12:15:21PM -0700, Navdeep Parhar wrote: > N> Revision 264905 and 266860 that followed it seem to leak ifaddr > N> references. ifa_ifwithdstaddr and ifa_ifwithnet both install a > N> reference on the ifaddr returned to the caller but ip_output does not > N> release it, eventually leading to a panic when the refcount wraps over > N> to 0 and the ifaddr is freed while it is still on various lists. > N> > N> I'm using this patch for now. Thoughts? > N> > N> Regards, > N> Navdeep > N> > N> > N> diff -r 6dfcecd314af sys/netinet/ip_output.c > N> --- a/sys/netinet/ip_output.c Fri Jun 20 10:33:22 2014 -0700 > N> +++ b/sys/netinet/ip_output.c Fri Jun 20 12:07:12 2014 -0700 > N> @@ -243,6 +243,7 @@ again: > N> ifp = ia->ia_ifp; > N> ip->ip_ttl = 1; > N> isbroadcast = 1; > N> + ifa_free((void *)ia); > N> } else if (flags & IP_ROUTETOIF) { > N> if ((ia = ifatoia(ifa_ifwithdstaddr(sintosa(dst)))) == NULL && > N> (ia = ifatoia(ifa_ifwithnet(sintosa(dst), 0))) == NULL) { > N> @@ -253,6 +254,7 @@ again: > N> ifp = ia->ia_ifp; > N> ip->ip_ttl = 1; > N> isbroadcast = in_broadcast(dst->sin_addr, ifp); > N> + ifa_free((void *)ia); > N> } else if (IN_MULTICAST(ntohl(ip->ip_dst.s_addr)) && > N> imo != NULL && imo->imo_multicast_ifp != NULL) { > N> /* > > The patch shouldn't use void * casts, but instead specify explicit member: > > ifa_free(&ia->ia_ifa); > > Apart from that it, the patch looks entirely correct and plugging a leak. > Thanks! I still don't see how this patch would work without breaking stuff like the statistics collection at line 673 of ip_output.c. If we call ifa_free immediately after choosing our ifp, then ia won't be available at lines 630 or 673, and ip_output will never record statistics, right? -Alan > > -- > Totus tuus, Glebius.