--5/uDoXvLw7AC5HRs
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sat, Aug 04, 2007 at 03:25:13AM -0500, David Young wrote:
> On Sat, Aug 04, 2007 at 03:47:12PM +0900, KIYOHARA Takashi wrote:
> > Hi! dyoung,
> > 
> > 
> > The rt_tables was broken since 07/19/2007 20:49:00 perhaps.  X-<
> > 
> >   http://mail-index.netbsd.org/source-changes/2007/07/19/0032.html
> 
> The bug is not new since 19 July 2007.  The kernel has been sloppy
> about copying link-layer addresses for a long time.  fwip(4) suffers
> the buffer overflow because its link-layer addresses are the longest.
> I have attached a patch that shows an approach to a fix for the bugs.
> I leave it to you to make the patch compile and run. :-)

Here is a new patch, also untested.  Lengthening sockaddr_dl is still
left as an exercise for the reader. :-)

Dave

-- 
David Young             OJC Technologies
dyoung@ojctech.com      Urbana, IL * (217) 278-3933 ext 24

--5/uDoXvLw7AC5HRs
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="dl_setaddr.patch2"

Index: sys/net/if_dl.h
===================================================================
RCS file: /cvsroot/src/sys/net/if_dl.h,v
retrieving revision 1.18
diff -p -u -u -p -r1.18 if_dl.h
--- sys/net/if_dl.h	11 Dec 2005 23:05:24 -0000	1.18
+++ sys/net/if_dl.h	5 Aug 2007 19:42:46 -0000
@@ -78,7 +78,12 @@ struct sockaddr_dl {
 #define LLADDR(s) ((char *)((s)->sdl_data + (s)->sdl_nlen))
 #define CLLADDR(s) ((const char *)((s)->sdl_data + (s)->sdl_nlen))
 
-#ifndef _KERNEL
+#ifdef _KERNEL
+uint8_t sockaddr_dl_measure(uint8_t, uint8_t);
+void sockaddr_dl_init(struct sockaddr_dl *, uint16_t, uint8_t,
+    const void *, uint8_t, const void *, uint8_t);
+struct sockaddr_dl *sockaddr_dl_setaddr(struct sockaddr_dl *, void *, uint8_t);
+#else
 
 #include <sys/cdefs.h>
 
Index: sys/net/if.c
===================================================================
RCS file: /cvsroot/src/sys/net/if.c,v
retrieving revision 1.194
diff -p -u -u -p -r1.194 if.c
--- sys/net/if.c	19 Jul 2007 20:48:52 -0000	1.194
+++ sys/net/if.c	5 Aug 2007 19:42:46 -0000
@@ -271,8 +271,8 @@ void
 if_alloc_sadl(struct ifnet *ifp)
 {
 	unsigned socksize, ifasize;
-	int namelen, masklen;
-	struct sockaddr_dl *sdl;
+	int addrlen, namelen;
+	struct sockaddr_dl *mask, *sdl;
 	struct ifaddr *ifa;
 
 	/*
@@ -284,23 +284,23 @@ if_alloc_sadl(struct ifnet *ifp)
 		if_free_sadl(ifp);
 
 	namelen = strlen(ifp->if_xname);
-	masklen = offsetof(struct sockaddr_dl, sdl_data[0]) + namelen;
-	socksize = masklen + ifp->if_addrlen;
-#define ROUNDUP(a) (1 + (((a) - 1) | (sizeof(long) - 1)))
-	if (socksize < sizeof(*sdl))
-		socksize = sizeof(*sdl);
-	socksize = ROUNDUP(socksize);
+	addrlen = ifp->if_addrlen;
+	socksize = roundup(
+	    MAX(sizeof(*sdl),
+	    sockaddr_dl_measure(namelen, addrlen)), sizeof(long));
 	ifasize = sizeof(*ifa) + 2 * socksize;
 	ifa = (struct ifaddr *)malloc(ifasize, M_IFADDR, M_WAITOK);
-	memset((void *)ifa, 0, ifasize);
+	memset(ifa, 0, ifasize);
+
 	sdl = (struct sockaddr_dl *)(ifa + 1);
-	sdl->sdl_len = socksize;
-	sdl->sdl_family = AF_LINK;
-	memcpy(sdl->sdl_data, ifp->if_xname, namelen);
-	sdl->sdl_nlen = namelen;
-	sdl->sdl_alen = ifp->if_addrlen;
-	sdl->sdl_index = ifp->if_index;
-	sdl->sdl_type = ifp->if_type;
+	mask = (struct sockaddr_dl *)(socksize + (char *)sdl);
+
+	sockaddr_dl_init(sdl, ifp->if_index, ifp->if_type,
+	    ifp->if_xname, namelen, NULL, addrlen);
+	mask->sdl_len = sockaddr_dl_measure(namelen, 0);
+	while (namelen != 0)
+		mask->sdl_data[--namelen] = 0xff;
+
 	ifnet_addrs[ifp->if_index] = ifa;
 	IFAREF(ifa);
 	ifa->ifa_ifp = ifp;
@@ -309,11 +309,7 @@ if_alloc_sadl(struct ifnet *ifp)
 	IFAREF(ifa);
 	ifa->ifa_addr = (struct sockaddr *)sdl;
 	ifp->if_sadl = sdl;
-	sdl = (struct sockaddr_dl *)(socksize + (char *)sdl);
-	ifa->ifa_netmask = (struct sockaddr *)sdl;
-	sdl->sdl_len = masklen;
-	while (namelen != 0)
-		sdl->sdl_data[--namelen] = 0xff;
+	ifa->ifa_netmask = (struct sockaddr *)mask;
 }
 
 /*
Index: sys/net/link_proto.c
===================================================================
RCS file: /cvsroot/src/sys/net/link_proto.c,v
retrieving revision 1.1
diff -p -u -u -p -r1.1 link_proto.c
--- sys/net/link_proto.c	19 Jul 2007 20:48:52 -0000	1.1
+++ sys/net/link_proto.c	5 Aug 2007 19:42:46 -0000
@@ -110,6 +110,29 @@ submemcmp(const void *l, const void *r,
 	return 0;
 }
 
+uint8_t
+sockaddr_dl_measure(uint8_t namelen, uint8_t addrlen)
+{
+	return offsetof(struct sockaddr_dl, sdl_data[0]) + namelen + addrlen;
+}
+
+void
+sockaddr_dl_init(struct sockaddr_dl *sdl, uint16_t ifindex, uint8_t type,
+    const void *name, uint8_t namelen, const void *addr, uint8_t addrlen)
+{
+	sdl->sdl_family = AF_LINK;
+	sdl->sdl_len = sockaddr_dl_measure(namelen, addrlen);
+	KASSERT(sdl->sdl_len <= sizeof(*sdl));
+	sdl->sdl_index = ifindex;
+	sdl->sdl_type = type;
+	memset(&sdl->sdl_data[0], 0, sizeof(sdl->sdl_data));
+	memcpy(&sdl->sdl_data[0], name, namelen);
+	sdl->sdl_nlen = namelen;
+	if (addr != NULL)
+		memcpy(&sdl->sdl_data[sdl->sdl_nlen], addr, addrlen);
+	sdl->sdl_alen = addrlen;
+}
+
 static int
 sockaddr_dl_cmp(const struct sockaddr *sa1, const struct sockaddr *sa2)
 {
@@ -158,3 +181,22 @@ sockaddr_dl_cmp(const struct sockaddr *s
 
 	return sdl1->sdl_len - sdl2->sdl_len;
 }
+
+struct sockaddr_dl *
+sockaddr_dl_setaddr(struct sockaddr_dl *sdl, void *addr, uint8_t addrlen)
+{
+	uint_fast8_t endofs;
+
+	endofs =
+	    offsetof(struct sockaddr_dl, sdl_data[sdl->sdl_nlen + addrlen]);
+
+	if (endofs > sizeof(struct sockaddr_dl)) {
+		printf("%s: no room for %" PRIu8 "-byte media address\n",
+		    __func__, addrlen);
+		sdl->sdl_alen = 0;
+		return NULL;
+	}
+	memcpy(&sdl->sdl_data[sdl->sdl_nlen], addr, addrlen);
+	sdl->sdl_alen = addrlen;
+	return sdl;
+}
Index: sys/netinet6/nd6.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/nd6.c,v
retrieving revision 1.117
diff -p -u -u -p -r1.117 nd6.c
--- sys/netinet6/nd6.c	19 Jul 2007 20:48:57 -0000	1.117
+++ sys/netinet6/nd6.c	5 Aug 2007 19:42:46 -0000
@@ -1314,8 +1314,9 @@ nd6_rtrequest(int req, struct rtentry *r
 			ln->ln_state = ND6_LLINFO_REACHABLE;
 			ln->ln_byhint = 0;
 			if (macp) {
-				bcopy(macp, LLADDR(SDL(gate)), ifp->if_addrlen);
-				SDL(gate)->sdl_alen = ifp->if_addrlen;
+				/* XXX check for error */
+				(void)sockaddr_dl_setaddr(SDL(gate), macp,
+				    ifp->if_addrlen);
 			}
 			if (nd6_useloopback) {
 				rt->rt_ifp = lo0ifp;	/* XXX */
@@ -1738,8 +1739,8 @@ fail:
 		 * Record source link-layer address
 		 * XXX is it dependent to ifp->if_type?
 		 */
-		sdl->sdl_alen = ifp->if_addrlen;
-		bcopy(lladdr, LLADDR(sdl), ifp->if_addrlen);
+		/* XXX check for error */
+		(void)sockaddr_dl_setaddr(sdl, lladdr, ifp->if_addrlen);
 	}
 
 	if (!is_newentry) {
@@ -2130,7 +2131,8 @@ nd6_storelladdr(const struct ifnet *ifp,
 			ETHER_MAP_IPV6_MULTICAST(&SIN6(dst)->sin6_addr, lldst);
 			return 1;
 		case IFT_IEEE1394:
-			bcopy(ifp->if_broadcastaddr, lldst, ifp->if_addrlen);
+			memcpy(lldst, ifp->if_broadcastaddr,
+			    MIN(dstsize, ifp->if_addrlen));
 			return 1;
 		case IFT_ARCNET:
 			*lldst = 0;
Index: sys/netinet/if_arp.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/if_arp.c,v
retrieving revision 1.125
diff -p -u -u -p -r1.125 if_arp.c
--- sys/netinet/if_arp.c	19 Jul 2007 20:48:53 -0000	1.125
+++ sys/netinet/if_arp.c	5 Aug 2007 19:42:47 -0000
@@ -575,9 +596,9 @@ arp_rtrequest(int req, struct rtentry *r
 			 * interface.
 			 */
 			rt->rt_expire = 0;
-			Bcopy(LLADDR(rt->rt_ifp->if_sadl),
-			    LLADDR(SDL(gate)),
-			    SDL(gate)->sdl_alen = rt->rt_ifp->if_addrlen);
+			(void)sockaddr_dl_setaddr(SDL(gate),
+			    LLADDR(rt->rt_ifp->if_sadl),
+			    rt->rt_ifp->if_addrlen);
 			if (useloopback)
 				rt->rt_ifp = lo0ifp;
 			/*
@@ -1069,7 +1087,7 @@ in_arpinput(struct mbuf *m)
 			}
 		}
 #endif /* NTOKEN > 0 */
-		memcpy(LLADDR(sdl), ar_sha(ah), sdl->sdl_alen = ah->ar_hln);
+		(void)sockaddr_dl_setaddr(sdl, ar_sha(ah), ah->ar_hln);
 		if (rt->rt_expire)
 			rt->rt_expire = time_second + arpt_keep;
 		rt->rt_flags &= ~RTF_REJECT;
Index: sys/kern/uipc_domain.c
===================================================================
RCS file: /cvsroot/src/sys/kern/uipc_domain.c,v
retrieving revision 1.67
diff -p -u -u -p -r1.67 uipc_domain.c
--- sys/kern/uipc_domain.c	19 Jul 2007 20:48:51 -0000	1.67
+++ sys/kern/uipc_domain.c	5 Aug 2007 20:06:54 -0000
@@ -204,11 +204,28 @@ sockaddr_alloc(sa_family_t af, int flags
 	return sa;
 }
 
+static void
+sockaddr_fixlen(struct sockaddr *dst, uint8_t deslen)
+{
+	struct domain *dom;
+
+	if ((dom = pffinddomain(dst->sa_family)) == NULL)
+		panic("%s: unknown domain %d", __func__, dst->sa_family);
+	if (dom->dom_sa_len < deslen)
+		panic("%s: source too long, %d bytes", __func__, deslen);
+	dst->sa_len = dom->dom_sa_len;
+}
+
 struct sockaddr *
 sockaddr_copy(struct sockaddr *dst, const struct sockaddr *src)
 {
 	KASSERT(dst->sa_family == src->sa_family);
+
+	if (__predict_false(dst->sa_len < src->sa_len))
+		sockaddr_fixlen(dst, src->sa_len);
+
 	memcpy(dst, src, src->sa_len);
+
 	return dst;
 }
 

--5/uDoXvLw7AC5HRs--