From owner-freebsd-net@FreeBSD.ORG Tue Jan 3 22:22:12 2012 Return-Path: Delivered-To: net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C90691065672; Tue, 3 Jan 2012 22:22:12 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 9F1C78FC1C; Tue, 3 Jan 2012 22:22:12 +0000 (UTC) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [96.47.65.170]) by cyrus.watson.org (Postfix) with ESMTPSA id 5471B46B06; Tue, 3 Jan 2012 17:22:12 -0500 (EST) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id D44F2B965; Tue, 3 Jan 2012 17:22:11 -0500 (EST) From: John Baldwin To: Hiroki Sato Date: Tue, 3 Jan 2012 17:22:11 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p8; KDE/4.5.5; amd64; ; ) References: <201201031517.36251.jhb@freebsd.org> <201201031608.59688.jhb@freebsd.org> <20120104.071422.69305300858758112.hrs@allbsd.org> In-Reply-To: <20120104.071422.69305300858758112.hrs@allbsd.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201201031722.11253.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Tue, 03 Jan 2012 17:22:11 -0500 (EST) Cc: bz@freebsd.org, pluknet@freebsd.org, net@freebsd.org Subject: Re: [PATCH] Use of unreferenced ifa in in6 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: Tue, 03 Jan 2012 22:22:12 -0000 On Tuesday, January 03, 2012 5:14:22 pm Hiroki Sato wrote: > John Baldwin wrote > in <201201031608.59688.jhb@freebsd.org>: > > jh> > With this patch in_lifaddr_ioctl() now looks more syntactically similar > jh> > to in6_lifaddr_ioctl(). They could look even more similar by eliminating > jh> > a lot of whitespace changes present here or there. > jh> > jh> Hmmm. Actually, it seems to be a bit more broken. Note that it is expecting > jh> to get a sockaddr_in, but it is checking for AF_INET6, not AF_INET in its > jh> loop. That bug seems to go back to the original import from KAME. I'm not > jh> sure if the two can be merged since they work on different underyling data > jh> structures though. > > Hmm, a fix for that bug was not merged for some reason. Something > like the attached patch should be applied. Ah, great, I've merged that into the patch, thanks! Index: in.c =================================================================== --- in.c (revision 229406) +++ in.c (working copy) @@ -735,7 +735,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca if (iflr->flags & IFLR_PREFIX) return (EINVAL); - /* copy args to in_aliasreq, perform ioctl(SIOCAIFADDR_IN6). */ + /* copy args to in_aliasreq, perform ioctl(SIOCAIFADDR). */ bzero(&ifra, sizeof(ifra)); bcopy(iflr->iflr_name, ifra.ifra_name, sizeof(ifra.ifra_name)); @@ -784,8 +784,9 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca } } + IF_ADDR_LOCK(ifp); TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { - if (ifa->ifa_addr->sa_family != AF_INET6) + if (ifa->ifa_addr->sa_family != AF_INET) continue; if (match.s_addr == 0) break; @@ -794,6 +795,9 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca if (candidate.s_addr == match.s_addr) break; } + if (ifa != NULL) + ifa_ref(ifa); + IF_ADDR_UNLOCK(ifp); if (ifa == NULL) return (EADDRNOTAVAIL); ia = (struct in_ifaddr *)ifa; @@ -812,12 +816,13 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca in_mask2len(&ia->ia_sockmask.sin_addr); iflr->flags = 0; /*XXX*/ + ifa_free(ifa); return (0); } else { struct in_aliasreq ifra; - /* fill in_aliasreq and do ioctl(SIOCDIFADDR_IN6) */ + /* fill in_aliasreq and do ioctl(SIOCDIFADDR) */ bzero(&ifra, sizeof(ifra)); bcopy(iflr->iflr_name, ifra.ifra_name, sizeof(ifra.ifra_name)); @@ -830,6 +835,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca } bcopy(&ia->ia_sockmask, &ifra.ifra_dstaddr, ia->ia_sockmask.sin_len); + ifa_free(ifa); return (in_control(so, SIOCDIFADDR, (caddr_t)&ifra, ifp, td)); -- John Baldwin