dns: clean up error paths in dns-manager

Specifically for resolvconf, if the write succeeded, but the pclose()
failed error would be left NULL and SR_ERROR would be returned, which
caused a crash in nm_dns_manager_end_updates().
This commit is contained in:
Dan Williams 2016-01-20 13:52:59 -06:00
parent acc0d79224
commit af189b3295

View file

@ -367,7 +367,6 @@ dispatch_netconfig (NMDnsManager *self,
if (searches) {
str = g_strjoinv (" ", searches);
write_to_netconfig (self, fd, "DNSSEARCH", str);
g_free (str);
}
@ -415,10 +414,9 @@ write_resolv_conf (FILE *f,
char **options,
GError **error)
{
char *searches_str = NULL;
char *nameservers_str = NULL;
char *options_str = NULL;
gboolean retval = FALSE;
gs_free char *searches_str = NULL;
gs_free char *nameservers_str = NULL;
gs_free char *options_str = NULL;
char *tmp_str;
GString *str;
int i;
@ -435,11 +433,10 @@ write_resolv_conf (FILE *f,
g_free (tmp_str);
}
str = g_string_new ("");
if (nameservers) {
int num = g_strv_length (nameservers);
str = g_string_new ("");
for (i = 0; i < num; i++) {
if (i == 3) {
g_string_append (str, "# ");
@ -453,28 +450,22 @@ write_resolv_conf (FILE *f,
g_string_append (str, nameservers[i]);
g_string_append_c (str, '\n');
}
nameservers_str = g_string_free (str, FALSE);
}
nameservers_str = g_string_free (str, FALSE);
if (fprintf (f, "# Generated by NetworkManager\n%s%s%s",
searches_str ? searches_str : "",
nameservers_str,
options_str ? options_str : "") > 0)
retval = TRUE;
else {
nameservers_str ? nameservers_str : "",
options_str ? options_str : "") < 0) {
g_set_error (error,
NM_MANAGER_ERROR,
NM_MANAGER_ERROR_FAILED,
"Could not write " _PATH_RESCONF ": %s\n",
g_strerror (errno));
return FALSE;
}
g_free (searches_str);
g_free (nameservers_str);
g_free (options_str);
return retval;
return TRUE;
}
static SpawnResult
@ -484,9 +475,9 @@ dispatch_resolvconf (NMDnsManager *self,
char **options,
GError **error)
{
char *cmd;
gs_free char *cmd = NULL;
FILE *f;
gboolean retval = FALSE;
gboolean success = FALSE;
int errnosv, err;
if (!g_file_test (RESOLVCONF_PATH, G_FILE_TEST_IS_EXECUTABLE)) {
@ -497,39 +488,46 @@ dispatch_resolvconf (NMDnsManager *self,
return SR_NOTFOUND;
}
if (searches || nameservers) {
cmd = g_strconcat (RESOLVCONF_PATH, " -a ", "NetworkManager", NULL);
_LOGI ("Writing DNS information to %s", RESOLVCONF_PATH);
if ((f = popen (cmd, "w")) == NULL)
g_set_error (error,
NM_MANAGER_ERROR,
NM_MANAGER_ERROR_FAILED,
"Could not write to %s: %s\n",
RESOLVCONF_PATH,
g_strerror (errno));
else {
retval = write_resolv_conf (f, searches, nameservers, options, error);
err = pclose (f);
if (err < 0) {
errnosv = errno;
g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errnosv),
"Failed to close pipe to resolvconf: %d", errnosv);
retval = FALSE;
} else if (err > 0) {
_LOGW ("resolvconf failed with status %d", err);
retval = FALSE;
}
}
} else {
cmd = g_strconcat (RESOLVCONF_PATH, " -d ", "NetworkManager", NULL);
if (!searches && !nameservers) {
_LOGI ("Removing DNS information from %s", RESOLVCONF_PATH);
if (nm_spawn_process (cmd, error) == 0)
retval = TRUE;
cmd = g_strconcat (RESOLVCONF_PATH, " -d ", "NetworkManager", NULL);
if (nm_spawn_process (cmd, error) != 0)
return SR_ERROR;
return SR_SUCCESS;
}
g_free (cmd);
_LOGI ("Writing DNS information to %s", RESOLVCONF_PATH);
return retval ? SR_SUCCESS : SR_ERROR;
cmd = g_strconcat (RESOLVCONF_PATH, " -a ", "NetworkManager", NULL);
if ((f = popen (cmd, "w")) == NULL) {
g_set_error (error,
NM_MANAGER_ERROR,
NM_MANAGER_ERROR_FAILED,
"Could not write to %s: %s\n",
RESOLVCONF_PATH,
g_strerror (errno));
return SR_ERROR;
}
success = write_resolv_conf (f, searches, nameservers, options, error);
err = pclose (f);
if (err < 0) {
errnosv = errno;
g_clear_error (error);
g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errnosv),
"Failed to close pipe to resolvconf: %d", errnosv);
return SR_ERROR;
} else if (err > 0) {
_LOGW ("resolvconf failed with status %d", err);
g_clear_error (error);
g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
"resolvconf failed with status %d", err);
return SR_ERROR;
}
return success ? SR_SUCCESS : SR_ERROR;
}
#define MY_RESOLV_CONF NMRUNDIR "/resolv.conf"
@ -546,7 +544,7 @@ update_resolv_conf (NMDnsManager *self,
{
FILE *f;
struct stat st;
gboolean ret;
gboolean success;
/* If we are not managing /etc/resolv.conf and it points to
* MY_RESOLV_CONF, don't write the private DNS configuration to
@ -554,15 +552,12 @@ update_resolv_conf (NMDnsManager *self,
* some external application.
*/
if (!install_etc) {
char *path = g_file_read_link (_PATH_RESCONF, NULL);
gboolean ours = !g_strcmp0 (path, MY_RESOLV_CONF);
gs_free char *path = g_file_read_link (_PATH_RESCONF, NULL);
g_free (path);
if (ours) {
if (g_strcmp0 (path, MY_RESOLV_CONF) == 0) {
_LOGD ("not updating " MY_RESOLV_CONF
" since it points to " _PATH_RESCONF);
return SR_ERROR;
return SR_SUCCESS;
}
}
@ -576,10 +571,10 @@ update_resolv_conf (NMDnsManager *self,
return SR_ERROR;
}
ret = write_resolv_conf (f, searches, nameservers, options, error);
success = write_resolv_conf (f, searches, nameservers, options, error);
if (fclose (f) < 0) {
if (ret) {
if (success) {
/* only set an error here if write_resolv_conf() was successful,
* since its error is more important.
*/
@ -590,9 +585,8 @@ update_resolv_conf (NMDnsManager *self,
MY_RESOLV_CONF_TMP,
g_strerror (errno));
}
}
if (!ret)
return SR_ERROR;
} else if (!success)
return SR_ERROR;
if (rename (MY_RESOLV_CONF_TMP, MY_RESOLV_CONF) < 0) {
@ -608,30 +602,32 @@ update_resolv_conf (NMDnsManager *self,
if (!install_etc)
return SR_SUCCESS;
/* Don't overwrite a symbolic link unless it points to MY_RESOLV_CONF. */
/* A symlink pointing to NM's own resolv.conf (MY_RESOLV_CONF) is always
* overwritten to ensure that changes are indicated with inotify. Symlinks
* pointing to any other file are never overwritten.
*/
if (lstat (_PATH_RESCONF, &st) != -1) {
/* Don't overwrite a symbolic link. */
if (S_ISLNK (st.st_mode)) {
if (stat (_PATH_RESCONF, &st) != -1) {
char *path = g_file_read_link (_PATH_RESCONF, NULL);
gboolean not_ours = g_strcmp0 (path, MY_RESOLV_CONF) != 0;
gs_free char *path = g_file_read_link (_PATH_RESCONF, NULL);
g_free (path);
if (not_ours)
if (g_strcmp0 (path, MY_RESOLV_CONF) != 0) {
/* It's not NM's symlink; do nothing */
return SR_SUCCESS;
}
/* resolv.conf is a symlink owned by NM and the target is accessible
*/
} else {
if (errno != ENOENT)
return SR_SUCCESS;
g_set_error (error,
NM_MANAGER_ERROR,
NM_MANAGER_ERROR_FAILED,
"Could not stat %s: %s\n",
_PATH_RESCONF,
g_strerror (errno));
return SR_ERROR;
/* resolv.conf is a symlink but the target is not accessible;
* some other program is probably managing resolv.conf and
* NM should not touch it.
*/
return SR_SUCCESS;
}
}
} else if (errno != ENOENT) {
/* NM cannot read /etc/resolv.conf */
g_set_error (error,
NM_MANAGER_ERROR,
NM_MANAGER_ERROR_FAILED,
@ -641,6 +637,10 @@ update_resolv_conf (NMDnsManager *self,
return SR_ERROR;
}
/* By this point, either /etc/resolv.conf does not exist, is a regular
* file, or is a symlink already owned by NM. In all cases /etc/resolv.conf
* is replaced with a symlink pointing to NM's resolv.conf in /var/run/.
*/
if (unlink (RESOLV_CONF_TMP) == -1 && errno != ENOENT) {
g_set_error (error,
NM_MANAGER_ERROR,