Subject: kern/37439: tap(4) assigning duplicate MAC addresses
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: None <mmondor@pulsar-zone.net>
List: netbsd-bugs
Date: 11/26/2007 22:20:00
>Number:         37439
>Category:       kern
>Synopsis:       tap(4) assigning duplicate MAC addresses
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Nov 26 22:20:00 +0000 2007
>Originator:     Matthew Mondor
>Release:        NetBSD 4.99.37
>Organization:
>Environment:
        System: NetBSD sat.xisop 4.99.37 NetBSD 4.99.37 (GENERIC_LAPTOP_MM) #4: Sat Nov 24 22:19:54 EST 2007  root@sat.xisop:/usr/src/sys/arch/i386/compile/GENERIC_LAPTOP_MM i386
Architecture: i386
Machine: i386
>Description:
	When creating multiple tap(4) interfaces in a short time frame,
	they are assigned the same MAC address.  This address is calculated
	from the current uptime trivially, not to simply use a hardcoded
	address.  However, it's not difficult to enhance the process.
	A diff is included showing an example.
>How-To-Repeat:
	Create two or more devices using /dev/tap.
>Fix:
	An example solution might look like (untested):


--- if_tap.c.orig	2007-09-10 06:35:54.000000000 -0400
+++ if_tap.c	2007-11-26 17:03:40.000000000 -0500
@@ -87,6 +87,14 @@ static int	tap_sysctl_handler(SYSCTLFN_P
 SYSCTL_SETUP_PROTO(sysctl_tap_setup);
 
 /*
+ * Since we use a trivial uptime based randomization to
+ * generate tap virtual MAC addresses, this reduces the
+ * likelyhood of having two interfaces with the same address
+ * when multiple interfaces are created rapidly.
+ */
+static uint32_t ui_cnt = 0;
+
+/*
  * Since we're an Ethernet device, we need the 3 following
  * components: a leading struct device, a struct ethercom,
  * and also a struct ifmedia since we don't attach a PHY to
@@ -258,9 +266,11 @@ tap_attach(struct device *parent, struct
 	 * In order to obtain unique initial Ethernet address on a host,
 	 * do some randomisation using the current uptime.  It's not meant
 	 * for anything but avoiding hard-coding an address.
+	 * We also use a counter in order to prevent two identical addresses
+	 * when creating them in a short time frame.
 	 */
 	getmicrouptime(&tv);
-	ui = (tv.tv_sec ^ tv.tv_usec) & 0xffffff;
+	ui = (++ui_cnt + (tv.tv_sec ^ tv.tv_usec)) & 0xffffff;
 	memcpy(enaddr+3, (u_int8_t *)&ui, 3);
 
 	aprint_verbose("%s: Ethernet address %s\n", device_xname(&sc->sc_dev),