From owner-freebsd-net@FreeBSD.ORG Tue Jan 3 23:17:04 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 0BBF0106564A; Tue, 3 Jan 2012 23:17:04 +0000 (UTC) (envelope-from bz@freebsd.org) Received: from mx1.sbone.de (mx1.sbone.de [IPv6:2a01:4f8:130:3ffc::401:25]) by mx1.freebsd.org (Postfix) with ESMTP id 849648FC0C; Tue, 3 Jan 2012 23:17:03 +0000 (UTC) Received: from mail.sbone.de (mail.sbone.de [IPv6:fde9:577b:c1a9:31::2013:587]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.sbone.de (Postfix) with ESMTPS id 3F2CC25D385D; Tue, 3 Jan 2012 23:17:02 +0000 (UTC) Received: from content-filter.sbone.de (content-filter.sbone.de [IPv6:fde9:577b:c1a9:31::2013:2742]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPS id 4686FBD86A8; Tue, 3 Jan 2012 23:17:01 +0000 (UTC) X-Virus-Scanned: amavisd-new at sbone.de Received: from mail.sbone.de ([IPv6:fde9:577b:c1a9:31::2013:587]) by content-filter.sbone.de (content-filter.sbone.de [fde9:577b:c1a9:31::2013:2742]) (amavisd-new, port 10024) with ESMTP id zM1hD3dvsWeB; Tue, 3 Jan 2012 23:16:59 +0000 (UTC) Received: from orange-en1.sbone.de (orange-en1.sbone.de [IPv6:fde9:577b:c1a9:31:cabc:c8ff:fecf:e8e3]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPSA id 74213BD86A7; Tue, 3 Jan 2012 23:16:59 +0000 (UTC) Mime-Version: 1.0 (Apple Message framework v1084) Content-Type: text/plain; charset=us-ascii From: "Bjoern A. Zeeb" In-Reply-To: <201201031722.11253.jhb@freebsd.org> Date: Tue, 3 Jan 2012 23:16:58 +0000 Content-Transfer-Encoding: quoted-printable Message-Id: <03930D32-DC79-49E1-96AF-0698704D28BE@freebsd.org> References: <201201031517.36251.jhb@freebsd.org> <201201031608.59688.jhb@freebsd.org> <20120104.071422.69305300858758112.hrs@allbsd.org> <201201031722.11253.jhb@freebsd.org> To: John Baldwin X-Mailer: Apple Mail (2.1084) Cc: Hiroki Sato , 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 23:17:04 -0000 On 3. Jan 2012, at 22:22 , John Baldwin wrote: > On Tuesday, January 03, 2012 5:14:22 pm Hiroki Sato wrote: >> John Baldwin wrote >> in <201201031608.59688.jhb@freebsd.org>: >>=20 >> 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. >>=20 >> Hmm, a fix for that bug was not merged for some reason. Something >> like the attached patch should be applied. >=20 > Ah, great, I've merged that into the patch, thanks! >=20 > Index: in.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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); >=20 > - /* 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 > } > } >=20 > + IF_ADDR_LOCK(ifp); > TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) { > - if (ifa->ifa_addr->sa_family !=3D AF_INET6) > + if (ifa->ifa_addr->sa_family !=3D AF_INET) > continue; > if (match.s_addr =3D=3D 0) > break; When doing "noise changes" like fixing comment in the same go (I'd = prefer that to be independent of the locking/refcount changes if possible, you could also wrap the long line: 792 candidate.s_addr =3D ((struct = sockaddr_in *)&ifa->ifa_addr)->sin_addr.s_addr; Otherwise looks good. > @@ -794,6 +795,9 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca > if (candidate.s_addr =3D=3D match.s_addr) > break; > } > + if (ifa !=3D NULL) > + ifa_ref(ifa); > + IF_ADDR_UNLOCK(ifp); > if (ifa =3D=3D NULL) > return (EADDRNOTAVAIL); > ia =3D (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); >=20 > iflr->flags =3D 0; /*XXX*/ > + ifa_free(ifa); >=20 > return (0); > } else { > struct in_aliasreq ifra; >=20 > - /* 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); >=20 > return (in_control(so, SIOCDIFADDR, = (caddr_t)&ifra, > ifp, td)); >=20 > --=20 > John Baldwin > _______________________________________________ > freebsd-net@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-net > To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" --=20 Bjoern A. Zeeb You have to have visions! It does not matter how good you are. It matters what good you do!