[PATCH 2/2] udhcpc: fixed a TODO in fill_envp using option scanner

Martin Lewis martin.lewis.x84 at gmail.com
Tue Jun 23 20:25:09 UTC 2020


fill_envp now iterates over the packet only once instead of a few hundred times
using the new option scanner.

Signed-off-by: Martin Lewis <martin.lewis.x84 at gmail.com>
---
 networking/udhcp/dhcpc.c | 201 ++++++++++++++++++++---------------------------
 1 file changed, 87 insertions(+), 114 deletions(-)

diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c
index 102178a4f..2669428e1 100644
--- a/networking/udhcp/dhcpc.c
+++ b/networking/udhcp/dhcpc.c
@@ -386,59 +386,81 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_
 	return ret;
 }
 
+static void putenvp(llist_t **envp, char *new_opt)
+{
+	putenv(new_opt);
+	llist_add_to(envp, new_opt);
+}
+
+static int is_unknown_opt(uint8_t code, const struct dhcp_optflag **dh, const char **opt_name)
+{
+    *opt_name = "";
+
+    /* Find the option:
+     * dhcp_optflags is sorted so we stop searching when dh->code >= code, which is faster
+     * than iterating over the entire array.
+     * Options which don't have a match in dhcp_option_strings[], e.g DHCP_REQUESTED_IP,
+     * are located after the sorted array, so these entries will never be reached
+     * and they'll count as unknown options.
+     */
+    for (*dh = dhcp_optflags; (*dh)->code && (*dh)->code < code; (*dh)++);
+
+    if ((*dh)->code == code)
+        *opt_name = nth_string(dhcp_option_strings, (*dh - dhcp_optflags));
+
+    return (*dh)->code > code;
+}
+
 /* put all the parameters into the environment */
