[PATCH] Use of unreferenced ifa in in6

[John Baldwin]

On Tuesday, January 03, 2012 12:35:30 pm Bjoern A. Zeeb wrote:
> 
> On 23. Dec 2011, at 20:08 , John Baldwin wrote:
> 
> > The code to handle the SIOCGLIFADDR and SIOCDLIFADDR ioctls in 
> > in6_lifaddr_ioctl() does not grab a reference to an ifnet address structure 
> > that it uses after dropping the IF_ADDR_LOCK().  Based on other code that uses 
> > a similar pattern of finding an ifa while under the lock and then using it 
> > after dropping the lock, I believe it should be acquiring a reference on the 
> > ifa and then dropping that reference when it is done using the ifa.  This 
> > (untested) patch should fix this I believe:
> 
> I almost assume it's been tested by now.  From reading it looks right.

Hmm, I don't have a good way to test it. :(  I've booted a GENERIC kernel
with it, but I don't have IPv6 setup for my test machines.

> /bz
> 
> > Index: in6.c
> > ===================================================================
> > --- in6.c	(revision 228777)
> > +++ in6.c	(working copy)
> > @@ -1767,6 +1767,8 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> > 			if (IN6_ARE_ADDR_EQUAL(&candidate, &match))
> > 				break;
> > 		}
> > +		if (ifa != NULL)
> > +			ifa_ref(ifa);
> > 		IF_ADDR_UNLOCK(ifp);
> > 		if (!ifa)
> > 			return EADDRNOTAVAIL;
> > @@ -1779,16 +1781,20 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> > 			bcopy(&ia->ia_addr, &iflr->addr, ia->ia_addr.sin6_len);
> > 			error = sa6_recoverscope(
> > 			    (struct sockaddr_in6 *)&iflr->addr);
> > -			if (error != 0)
> > +			if (error != 0) {
> > +				ifa_free(ifa);
> > 				return (error);
> > +			}
> > 
> > 			if ((ifp->if_flags & IFF_POINTOPOINT) != 0) {
> > 				bcopy(&ia->ia_dstaddr, &iflr->dstaddr,
> > 				    ia->ia_dstaddr.sin6_len);
> > 				error = sa6_recoverscope(
> > 				    (struct sockaddr_in6 *)&iflr->dstaddr);
> > -				if (error != 0)
> > +				if (error != 0) {
> > +					ifa_free(ifa);
> > 					return (error);
> > +				}
> > 			} else
> > 				bzero(&iflr->dstaddr, sizeof(iflr->dstaddr));
> > 
> > @@ -1796,6 +1802,7 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> > 			    in6_mask2len(&ia->ia_prefixmask.sin6_addr, NULL);
> > 
> > 			iflr->flags = ia->ia6_flags;	/* XXX */
> > +			ifa_free(ifa);
> > 
> > 			return 0;
> > 		} else {
> > @@ -1819,6 +1826,7 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> > 			    ia->ia_prefixmask.sin6_len);
> > 
> > 			ifra.ifra_flags = ia->ia6_flags;
> > +			ifa_free(ifa);
> > 			return in6_control(so, SIOCDIFADDR_IN6, (caddr_t)&ifra,
> > 			    ifp, td);
> > 		}
> > 
> > 
> > -- 
> > John Baldwin
> 
> -- 
> Bjoern A. Zeeb                                 You have to have visions!
>    It does not matter how good you are. It matters what good you do!
> 
> 

-- 
John Baldwin