From owner-freebsd-net@FreeBSD.ORG Wed Jul 9 04:24:11 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 99340180; Wed, 9 Jul 2014 04:24:11 +0000 (UTC) Received: from stargate.chelsio.com (99-65-72-228.uvs.sntcca.sbcglobal.net [99.65.72.228]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 7047F2D61; Wed, 9 Jul 2014 04:24:10 +0000 (UTC) Received: from nice.asicdesigners.com (nice.asicdesigners.com [10.192.160.7]) by stargate.chelsio.com (8.13.8/8.13.8) with ESMTP id s694OAc3029609; Tue, 8 Jul 2014 21:24:10 -0700 Received: from dwarf.asicdesigners.com (10.192.166.0) by nice.asicdesigners.com (10.192.160.7) with Microsoft SMTP Server (TLS) id 14.2.247.3; Tue, 8 Jul 2014 21:24:09 -0700 Date: Tue, 8 Jul 2014 21:24:06 -0700 From: Navdeep Parhar To: Gleb Smirnoff , Alan Somers Subject: Re: ifaddr refcount problem Message-ID: <20140709042406.GA64350@dwarf.asicdesigners.com> References: <53A48849.8080504@chelsio.com> <20140623085229.GQ28199@FreeBSD.org> <20140624090847.GB28199@glebius.int.ru> <20140625100835.GK28199@glebius.int.ru> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20140625100835.GK28199@glebius.int.ru> User-Agent: Mutt/1.5.23 (2014-03-12) X-Originating-IP: [10.192.166.0] 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: Wed, 09 Jul 2014 04:24:11 -0000 I got distracted by some other issues and lost track of this thread. Can one of you please commit the fix if the discussion has reached a conclusion? Thanks! Regards, Navdeep On Wed, Jun 25, 2014 at 02:08:35PM +0400, Gleb Smirnoff wrote: > Alan, > > On Tue, Jun 24, 2014 at 08:43:40AM -0600, Alan Somers wrote: > A> That looks better. But I think there is one more possibility for a > A> leak. For multicast packets, IFP_TO_IA at line 263 will call > A> ifa_ref(), unless the the interface has no address assigned. How > A> about this patch instead? Also, not tested. > > Again you are right, thanks. > > The patch needs to reset have_ia_ref in the 'goto again' case. > > Also, I'd suggest a tiny change to your patch so that we don't > have a broken logic reading the code as "I have ia reference, > but don't have ia". > > -- > Totus tuus, Glebius. > Index: ip_output.c > =================================================================== > --- ip_output.c (revision 267536) > +++ ip_output.c (working copy) > @@ -136,6 +136,7 @@ ip_output(struct mbuf *m, struct mbuf *opt, struct > struct rtentry *rte; /* cache for ro->ro_rt */ > struct in_addr odst; > struct m_tag *fwd_tag = NULL; > + int have_ia_ref; > #ifdef IPSEC > int no_route_but_check_spd = 0; > #endif > @@ -202,6 +203,7 @@ ip_output(struct mbuf *m, struct mbuf *opt, struct > gw = dst = (struct sockaddr_in *)&ro->ro_dst; > again: > ia = NULL; > + have_ia_ref = 0; > /* > * If there is a cached route, check that it is to the same > * destination and is still up. If not, free it and try again. > @@ -238,6 +240,7 @@ again: > error = ENETUNREACH; > goto bad; > } > + have_ia_ref = 1; > ip->ip_dst.s_addr = INADDR_BROADCAST; > dst->sin_addr = ip->ip_dst; > ifp = ia->ia_ifp; > @@ -250,6 +253,7 @@ again: > error = ENETUNREACH; > goto bad; > } > + have_ia_ref = 1; > ifp = ia->ia_ifp; > ip->ip_ttl = 1; > isbroadcast = in_broadcast(dst->sin_addr, ifp); > @@ -261,6 +265,8 @@ again: > */ > ifp = imo->imo_multicast_ifp; > IFP_TO_IA(ifp, ia); > + if (ia) > + have_ia_ref = 1; > isbroadcast = 0; /* fool gcc */ > } else { > /* > @@ -552,8 +558,11 @@ sendit: > #endif > error = netisr_queue(NETISR_IP, m); > goto done; > - } else > + } else { > + if (have_ia_ref) > + ifa_free(&ia->ifa); > goto again; /* Redo the routing table lookup. */ > + } > } > > /* See if local, if yes, send it to netisr with IP_FASTFWD_OURS. */ > @@ -582,6 +591,8 @@ sendit: > m->m_flags |= M_SKIP_FIREWALL; > m->m_flags &= ~M_IP_NEXTHOP; > m_tag_delete(m, fwd_tag); > + if (have_ia_ref) > + ifa_free(&ia->ifa); > goto again; > } > > @@ -694,6 +705,8 @@ passout: > done: > if (ro == &iproute) > RO_RTFREE(ro); > + if (have_ia_ref) > + ifa_free(&ia->ifa); > return (error); > bad: > m_freem(m);