-static char **fill_envp(struct dhcp_packet *packet)
+static llist_t *fill_envp(struct dhcp_packet *packet)
 {
-	int envc;
-	int i;
-	char **envp, **curr;
-	const char *opt_name;
-	uint8_t *temp;
-	uint8_t overload = 0;
-
-#define BITMAP unsigned
-#define BBITS (sizeof(BITMAP) * 8)
-#define BMASK(i) (1 << (i & (sizeof(BITMAP) * 8 - 1)))
-#define FOUND_OPTS(i) (found_opts[(unsigned)i / BBITS])
-	BITMAP found_opts[256 / BBITS];
-
-	memset(found_opts, 0, sizeof(found_opts));
-
-	/* We need 7 elements for:
-	 * "interface=IFACE"
-	 * "ip=N.N.N.N" from packet->yiaddr
-	 * "giaddr=IP" from packet->gateway_nip (unless 0)
-	 * "siaddr=IP" from packet->siaddr_nip (unless 0)
-	 * "boot_file=FILE" from packet->file (unless overloaded)
-	 * "sname=SERVER_HOSTNAME" from packet->sname (unless overloaded)
-	 * terminating NULL
-	 */
-	envc = 7;
-	/* +1 element for each option, +2 for subnet option: */
-	if (packet) {
-		/* note: do not search for "pad" (0) and "end" (255) options */
-//TODO: change logic to scan packet _once_
-		for (i = 1; i < 255; i++) {
-			temp = udhcp_get_option(packet, i);
-			if (temp) {
-				if (i == DHCP_OPTION_OVERLOAD)
-					overload |= *temp;
-				else if (i == DHCP_SUBNET)
-					envc++; /* for $mask */
-				envc++;
-				/*if (i != DHCP_MESSAGE_TYPE)*/
-				FOUND_OPTS(i) |= BMASK(i);
-			}
-		}
-	}
-	curr = envp = xzalloc(sizeof(envp[0]) * envc);
+	char *new_opt;
+	uint8_t *optptr;
+	const struct dhcp_optflag *dh;
+	struct dhcp_scan_state scan_state;
+	const char *opt_name = "";
+	llist_t *envp = NULL;
 
-	*curr = xasprintf("interface=%s", client_data.interface);
-	putenv(*curr++);
+	new_opt = xasprintf("interface=%s", client_data.interface);
+	putenvp(&envp, new_opt);
 
 	if (!packet)
 		return envp;
 
+	init_scan_state(packet, &scan_state);
+
+    /* Iterate over the packet options.
+     * Handle each option based on whether it's an unknown / known option.
+     * There may be (although unlikely) duplicate options. For now, only the last
+     * appearing option will be stored in the environment, and all duplicates
+     * are freed properly.
+     * Long options may be implemented in the future (see RFC 3396) if needed.
+     */
+	while ((optptr = udhcp_scan_options(packet, &scan_state)) != NULL) {
+		uint8_t code = optptr[OPT_CODE];
+		uint8_t len = optptr[OPT_LEN];
+		uint8_t *data = optptr + OPT_DATA;
+
+		if (is_unknown_opt(code, &dh, &opt_name)) {
+			unsigned ofs;
+
+			new_opt = xmalloc(sizeof("optNNN=") + 1 + len*2);
+			ofs = sprintf(new_opt, "opt%u=", code);
+			*bin2hex(new_opt + ofs, (char *)data, len) = '\0';
+			putenvp(&envp, new_opt);
+		} else {
+			new_opt = xmalloc_optname_optval(data, dh, opt_name);
+			putenvp(&envp, new_opt);
+
+			if (code == DHCP_SUBNET && len == 4) {
+				uint32_t subnet;
+				move_from_unaligned32(subnet, data);
+				new_opt = xasprintf("mask=%u", mton(subnet));
+				putenvp(&envp, new_opt);
+			}
+		}
+	}
+
 	/* Export BOOTP fields. Fields we don't (yet?) export:
 	 * uint8_t op;      // always BOOTREPLY
 	 * uint8_t htype;   // hardware address type. 1 = 10mb ethernet
@@ -452,77 +474,31 @@ static char **fill_envp(struct dhcp_packet *packet)
 	 * uint8_t chaddr[16]; // link-layer client hardware address (MAC)
 	 */
 	/* Most important one: yiaddr as $ip */
-	*curr = xmalloc(sizeof("ip=255.255.255.255"));
-	sprint_nip(*curr, "ip=", (uint8_t *) &packet->yiaddr);
-	putenv(*curr++);
+	new_opt = xmalloc(sizeof("ip=255.255.255.255"));
+	sprint_nip(new_opt, "ip=", (uint8_t *) &packet->yiaddr);
+	putenvp(&envp, new_opt);
+
 	if (packet->siaddr_nip) {
 		/* IP address of next server to use in bootstrap */
-		*curr = xmalloc(sizeof("siaddr=255.255.255.255"));
-		sprint_nip(*curr, "siaddr=", (uint8_t *) &packet->siaddr_nip);
-		putenv(*curr++);
+		new_opt = xmalloc(sizeof("siaddr=255.255.255.255"));
+		sprint_nip(new_opt, "siaddr=", (uint8_t *) &packet->siaddr_nip);
+		putenvp(&envp, new_opt);
 	}
 	if (packet->gateway_nip) {
 		/* IP address of DHCP relay agent */
-		*curr = xmalloc(sizeof("giaddr=255.255.255.255"));
-		sprint_nip(*curr, "giaddr=", (uint8_t *) &packet->gateway_nip);
-		putenv(*curr++);
+		new_opt = xmalloc(sizeof("giaddr=255.255.255.255"));
+		sprint_nip(new_opt, "giaddr=", (uint8_t *) &packet->gateway_nip);
+		putenvp(&envp, new_opt);
 	}
-	if (!(overload & FILE_FIELD) && packet->file[0]) {
+	if (!(scan_state.overload & FILE_FIELD) && packet->file[0]) {
 		/* watch out for invalid packets */
-		*curr = xasprintf("boot_file=%."DHCP_PKT_FILE_LEN_STR"s", packet->file);
-		putenv(*curr++);
+		new_opt = xasprintf("boot_file=%."DHCP_PKT_FILE_LEN_STR"s", packet->file);
+		putenvp(&envp, new_opt);
 	}
-	if (!(overload & SNAME_FIELD) && packet->sname[0]) {
+	if (!(scan_state.overload & SNAME_FIELD) && packet->sname[0]) {
 		/* watch out for invalid packets */
-		*curr = xasprintf("sname=%."DHCP_PKT_SNAME_LEN_STR"s", packet->sname);
-		putenv(*curr++);
-	}
-
-	/* Export known DHCP options */
-	opt_name = dhcp_option_strings;
-	i = 0;
-	while (*opt_name) {
-		uint8_t code = dhcp_optflags[i].code;
-		BITMAP *found_ptr = &FOUND_OPTS(code);
-		BITMAP found_mask = BMASK(code);
-		if (!(*found_ptr & found_mask))
-			goto next;
-		*found_ptr &= ~found_mask; /* leave only unknown options */
-		temp = udhcp_get_option(packet, code);
-		*curr = xmalloc_optname_optval(temp, &dhcp_optflags[i], opt_name);
-		putenv(*curr++);
-		if (code == DHCP_SUBNET && temp[-OPT_DATA + OPT_LEN] == 4) {
-			/* Subnet option: make things like "$ip/$mask" possible */
-			uint32_t subnet;
-			move_from_unaligned32(subnet, temp);
-			*curr = xasprintf("mask=%u", mton(subnet));
-			putenv(*curr++);
-		}
- next:
-		opt_name += strlen(opt_name) + 1;
-		i++;
-	}
-	/* Export unknown options */
-	for (i = 0; i < 256;) {
-		BITMAP bitmap = FOUND_OPTS(i);
-		if (!bitmap) {
-			i += BBITS;
-			continue;
-		}
-		if (bitmap & BMASK(i)) {
-			unsigned len, ofs;
-
-			temp = udhcp_get_option(packet, i);
-			/* udhcp_get_option returns ptr to data portion,
-			 * need to go back to get len
-			 */
-			len = temp[-OPT_DATA + OPT_LEN];
-			*curr = xmalloc(sizeof("optNNN=") + 1 + len*2);
-			ofs = sprintf(*curr, "opt%u=", i);
-			*bin2hex(*curr + ofs, (void*) temp, len) = '\0';
-			putenv(*curr++);
-		}
-		i++;
+		new_opt = xasprintf("sname=%."DHCP_PKT_SNAME_LEN_STR"s", packet->sname);
+		putenvp(&envp, new_opt);
 	}
 
 	return envp;
@@ -531,7 +507,7 @@ static char **fill_envp(struct dhcp_packet *packet)
 /* Call a script with a par file and env vars */
 static void udhcp_run_script(struct dhcp_packet *packet, const char *name)
 {
-	char **envp, **curr;
+	llist_t *envp;
 	char *argv[3];
 
 	envp = fill_envp(packet);
@@ -543,11 +519,8 @@ static void udhcp_run_script(struct dhcp_packet *packet, const char *name)
 	argv[2] = NULL;
 	spawn_and_wait(argv);
 
-	for (curr = envp; *curr; curr++) {
-		log2(" %s", *curr);
-		bb_unsetenv_and_free(*curr);
-	}
-	free(envp);
+    /* Free all allocated environment variables */
+	llist_free(envp, (void (*)(void *))bb_unsetenv_and_free);
 }
 
 
-- 
2.11.0



More information about the busybox mailing list