Date: Tue, 3 Jan 2012 23:16:58 +0000 From: "Bjoern A. Zeeb" <bz@freebsd.org> To: John Baldwin <jhb@freebsd.org> Cc: Hiroki Sato <hrs@freebsd.org>, pluknet@freebsd.org, net@freebsd.org Subject: Re: [PATCH] Use of unreferenced ifa in in6 Message-ID: <03930D32-DC79-49E1-96AF-0698704D28BE@freebsd.org> In-Reply-To: <201201031722.11253.jhb@freebsd.org> References: <201201031517.36251.jhb@freebsd.org> <201201031608.59688.jhb@freebsd.org> <20120104.071422.69305300858758112.hrs@allbsd.org> <201201031722.11253.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 3. Jan 2012, at 22:22 , John Baldwin wrote:
> On Tuesday, January 03, 2012 5:14:22 pm Hiroki Sato wrote:
>> John Baldwin <jhb@freebsd.org> 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!
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?03930D32-DC79-49E1-96AF-0698704D28BE>