The bind_lease() function has several strdup() calls for the domainname
and nameservers variables, but their return values are not checked.
In my tests, dhclient won't crash even if these strdup() calls return
NULL; however, if one of those variables become NULL as a result, the
"search" or "nameserver" line will be missing in /etc/resolv.conf. If
both variables are NULL, /etc/resolv.conf won't be updated at all. In
either case, the user won't know why this happened.
This diff makes the following changes to address this:
- In new_resolv_conf(), confirm that nameservers is not NULL and
actually has a value before attempting to use it. While not strictly
necessary, this makes the code consistent with the domainname check
right above it.
- In bind_lease(), error out if the strdup() calls fail. Also
initialize domainname and nameservers to NULL at the beginning of
the function; since new_resolv_conf() ensures that these variables
are not NULL before using them, initializing them to NULL helps us
avoid the need to do strdup("").
I have tested this diff for almost a week and also with (U)pgrade.
Comments? OK?
Lawrence
Index: dhclient.c
===================================================================
RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
retrieving revision 1.202
diff -u -p -r1.202 dhclient.c
--- dhclient.c 6 Jan 2013 15:33:12 -0000 1.202
+++ dhclient.c 12 Jan 2013 20:02:13 -0000
@@ -701,7 +701,7 @@ bind_lease(void)
struct in_addr gateway, mask;
struct option_data *options;
struct client_lease *lease;
- char *domainname, *nameservers;
+ char *domainname = NULL, *nameservers = NULL;
delete_addresses(ifi->name, ifi->rdomain);
flush_routes_and_arp_cache(ifi->rdomain);
@@ -713,17 +713,19 @@ bind_lease(void)
memcpy(&mask.s_addr, options[DHO_SUBNET_MASK].data,
options[DHO_SUBNET_MASK].len);
- if (options[DHO_DOMAIN_NAME].len)
+ if (options[DHO_DOMAIN_NAME].len) {
domainname = strdup(pretty_print_option(
DHO_DOMAIN_NAME, &options[DHO_DOMAIN_NAME], 0));
- else
- domainname = strdup("");
+ if (domainname == NULL)
+ error("no memory for domainname");
+ }
if (options[DHO_DOMAIN_NAME_SERVERS].len) {
nameservers = strdup(pretty_print_option(
DHO_DOMAIN_NAME_SERVERS,
&options[DHO_DOMAIN_NAME_SERVERS], 0));
- } else
- nameservers = strdup("");
+ if (nameservers == NULL)
+ error("no memory for nameservers");
+ }
new_resolv_conf(ifi->name, domainname, nameservers);
@@ -1929,13 +1931,15 @@ new_resolv_conf(char *ifname, char *doma
strlcat(imsg.contents, "\n", MAXRESOLVCONFSIZE);
}
- for (p = strsep(&nameservers, " "); p != NULL;
- p = strsep(&nameservers, " ")) {
- if (*p == '\0')
- continue;
- strlcat(imsg.contents, "nameserver ", MAXRESOLVCONFSIZE);
- strlcat(imsg.contents, p, MAXRESOLVCONFSIZE);
- strlcat(imsg.contents, "\n", MAXRESOLVCONFSIZE);
+ if (nameservers && strlen(nameservers)) {
+ for (p = strsep(&nameservers, " "); p != NULL;
+ p = strsep(&nameservers, " ")) {
+ if (*p == '\0')
+ continue;
+ strlcat(imsg.contents, "nameserver ",
MAXRESOLVCONFSIZE);
+ strlcat(imsg.contents, p, MAXRESOLVCONFSIZE);
+ strlcat(imsg.contents, "\n", MAXRESOLVCONFSIZE);
+ }
}
rslt = imsg_compose(unpriv_ibuf, IMSG_NEW_RESOLV_CONF, 0, 0, -1, &imsg,
