From 28efac0d37ceb5093a804da6a00c620034c5484f Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>

Date: Wed, 3 Sep 2014 10:28:24 -0400

Subject: [PATCH] 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 4d22568787..c9f7105bb3 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;

 }