dhclient(8): check strdup() return values in bind_lease()
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 - 1.202
+++ dhclient.c 12 Jan 2013 20:02:13 -
@@ -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,
Re: [miniroot/install.sub] skip x* sets if do not expect to run X.
No, I normally install all the X sets, I just do not run X on the console.
So I don't like this.
On Wed, Jan 9, 2013 at 3:43 PM, Gleydson Soares gsoa...@trusted.com.brwrote:
the diff below changes src/distrib/miniroot/install.sub to by default skip
x* sets if someone do not expect to run X
Do you expect to run the X Window System [no]
if someone still want to install those sets may select by hand afterwards:
Set name(s)? (or 'abort' or 'done') [done] x*
i've compile a RAMDISK_CD and seems to work fine.
ok? feedback?
Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.674
diff -u -p -r1.674 install.sub
--- install.sub 2 Jan 2013 20:35:00 - 1.674
+++ install.sub 11 Jan 2013 23:41:45 -
@@ -1098,8 +1098,9 @@ install_files() {
for _f in $THESETS; do
isin $_f $_files || continue;
_sets=$(addel $_f $_sets)
- if [[ -z $DISPLAY ! -d /mnt/etc/X11 ]]; then
- # No displays and X isn't installed == skip X sets
+ if [[ -z $DISPLAY ! -d /mnt/etc/X11 || $x11 == n ]];
then
+ # No displays and X isn't installed or do not
expect to run X
+ # = skip X sets
isin ${_f%${VERSION}.tgz} xbase xetc xshare xfont
xserv continue
fi
isin $_f $DEFAULTSETS site$VERSION-$(hostname -s).tgz
\
Re: [miniroot/install.sub] skip x* sets if do not expect to run X.
There are cases where you want to compile some port not directly related to X
but the dependency is missing if you didn't load the X sets. I don't remember
the particular, but I know this has happened to me.
devin
On Jan 12, 2013, at 9:33 PM, Bob Beck wrote:
No, I normally install all the X sets, I just do not run X on the console.
So I don't like this.
On Wed, Jan 9, 2013 at 3:43 PM, Gleydson Soares gsoa...@trusted.com.brwrote:
the diff below changes src/distrib/miniroot/install.sub to by default skip
x* sets if someone do not expect to run X
Do you expect to run the X Window System [no]
if someone still want to install those sets may select by hand afterwards:
Set name(s)? (or 'abort' or 'done') [done] x*
i've compile a RAMDISK_CD and seems to work fine.
ok? feedback?
Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.674
diff -u -p -r1.674 install.sub
--- install.sub 2 Jan 2013 20:35:00 - 1.674
+++ install.sub 11 Jan 2013 23:41:45 -
@@ -1098,8 +1098,9 @@ install_files() {
for _f in $THESETS; do
isin $_f $_files || continue;
_sets=$(addel $_f $_sets)
- if [[ -z $DISPLAY ! -d /mnt/etc/X11 ]]; then
- # No displays and X isn't installed == skip X sets
+ if [[ -z $DISPLAY ! -d /mnt/etc/X11 || $x11 == n ]];
then
+ # No displays and X isn't installed or do not
expect to run X
+ # = skip X sets
isin ${_f%${VERSION}.tgz} xbase xetc xshare xfont
xserv continue
fi
isin $_f $DEFAULTSETS site$VERSION-$(hostname -s).tgz
\
