Subject: Re: rt_tables broken
To: None <current-users@netbsd.org>
From: David Young <dyoung@pobox.com>
List: current-users
Date: 08/04/2007 03:25:13
--k+w/mQv8wyuph6w0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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. :-)

Dave

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

--k+w/mQv8wyuph6w0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="dl_setaddr.patch"

Index: 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
--- net/if_dl.h	11 Dec 2005 23:05:24 -0000	1.18
+++ net/if_dl.h	4 Aug 2007 08:08:56 -0000
@@ -78,7 +78,10 @@ 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
+struct sockaddr_dl *sockaddr_dl_setaddr(struct sockaddr_dl *, const void *buf,
+    uint8_t);
+#else
 
 #include <sys/cdefs.h>
 
Index: 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
--- net/link_proto.c	19 Jul 2007 20:48:52 -0000	1.1
+++ net/link_proto.c	4 Aug 2007 08:08:56 -0000
@@ -158,3 +158,21 @@ sockaddr_dl_cmp(const struct sockaddr *s
 
 	return sdl1->sdl_len - sdl2->sdl_len;
 }
+
+struct sockaddr_dl *
+sockaddr_dl_setaddr(struct sockaddr_dl *sdl, const void *buf, uint8_t len) 
+{
+	fast_uint8_t endofs;
+
+	endofs = offsetof(struct sockaddr_dl, sdl_data[sdl->sdl_nlen + len]);
+
+	if (endofs > sizeof(struct sockaddr_dl)) {
+		printf("%s: no room for %" PRIu8 "-byte media address\n",
+		    __func__, len);
+		sdl->sdl_alen = 0;
+		return NULL;
+	}
+	memcpy(&sdl->sdl_data[sdl->sdl_nlen], buf, len);
+	sdl->sdl_alen = len;
+	return sdl;
+}
Index: 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
--- netinet6/nd6.c	19 Jul 2007 20:48:57 -0000	1.117
+++ netinet6/nd6.c	4 Aug 2007 08:08:57 -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: 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
--- netinet/if_arp.c	19 Jul 2007 20:48:53 -0000	1.125
+++ netinet/if_arp.c	4 Aug 2007 08:18:54 -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;

--k+w/mQv8wyuph6w0--