[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#426756: ported pypxeboot patches to busybox udhcpc



Hi Michael,

On 13.05.2011 10:13, Michael Tokarev wrote:
So basically, the only modification this patch provides is
to add -M option to specify alternative MAC address, right?

Yes

I think it should be addressed upstream first, or at least
tried to.  I may take care of that.

That would be great!

But I've at least one comment about the code, see below.

This has 2 problems.  First, you can't cast uint8_t to unsigned int*,
the result is at least endian-dependent (where's the byte of int which
should go to our uint8 -- in the end of int or at the beginning of it),
and sscanf will access bytes which are not within the hwmac array.

Second, if you make a mistake/typo in the mac address, it will be
silently ignored - at least a warning should be printed in this
case.

Thanks for the good feedback! Both problems should have been adressed in the attached patches.

Regards,
Bjoern
diff -urN busybox-1.18.4/networking/udhcp/dhcpc.c busybox-1.18.4-patched/networking/udhcp/dhcpc.c
--- busybox-1.18.4/networking/udhcp/dhcpc.c	2011-03-13 02:45:40.000000000 +0100
+++ busybox-1.18.4-patched/networking/udhcp/dhcpc.c	2011-05-13 12:10:25.007347099 +0200
@@ -856,6 +856,7 @@
 //usage:     "\n	-i,--interface IFACE	Interface to use (default eth0)"
 //usage:     "\n	-p,--pidfile FILE	Create pidfile"
 //usage:     "\n	-s,--script PROG	Run PROG at DHCP events (default "CONFIG_UDHCPC_DEFAULT_SCRIPT")"
+//usage:     "\n	-M,--mac MAC		MAC address to use instead of HW MAC (example: 00:13:37:c0:ff:ee)"
 //usage:     "\n	-t,--retries N		Send up to N discover packets"
 //usage:     "\n	-T,--timeout N		Pause between packets (default 3 seconds)"
 //usage:     "\n	-A,--tryagain N		Wait N seconds after failure (default 20)"
@@ -931,7 +932,8 @@
 int udhcpc_main(int argc UNUSED_PARAM, char **argv)
 {
 	uint8_t *temp, *message;
-	const char *str_V, *str_h, *str_F, *str_r;
+	uint32_t hwmac[6];
+	const char *str_V, *str_h, *str_F, *str_r, *str_M;
 	IF_FEATURE_UDHCP_PORT(char *str_P;)
 	void *clientid_mac_ptr;
 	llist_t *list_O = NULL;
@@ -966,6 +968,7 @@
 		"release\0"        No_argument       "R"
 		"request\0"        Required_argument "r"
 		"script\0"         Required_argument "s"
+		"mac\0"            Required_argument "M"
 		"timeout\0"        Required_argument "T"
 		"version\0"        No_argument       "v"
 		"retries\0"        Required_argument "t"
@@ -992,16 +995,17 @@
 		OPT_R = 1 << 9,
 		OPT_r = 1 << 10,
 		OPT_s = 1 << 11,
-		OPT_T = 1 << 12,
-		OPT_t = 1 << 13,
-		OPT_S = 1 << 14,
-		OPT_A = 1 << 15,
-		OPT_O = 1 << 16,
-		OPT_o = 1 << 17,
-		OPT_x = 1 << 18,
-		OPT_f = 1 << 19,
+		OPT_M = 1 << 12,
+		OPT_T = 1 << 13,
+		OPT_t = 1 << 14,
+		OPT_S = 1 << 15,
+		OPT_A = 1 << 16,
+		OPT_O = 1 << 17,
+		OPT_o = 1 << 18,
+		OPT_x = 1 << 19,
+		OPT_f = 1 << 20,
 /* The rest has variable bit positions, need to be clever */
-		OPTBIT_f = 19,
+		OPTBIT_f = 20,
 		USE_FOR_MMU(             OPTBIT_b,)
 		IF_FEATURE_UDHCPC_ARPING(OPTBIT_a,)
 		IF_FEATURE_UDHCP_PORT(   OPTBIT_P,)
@@ -1025,7 +1029,7 @@
 #endif
 		;
 	IF_LONG_OPTS(applet_long_options = udhcpc_longopts;)
-	opt = getopt32(argv, "CV:H:h:F:i:np:qRr:s:T:t:SA:O:ox:f"
+	opt = getopt32(argv, "CV:H:h:F:i:np:qRr:s:M:T:t:SA:O:ox:f"
 		USE_FOR_MMU("b")
 		IF_FEATURE_UDHCPC_ARPING("a")
 		IF_FEATURE_UDHCP_PORT("P:")
@@ -1033,6 +1037,7 @@
 		, &str_V, &str_h, &str_h, &str_F
 		, &client_config.interface, &client_config.pidfile, &str_r /* i,p */
 		, &client_config.script /* s */
+		, &str_M /* M */
 		, &discover_timeout, &discover_retries, &tryagain_timeout /* T,t,A */
 		, &list_O
 		, &list_x
@@ -1092,6 +1097,27 @@
 		return 1;
 	}
 
+	if (opt & (OPT_M)) {
+		int nrmacfields = sscanf(str_M,"%x:%x:%x:%x:%x:%x",
+						&hwmac[0],
+						&hwmac[1],
+						&hwmac[2],
+						&hwmac[3],
+						&hwmac[4],
+						&hwmac[5]);
+		if (nrmacfields == 6) {
+			for (int i = 0; i < 6; ++i) {
+				if (hwmac[i] > 255) {
+					bb_info_msg("Invalid MAC segment: %x, exiting...", hwmac[i]);
+					return 1;
+				}
+				client_config.client_mac[i] = (uint8_t) hwmac[i];
+			}
+		} else {
+			bb_info_msg("Invalid MAC address: %s, exiting...", str_M);
+			return 1;
+		}
+	}
 	clientid_mac_ptr = NULL;
 	if (!(opt & OPT_C) && !udhcp_find_option(client_config.options, DHCP_CLIENT_ID)) {
 		/* not suppressed and not set, set the default client ID */
diff -urN busybox-1.17.1-debianorig/include/usage.src.h busybox-1.17.1-patched/include/usage.src.h
--- busybox-1.17.1-debianorig/include/usage.src.h	2010-07-25 00:12:43.000000000 +0200
+++ busybox-1.17.1-patched/include/usage.src.h	2011-05-12 14:47:10.601425094 +0200
@@ -4522,6 +4522,7 @@
      "\n	-p,--pidfile FILE	Create pidfile" \
      "\n	-r,--request IP		IP address to request" \
      "\n	-s,--script PROG	Run PROG at DHCP events (default "CONFIG_UDHCPC_DEFAULT_SCRIPT")" \
+     "\n	-M,--mac MAC		MAC address to use instead of HW MAC (example: 00:13:37:c0:ff:ee)" \
      "\n	-t,--retries N		Send up to N discover packets" \
      "\n	-T,--timeout N		Pause between packets (default 3 seconds)" \
      "\n	-A,--tryagain N		Wait N seconds after failure (default 20)" \
@@ -4553,6 +4554,7 @@
      "\n	-p FILE		Create pidfile" \
      "\n	-r IP		IP address to request" \
      "\n	-s PROG		Run PROG at DHCP events (default "CONFIG_UDHCPC_DEFAULT_SCRIPT")" \
+     "\n	-M MAC		MAC address to use instead of HW MAC (example: 00:13:37:c0:ff:ee)" \
      "\n	-t N		Send up to N discover packets" \
      "\n	-T N		Pause between packets (default 3 seconds)" \
      "\n	-A N		Wait N seconds (default 20) after failure" \
diff -urN busybox-1.17.1-debianorig/networking/udhcp/dhcpc.c busybox-1.17.1-patched/networking/udhcp/dhcpc.c
--- busybox-1.17.1-debianorig/networking/udhcp/dhcpc.c	2010-07-06 04:25:54.000000000 +0200
+++ busybox-1.17.1-patched/networking/udhcp/dhcpc.c	2011-05-13 12:05:21.370370159 +0200
@@ -766,7 +766,8 @@
 int udhcpc_main(int argc UNUSED_PARAM, char **argv)
 {
 	uint8_t *temp, *message;
-	const char *str_c, *str_V, *str_h, *str_F, *str_r;
+	uint32_t hwmac[6];
+	const char *str_c, *str_V, *str_h, *str_F, *str_r, *str_M;
 	IF_FEATURE_UDHCP_PORT(char *str_P;)
 	llist_t *list_O = NULL;
 	llist_t *list_x = NULL;
@@ -801,6 +802,7 @@
 		"release\0"        No_argument       "R"
 		"request\0"        Required_argument "r"
 		"script\0"         Required_argument "s"
+		"mac\0"            Required_argument "M"
 		"timeout\0"        Required_argument "T"
 		"version\0"        No_argument       "v"
 		"retries\0"        Required_argument "t"
@@ -828,16 +830,17 @@
 		OPT_R = 1 << 10,
 		OPT_r = 1 << 11,
 		OPT_s = 1 << 12,
-		OPT_T = 1 << 13,
-		OPT_t = 1 << 14,
-		OPT_S = 1 << 15,
-		OPT_A = 1 << 16,
-		OPT_O = 1 << 17,
-		OPT_o = 1 << 18,
-		OPT_x = 1 << 19,
-		OPT_f = 1 << 20,
+		OPT_M = 1 << 13,
+		OPT_T = 1 << 14,
+		OPT_t = 1 << 15,
+		OPT_S = 1 << 16,
+		OPT_A = 1 << 17,
+		OPT_O = 1 << 18,
+		OPT_o = 1 << 19,
+		OPT_x = 1 << 20,
+		OPT_f = 1 << 21,
 /* The rest has variable bit positions, need to be clever */
-		OPTBIT_f = 20,
+		OPTBIT_f = 21,
 		USE_FOR_MMU(             OPTBIT_b,)
 		IF_FEATURE_UDHCPC_ARPING(OPTBIT_a,)
 		IF_FEATURE_UDHCP_PORT(   OPTBIT_P,)
@@ -861,7 +864,7 @@
 #endif
 		;
 	IF_LONG_OPTS(applet_long_options = udhcpc_longopts;)
-	opt = getopt32(argv, "c:CV:H:h:F:i:np:qRr:s:T:t:SA:O:ox:f"
+	opt = getopt32(argv, "c:CV:H:h:F:i:np:qRr:s:M:T:t:SA:O:ox:f"
 		USE_FOR_MMU("b")
 		IF_FEATURE_UDHCPC_ARPING("a")
 		IF_FEATURE_UDHCP_PORT("P:")
@@ -869,6 +872,7 @@
 		, &str_c, &str_V, &str_h, &str_h, &str_F
 		, &client_config.interface, &client_config.pidfile, &str_r /* i,p */
 		, &client_config.script /* s */
+		, &str_M /* M */
 		, &discover_timeout, &discover_retries, &tryagain_timeout /* T,t,A */
 		, &list_O
 		, &list_x
@@ -928,6 +932,27 @@
 		return 1;
 	}
 
+	if (opt & (OPT_M)) {
+		int nrmacfields = sscanf(str_M,"%x:%x:%x:%x:%x:%x", 
+					        &hwmac[0],
+						&hwmac[1],
+						&hwmac[2],
+						&hwmac[3],
+						&hwmac[4],
+						&hwmac[5]);
+		if (nrmacfields == 6) {
+			for (int i = 0; i < 6; ++i) {
+				if (hwmac[i] > 255) {
+					bb_info_msg("Invalid MAC segment: %x, exiting...", hwmac[i]);
+					return 1;
+				}
+				client_config.client_mac[i] = (uint8_t) hwmac[i];
+			}
+		} else {
+			bb_info_msg("Invalid MAC address: %s, exiting...", str_M);
+			return 1;
+		}
+	}
 	if (opt & OPT_c) {
 		client_config.clientid = alloc_dhcp_option(DHCP_CLIENT_ID, str_c, 0);
 	} else if (!(opt & OPT_C)) {

Reply to: