[PATCH] Use of unreferenced ifa in in6

[John Baldwin]

On Tuesday, January 03, 2012 5:14:22 pm Hiroki Sato wrote:
> John Baldwin <jhb at freebsd.org> wrote
>   in <201201031608.59688.jhb at 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