Duplicate options are currently overridden (only the last option is kept). This leads to unexpected behavior when using long options.
The patch adds support for long options in compliance with RFC 3396. Fixes #13136. function old new delta udhcp_run_script 704 765 +61 optitem_unset_env_and_free - 31 +31 static.xmalloc_optname_optval 837 833 -4 putenvp 60 52 -8 ------------------------------------------------------------------------------ (add/remove: 1/0 grow/shrink: 1/2 up/down: 92/-12) Total: 80 bytes text data bss dec hex filename 994091 16939 1872 1012902 f74a6 busybox_old 994171 16939 1872 1012982 f74f6 busybox_unstripped Signed-off-by: Martin Lewis <[email protected]> --- networking/udhcp/dhcpc.c | 125 ++++++++++++++++++++++++++++++++++------------- networking/udhcp/dhcpc.h | 1 + 2 files changed, 93 insertions(+), 33 deletions(-) diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c index 50dfead63..d4a7a825e 100644 --- a/networking/udhcp/dhcpc.c +++ b/networking/udhcp/dhcpc.c @@ -115,6 +115,13 @@ enum { /*** Script execution code ***/ +struct dhcp_optitem +{ + unsigned len; + uint8_t *data; + char *env; + uint8_t code; +}; /* get a rough idea of how long an option will be (rounding up...) */ static const uint8_t len_of_option_as_string[] ALIGN1 = { @@ -186,15 +193,15 @@ static int good_hostname(const char *name) #endif /* Create "opt_name=opt_value" string */ -static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_optflag *optflag, const char *opt_name) +static NOINLINE char *xmalloc_optname_optval(const struct dhcp_optitem *opt_item, const struct dhcp_optflag *optflag, const char *opt_name) { unsigned upper_length; int len, type, optlen; char *dest, *ret; + uint8_t *option; - /* option points to OPT_DATA, need to go back to get OPT_LEN */ - len = option[-OPT_DATA + OPT_LEN]; - + option = opt_item->data; + len = opt_item->len; type = optflag->flags & OPTION_TYPE_MASK; optlen = dhcp_option_lengths[type]; upper_length = len_of_option_as_string[type] @@ -386,11 +393,50 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_ return ret; } -static void putenvp(llist_t **envp, char *new_opt) +/* Used by static options (interface, siaddr, etc) */ +static void putenvp(char *new_opt) { + struct dhcp_optitem *opt_item; + opt_item = xzalloc(sizeof(struct dhcp_optitem)); + /* Set opt_item->code = 0, so it won't appear in concat_option's lookup. */ + opt_item->env = new_opt; putenv(new_opt); - log2(" %s", new_opt); - llist_add_to(envp, new_opt); + llist_add_to(&client_data.envp, opt_item); +} + +/* Support RFC3396 Long Encoded Options */ +static struct dhcp_optitem *concat_option(uint8_t *data, uint8_t len, uint8_t code) +{ + llist_t *item; + struct dhcp_optitem *opt_item; + unsigned new_len = len; + + /* Check if an option with the code already exists. + * A possible optimization is to create a bitmap of all existing options in the packet, + * and iterate over the option list only if they exist. + * This will result in bigger code, and because dhcp packets don't have too many options it + * shouldn't have a big impact on performance. + */ + for (item = client_data.envp; item != NULL; item = item->link) { + opt_item = (struct dhcp_optitem *)item->data; + if (opt_item->code == code) + break; + } + + if (item) { + /* This is a duplicate, concatenate data according to RFC 3396 */ + new_len += opt_item->len; + } else { + /* This is a new option, add a new dhcp_optitem to the list */ + opt_item = xzalloc(sizeof(struct dhcp_optitem)); + opt_item->code = code; + llist_add_to(&client_data.envp, opt_item); + } + + opt_item->data = xrealloc(opt_item->data, new_len); /* xrealloc on the first occurrence (NULL) will call malloc */ + memcpy(opt_item->data + opt_item->len, data, len); + opt_item->len = new_len; + return opt_item; } static const char* get_optname(uint8_t code, const struct dhcp_optflag **dh) @@ -412,50 +458,58 @@ static const char* get_optname(uint8_t code, const struct dhcp_optflag **dh) } /* put all the parameters into the environment */ -static llist_t *fill_envp(struct dhcp_packet *packet) +static void fill_envp(struct dhcp_packet *packet) { uint8_t *optptr; struct dhcp_scan_state scan_state; char *new_opt; - llist_t *envp = NULL; - putenvp(&envp, xasprintf("interface=%s", client_data.interface)); + putenvp(xasprintf("interface=%s", client_data.interface)); if (!packet) - return envp; + return; 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. + * Long options are supported in compliance with RFC 3396. */ while ((optptr = udhcp_scan_options(packet, &scan_state)) != NULL) { const struct dhcp_optflag *dh; const char *opt_name; + struct dhcp_optitem *opt_item; uint8_t code = optptr[OPT_CODE]; uint8_t len = optptr[OPT_LEN]; uint8_t *data = optptr + OPT_DATA; + opt_item = concat_option(data, len, code); opt_name = get_optname(code, &dh); if (opt_name) { - new_opt = xmalloc_optname_optval(data, dh, opt_name); - if (code == DHCP_SUBNET && len == 4) { + new_opt = xmalloc_optname_optval(opt_item, dh, opt_name); + if (opt_item->code == DHCP_SUBNET && opt_item->len == 4) { + /* DHCP_SUBNET option is special, because it requires + * two environment variables. Since DHCP servers don't + * send this option more than once, we're not handling + * it as a duplicate. If by any chance it's sent twice, + * everything is still freed as expected. + */ uint32_t subnet; - putenvp(&envp, new_opt); - move_from_unaligned32(subnet, data); - new_opt = xasprintf("mask=%u", mton(subnet)); + move_from_unaligned32(subnet, opt_item->data); + putenvp(xasprintf("mask=%u", mton(subnet))); } } else { 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] = '\0'; + new_opt = xmalloc(sizeof("optNNN=") + 1 + opt_item->len*2); + ofs = sprintf(new_opt, "opt%u=", opt_item->code); + bin2hex(new_opt + ofs, (char *)opt_item->data, opt_item->len)[0] = '\0'; } - putenvp(&envp, new_opt); + putenv(new_opt); + /* putenv will replace the existing environment variable in case of a duplicate. + * Free the previous occurrence (NULL if it's the first one). + */ + free(opt_item->env); + opt_item->env = new_opt; } /* Export BOOTP fields. Fields we don't (yet?) export: @@ -473,41 +527,45 @@ static llist_t *fill_envp(struct dhcp_packet *packet) /* Most important one: yiaddr as $ip */ new_opt = xmalloc(sizeof("ip=255.255.255.255")); sprint_nip(new_opt, "ip=", (uint8_t *) &packet->yiaddr); - putenvp(&envp, new_opt); + putenvp(new_opt); if (packet->siaddr_nip) { /* IP address of next server to use in bootstrap */ new_opt = xmalloc(sizeof("siaddr=255.255.255.255")); sprint_nip(new_opt, "siaddr=", (uint8_t *) &packet->siaddr_nip); - putenvp(&envp, new_opt); + putenvp(new_opt); } if (packet->gateway_nip) { /* IP address of DHCP relay agent */ new_opt = xmalloc(sizeof("giaddr=255.255.255.255")); sprint_nip(new_opt, "giaddr=", (uint8_t *) &packet->gateway_nip); - putenvp(&envp, new_opt); + putenvp(new_opt); } if (!(scan_state.overload & FILE_FIELD) && packet->file[0]) { /* watch out for invalid packets */ new_opt = xasprintf("boot_file=%."DHCP_PKT_FILE_LEN_STR"s", packet->file); - putenvp(&envp, new_opt); + putenvp(new_opt); } if (!(scan_state.overload & SNAME_FIELD) && packet->sname[0]) { /* watch out for invalid packets */ new_opt = xasprintf("sname=%."DHCP_PKT_SNAME_LEN_STR"s", packet->sname); - putenvp(&envp, new_opt); + putenvp(new_opt); } +} - return envp; +static void optitem_unset_env_and_free(struct dhcp_optitem *opt_item) +{ + bb_unsetenv_and_free(opt_item->env); + free(opt_item->data); + free(opt_item); } /* Call a script with a par file and env vars */ static void udhcp_run_script(struct dhcp_packet *packet, const char *name) { - llist_t *envp; char *argv[3]; - envp = fill_envp(packet); + fill_envp(packet); /* call script */ log1("executing %s %s", client_data.script, name); @@ -517,7 +575,8 @@ static void udhcp_run_script(struct dhcp_packet *packet, const char *name) spawn_and_wait(argv); /* Free all allocated environment variables */ - llist_free(envp, (void (*)(void *))bb_unsetenv_and_free); + llist_free(client_data.envp, (void (*)(void *))optitem_unset_env_and_free); + client_data.envp = NULL; } diff --git a/networking/udhcp/dhcpc.h b/networking/udhcp/dhcpc.h index b407a6cdb..7ad01ea8f 100644 --- a/networking/udhcp/dhcpc.h +++ b/networking/udhcp/dhcpc.h @@ -21,6 +21,7 @@ struct client_data_t { uint8_t *vendorclass; /* Optional vendor class-id to use */ uint8_t *hostname; /* Optional hostname to use */ uint8_t *fqdn; /* Optional fully qualified domain name to use */ + llist_t *envp; /* list of DHCP options used for env vars */ unsigned first_secs; unsigned last_secs; -- 2.11.0 _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
