From 7895b91743702497efc179a4fe30808e49c67977 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 3 Jul 2009 16:59:59 +0200 Subject: udhcp: dname_dec may return NULL, account for that case Other random cleanips included... Signed-off-by: Denys Vlasenko --- networking/udhcp/clientpacket.c | 4 +-- networking/udhcp/domain_codec.c | 73 ++++++++++++++++++++++------------------- networking/udhcp/options.c | 2 +- networking/udhcp/options.h | 8 +++-- networking/udhcp/script.c | 37 +++++++++++---------- 5 files changed, 66 insertions(+), 58 deletions(-) (limited to 'networking') diff --git a/networking/udhcp/clientpacket.c b/networking/udhcp/clientpacket.c index 21c1a7bd5..84a6765ee 100644 --- a/networking/udhcp/clientpacket.c +++ b/networking/udhcp/clientpacket.c @@ -166,7 +166,7 @@ int FAST_FUNC send_select(uint32_t xid, uint32_t server, uint32_t requested) } -/* Unicasts or broadcasts a DHCP renew message */ +/* Unicast or broadcast a DHCP renew message */ int FAST_FUNC send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr) { struct dhcp_packet packet; @@ -186,7 +186,7 @@ int FAST_FUNC send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr) } -/* Unicasts a DHCP release message */ +/* Unicast a DHCP release message */ int FAST_FUNC send_release(uint32_t server, uint32_t ciaddr) { struct dhcp_packet packet; diff --git a/networking/udhcp/domain_codec.c b/networking/udhcp/domain_codec.c index 6f051c4b0..45354e727 100644 --- a/networking/udhcp/domain_codec.c +++ b/networking/udhcp/domain_codec.c @@ -25,16 +25,9 @@ */ char* FAST_FUNC dname_dec(const uint8_t *cstr, int clen, const char *pre) { - const uint8_t *c; - int crtpos, retpos, depth, plen = 0, len = 0; + char *ret = ret; /* for compiler */ char *dst = NULL; - if (!cstr) - return NULL; - - if (pre) - plen = strlen(pre); - /* We make two passes over the cstr string. First, we compute * how long the resulting string would be. Then we allocate a * new buffer of the required length, and fill it in with the @@ -42,59 +35,71 @@ char* FAST_FUNC dname_dec(const uint8_t *cstr, int clen, const char *pre) * having to deal with requiring callers to supply their own * buffer, then having to check if it's sufficiently large, etc. */ - - while (!dst) { - - if (len > 0) { /* second pass? allocate dst buffer and copy pre */ - dst = xmalloc(len + plen); - memcpy(dst, pre, plen); - } + while (1) { + /* note: "return NULL" below are leak-safe since + * dst isn't yet allocated */ + const uint8_t *c; + unsigned crtpos, retpos, depth, len; crtpos = retpos = depth = len = 0; - while (crtpos < clen) { c = cstr + crtpos; - if ((*c & NS_CMPRSFLGS) != 0) { /* pointer */ - if (crtpos + 2 > clen) /* no offset to jump to? abort */ + if (*c & NS_CMPRSFLGS) { + /* pointer */ + if (crtpos + 2 > clen) /* no offset to jump to? abort */ return NULL; - if (retpos == 0) /* toplevel? save return spot */ + if (retpos == 0) /* toplevel? save return spot */ retpos = crtpos + 2; depth++; - crtpos = ((*c & 0x3f) << 8) | (*(c + 1) & 0xff); /* jump */ - } else if (*c) { /* label */ - if (crtpos + *c + 1 > clen) /* label too long? abort */ + crtpos = ((c[0] & 0x3f) << 8) | (c[1] & 0xff); /* jump */ + } else if (*c) { + /* label */ + if (crtpos + *c + 1 > clen) /* label too long? abort */ return NULL; if (dst) - memcpy(dst + plen + len, c + 1, *c); + memcpy(dst + len, c + 1, *c); len += *c + 1; crtpos += *c + 1; if (dst) - *(dst + plen + len - 1) = '.'; - } else { /* null: end of current domain name */ - if (retpos == 0) { /* toplevel? keep going */ + dst[len - 1] = '.'; + } else { + /* null: end of current domain name */ + if (retpos == 0) { + /* toplevel? keep going */ crtpos++; - } else { /* return to toplevel saved spot */ + } else { + /* return to toplevel saved spot */ crtpos = retpos; retpos = depth = 0; } if (dst) - *(dst + plen + len - 1) = ' '; + dst[len - 1] = ' '; } - if (depth > NS_MAXDNSRCH || /* too many jumps? abort, it's a loop */ - len > NS_MAXDNAME * NS_MAXDNSRCH) /* result too long? abort */ + if (depth > NS_MAXDNSRCH /* too many jumps? abort, it's a loop */ + || len > NS_MAXDNAME * NS_MAXDNSRCH /* result too long? abort */ + ) { return NULL; + } } - if (!len) /* expanded string has 0 length? abort */ + if (!len) /* expanded string has 0 length? abort */ return NULL; - if (dst) - *(dst + plen + len - 1) = '\0'; + if (!dst) { /* first pass? */ + /* allocate dst buffer and copy pre */ + unsigned plen = strlen(pre); + ret = dst = xmalloc(plen + len); + memcpy(dst, pre, plen); + dst += plen; + } else { + dst[len - 1] = '\0'; + break; + } } - return dst; + return ret; } /* Convert a domain name (src) from human-readable "foo.blah.com" format into diff --git a/networking/udhcp/options.c b/networking/udhcp/options.c index 0f17feb2a..76d6e3737 100644 --- a/networking/udhcp/options.c +++ b/networking/udhcp/options.c @@ -115,7 +115,7 @@ const uint8_t dhcp_option_lengths[] ALIGN1 = { [OPTION_U16] = 2, [OPTION_S16] = 2, [OPTION_U32] = 4, - [OPTION_S32] = 4 + [OPTION_S32] = 4, }; diff --git a/networking/udhcp/options.h b/networking/udhcp/options.h index 8c80485be..9e624318f 100644 --- a/networking/udhcp/options.h +++ b/networking/udhcp/options.h @@ -19,11 +19,13 @@ enum { OPTION_U16, OPTION_S16, OPTION_U32, - OPTION_S32 + OPTION_S32, }; -#define OPTION_REQ 0x10 /* have the client request this option */ -#define OPTION_LIST 0x20 /* There can be a list of 1 or more of these */ +/* Client requests this option by default */ +#define OPTION_REQ 0x10 +/* There can be a list of 1 or more of these */ +#define OPTION_LIST 0x20 /*****************************************************************/ /* Do not modify below here unless you know what you are doing!! */ diff --git a/networking/udhcp/script.c b/networking/udhcp/script.c index 7ebef3553..94dabf4d1 100644 --- a/networking/udhcp/script.c +++ b/networking/udhcp/script.c @@ -14,7 +14,7 @@ /* get a rough idea of how long an option will be (rounding up...) */ -static const uint8_t max_option_length[] = { +static const uint8_t len_of_option_as_string[] = { [OPTION_IP] = sizeof("255.255.255.255 "), [OPTION_IP_PAIR] = sizeof("255.255.255.255 ") * 2, [OPTION_STRING] = 1, @@ -30,17 +30,10 @@ static const uint8_t max_option_length[] = { }; -static inline int upper_length(int length, int opt_index) -{ - return max_option_length[opt_index] * - (length / dhcp_option_lengths[opt_index]); -} - - /* note: ip is a pointer to an IP in network order, possibly misaliged */ static int sprint_nip(char *dest, const char *pre, const uint8_t *ip) { - return sprintf(dest, "%s%d.%d.%d.%d", pre, ip[0], ip[1], ip[2], ip[3]); + return sprintf(dest, "%s%u.%u.%u.%u", pre, ip[0], ip[1], ip[2], ip[3]); } @@ -57,9 +50,10 @@ static int mton(uint32_t mask) } -/* Allocate and fill with the text of option 'option'. */ -static char *alloc_fill_opts(uint8_t *option, const struct dhcp_option *type_p, const char *opt_name) +/* Create "opt_name=opt_value" string */ +static char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_option *type_p, const char *opt_name) { + unsigned upper_length; int len, type, optlen; uint16_t val_u16; int16_t val_s16; @@ -67,14 +61,16 @@ static char *alloc_fill_opts(uint8_t *option, const struct dhcp_option *type_p, int32_t val_s32; char *dest, *ret; - len = option[OPT_LEN - 2]; + /* option points to OPT_DATA, need to go back and get OPT_LEN */ + len = option[OPT_LEN - OPT_DATA]; type = type_p->flags & TYPE_MASK; optlen = dhcp_option_lengths[type]; + upper_length = len_of_option_as_string[type] * (len / optlen); - dest = ret = xmalloc(upper_length(len, type) + strlen(opt_name) + 2); + dest = ret = xmalloc(upper_length + strlen(opt_name) + 2); dest += sprintf(ret, "%s=", opt_name); - for (;;) { + while (len >= optlen) { switch (type) { case OPTION_IP_PAIR: dest += sprint_nip(dest, "", option); @@ -114,15 +110,20 @@ static char *alloc_fill_opts(uint8_t *option, const struct dhcp_option *type_p, case OPTION_STR1035: /* unpack option into dest; use ret for prefix (i.e., "optname=") */ dest = dname_dec(option, len, ret); - free(ret); - return dest; + if (dest) { + free(ret); + return dest; + } + /* error. return "optname=" string */ + return ret; #endif } option += optlen; len -= optlen; if (len <= 0) break; - dest += sprintf(dest, " "); + *dest++ = ' '; + *dest = '\0'; } return ret; } @@ -174,7 +175,7 @@ static char **fill_envp(struct dhcp_packet *packet) temp = get_option(packet, dhcp_options[i].code); if (!temp) goto next; - *curr = alloc_fill_opts(temp, &dhcp_options[i], opt_name); + *curr = xmalloc_optname_optval(temp, &dhcp_options[i], opt_name); putenv(*curr++); /* Fill in a subnet bits option for things like /24 */ -- cgit v1.2.3