[PATCH] 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().
---
src/dns-manager/nm-dns-manager.c | 126 ++-
1 file changed, 58 insertions(+), 68 deletions(-)
diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-dns-manager.c
index 01e8bf1..48f44ea 100644
--- a/src/dns-manager/nm-dns-manager.c
+++ b/src/dns-manager/nm-dns-manager.c
@@ -357,7 +357,6 @@ dispatch_netconfig (NMDnsManager *self,
if (searches) {
str = g_strjoinv (" ", searches);
-
write_to_netconfig (self, fd, "DNSSEARCH", str);
g_free (str);
}
@@ -405,10 +404,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;
@@ -425,12 +423,9 @@ write_resolv_conf (FILE *f,
g_free (tmp_str);
}
- str = g_string_new ("");
-
if (nameservers) {
- int num = g_strv_length (nameservers);
-
- for (i = 0; i < num; i++) {
+ str = g_string_new ("");
+ for (i = 0; i < g_strv_length (nameservers); i++) {
if (i == 3) {
g_string_append (str, "# ");
g_string_append (str, _("NOTE: the libc
resolver may not support more than 3 nameservers."));
@@ -443,28 +438,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
@@ -474,10 +463,11 @@ 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;
+ GError *local = NULL;
if (!g_file_test (RESOLVCONF_PATH, G_FILE_TEST_IS_EXECUTABLE)) {
g_set_error_literal (error,
@@ -487,39 +477,45 @@ 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) {
Re: [PATCH] dns: clean up error paths in dns-manager
On Wed, 2016-01-20 at 13:52 -0600, Dan Williams wrote:
> 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().
> ---
> src/dns-manager/nm-dns-manager.c | 126 ++---
> --
> 1 file changed, 58 insertions(+), 68 deletions(-)
>
> diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-
> dns-manager.c
> index 01e8bf1..48f44ea 100644
> --- a/src/dns-manager/nm-dns-manager.c
> +++ b/src/dns-manager/nm-dns-manager.c
> @@ -357,7 +357,6 @@ dispatch_netconfig (NMDnsManager *self,
>
> if (searches) {
> str = g_strjoinv (" ", searches);
> -
> write_to_netconfig (self, fd, "DNSSEARCH", str);
> g_free (str);
> }
> @@ -405,10 +404,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;
> @@ -425,12 +423,9 @@ write_resolv_conf (FILE *f,
> g_free (tmp_str);
> }
>
> - str = g_string_new ("");
> -
> if (nameservers) {
> - int num = g_strv_length (nameservers);
> -
> - for (i = 0; i < num; i++) {
> + str = g_string_new ("");
> + for (i = 0; i < g_strv_length (nameservers); i++) {
for (i = 0; nameservers[i]; i++)
otherwise, the length has to be calculated needlessly and for each
iteration.
> if (i == 3) {
> g_string_append (str, "# ");
> g_string_append (str, _("NOTE: the
> libc resolver may not support more than 3 nameservers."));
> @@ -443,28 +438,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
> @@ -474,10 +463,11 @@ 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;
> + GError *local = NULL;
>
> if (!g_file_test (RESOLVCONF_PATH,
> G_FILE_TEST_IS_EXECUTABLE)) {
> g_set_error_literal (error,
> @@ -487,39 +477,45 @@ 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
