From 28efac0d37ceb5093a804da6a00c620034c5484f Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 3 Sep 2014 10:28:24 -0400 Subject: [PATCH 1/1] localed: double free in error path and modernization Very unlikely to trigger, but in principle strv_free could be called twice: once explictly, and once from cleanup. --- src/locale/localed.c | 66 ++++++++++++++------------------------------ 1 file changed, 21 insertions(+), 45 deletions(-) diff --git a/src/locale/localed.c b/src/locale/localed.c index 4d2256878..c9f7105bb 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -208,7 +208,7 @@ static int vconsole_read_data(Context *c) { } static int x11_read_data(Context *c) { - FILE *f; + _cleanup_fclose_ FILE *f; char line[LINE_MAX]; bool in_section = false; int r; @@ -229,13 +229,11 @@ static int x11_read_data(Context *c) { continue; if (in_section && first_word(l, "Option")) { - char **a; + _cleanup_strv_free_ char **a = NULL; r = strv_split_quoted(&a, l); - if (r < 0) { - fclose(f); + if (r < 0) return r; - } if (strv_length(a) == 3) { if (streq(a[1], "XkbLayout")) { @@ -253,27 +251,20 @@ static int x11_read_data(Context *c) { } } - strv_free(a); - } else if (!in_section && first_word(l, "Section")) { - char **a; + _cleanup_strv_free_ char **a = NULL; r = strv_split_quoted(&a, l); - if (r < 0) { - fclose(f); + if (r < 0) return -ENOMEM; - } if (strv_length(a) == 2 && streq(a[1], "InputClass")) in_section = true; - strv_free(a); } else if (in_section && first_word(l, "EndSection")) in_section = false; } - fclose(f); - return 0; } @@ -289,14 +280,15 @@ static int context_read_data(Context *c) { static int locale_write_data(Context *c) { int r, p; - char **l = NULL; + _cleanup_strv_free_ char **l = NULL; r = load_env_file(NULL, "/etc/locale.conf", NULL, &l); if (r < 0 && r != -ENOENT) return r; for (p = 0; p < _LOCALE_MAX; p++) { - char *t, **u; + _cleanup_free_ char *t = NULL; + char **u; assert(names[p]); @@ -305,34 +297,25 @@ static int locale_write_data(Context *c) { continue; } - if (asprintf(&t, "%s=%s", names[p], c->locale[p]) < 0) { - strv_free(l); + if (asprintf(&t, "%s=%s", names[p], c->locale[p]) < 0) return -ENOMEM; - } u = strv_env_set(l, t); - free(t); - strv_free(l); - if (!u) return -ENOMEM; + strv_free(l); l = u; } if (strv_isempty(l)) { - strv_free(l); - if (unlink("/etc/locale.conf") < 0) return errno == ENOENT ? 0 : -errno; return 0; } - r = write_env_file_label("/etc/locale.conf", l); - strv_free(l); - - return r; + return write_env_file_label("/etc/locale.conf", l); } static int locale_update_system_manager(Context *c, sd_bus *bus) { @@ -403,38 +386,36 @@ static int vconsole_write_data(Context *c) { if (isempty(c->vc_keymap)) l = strv_env_unset(l, "KEYMAP"); else { - char *s, **u; + _cleanup_free_ char *s = NULL; + char **u; s = strappend("KEYMAP=", c->vc_keymap); if (!s) return -ENOMEM; u = strv_env_set(l, s); - free(s); - strv_free(l); - if (!u) return -ENOMEM; + strv_free(l); l = u; } if (isempty(c->vc_keymap_toggle)) l = strv_env_unset(l, "KEYMAP_TOGGLE"); else { - char *s, **u; + _cleanup_free_ char *s = NULL; + char **u; s = strappend("KEYMAP_TOGGLE=", c->vc_keymap_toggle); if (!s) return -ENOMEM; u = strv_env_set(l, s); - free(s); - strv_free(l); - if (!u) return -ENOMEM; + strv_free(l); l = u; } @@ -445,8 +426,7 @@ static int vconsole_write_data(Context *c) { return 0; } - r = write_env_file_label("/etc/vconsole.conf", l); - return r; + return write_env_file_label("/etc/vconsole.conf", l); } static int write_data_x11(Context *c) { @@ -868,13 +848,12 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata, sd_ } /* Check whether a variable is unset */ - if (!modified) { + if (!modified) for (p = 0; p < _LOCALE_MAX; p++) if (!isempty(c->locale[p]) && !passed[p]) { modified = true; break; } - } if (modified) { r = bus_verify_polkit_async(m, CAP_SYS_ADMIN, "org.freedesktop.locale1.set-locale", interactive, &c->polkit_registry, error); @@ -883,7 +862,7 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata, sd_ if (r == 0) return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ - STRV_FOREACH(i, l) { + STRV_FOREACH(i, l) for (p = 0; p < _LOCALE_MAX; p++) { size_t k; @@ -900,7 +879,6 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata, sd_ break; } } - } for (p = 0; p < _LOCALE_MAX; p++) { if (passed[p]) @@ -1112,7 +1090,7 @@ static int connect_bus(Context *c, sd_event *event, sd_bus **_bus) { } int main(int argc, char *argv[]) { - Context context = {}; + _cleanup_(context_free) Context context = {}; _cleanup_event_unref_ sd_event *event = NULL; _cleanup_bus_close_unref_ sd_bus *bus = NULL; int r; @@ -1155,7 +1133,5 @@ int main(int argc, char *argv[]) { } finish: - context_free(&context); - return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; } -- 2.30.2