Merge pull request #25718 from yuwata/locale-cleanups

locale: avoid TOCTOU in reading config files

4 files changed, 47 insertions, 41 deletions

diff --git a/src/locale/localectl.c b/src/locale/localectl.c
index 966f07d083..fb83881cc7 100644
--- a/src/locale/localectl.c
+++ b/src/locale/localectl.c
@@ -61,7 +61,7 @@ static int print_status_info(StatusInfo *i) {
         assert(i);
 
         if (arg_transport == BUS_TRANSPORT_LOCAL) {
-                _cleanup_(locale_context_clear) LocaleContext c = { .mtime = USEC_INFINITY };
+                _cleanup_(locale_context_clear) LocaleContext c = {};
 
                 r = locale_context_load(&c, LOCALE_LOAD_PROC_CMDLINE);
                 if (r < 0)
diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c
index 8fb65b9699..cace820bdd 100644
--- a/src/locale/localed-util.c
+++ b/src/locale/localed-util.c
@@ -20,6 +20,7 @@
 #include "mkdir-label.h"
 #include "nulstr-util.h"
 #include "process-util.h"
+#include "stat-util.h"
 #include "string-util.h"
 #include "strv.h"
 #include "tmpfile-util.h"
@@ -92,8 +93,8 @@ int locale_read_data(Context *c, sd_bus_message *m) {
 }
 
 int vconsole_read_data(Context *c, sd_bus_message *m) {
+        _cleanup_close_ int fd = -EBADFD;
         struct stat st;
-        usec_t t;
 
         /* Do not try to re-read the file within single bus operation. */
         if (m) {
@@ -104,33 +105,35 @@ int vconsole_read_data(Context *c, sd_bus_message *m) {
                 c->vc_cache = sd_bus_message_ref(m);
         }
 
-        if (stat("/etc/vconsole.conf", &st) < 0) {
-                if (errno != ENOENT)
-                        return -errno;
-
-                c->vc_mtime = USEC_INFINITY;
+        fd = RET_NERRNO(open("/etc/vconsole.conf", O_CLOEXEC | O_PATH));
+        if (fd == -ENOENT) {
+                c->vc_stat = (struct stat) {};
                 context_free_vconsole(c);
                 return 0;
         }
+        if (fd < 0)
+                return fd;
+
+        if (fstat(fd, &st) < 0)
+                return -errno;
 
-        /* If mtime is not changed, then we do not need to re-read */
-        t = timespec_load(&st.st_mtim);
-        if (c->vc_mtime != USEC_INFINITY && t == c->vc_mtime)
+        /* If the file is not changed, then we do not need to re-read */
+        if (stat_inode_unmodified(&c->vc_stat, &st))
                 return 0;
 
-        c->vc_mtime = t;
+        c->vc_stat = st;
         context_free_vconsole(c);
 
-        return parse_env_file(NULL, "/etc/vconsole.conf",
-                              "KEYMAP",        &c->vc_keymap,
-                              "KEYMAP_TOGGLE", &c->vc_keymap_toggle);
+        return parse_env_file_fd(fd, "/etc/vconsole.conf",
+                                 "KEYMAP",        &c->vc_keymap,
+                                 "KEYMAP_TOGGLE", &c->vc_keymap_toggle);
 }
 
 int x11_read_data(Context *c, sd_bus_message *m) {
+        _cleanup_close_ int fd = -EBADFD, fd_ro = -EBADFD;
         _cleanup_fclose_ FILE *f = NULL;
         bool in_section = false;
         struct stat st;
-        usec_t t;
         int r;
 
         /* Do not try to re-read the file within single bus operation. */
@@ -142,27 +145,35 @@ int x11_read_data(Context *c, sd_bus_message *m) {
                 c->x11_cache = sd_bus_message_ref(m);
         }
 
-        if (stat("/etc/X11/xorg.conf.d/00-keyboard.conf", &st) < 0) {
-                if (errno != ENOENT)
-                        return -errno;
-
-                c->x11_mtime = USEC_INFINITY;
+        fd = RET_NERRNO(open("/etc/X11/xorg.conf.d/00-keyboard.conf", O_CLOEXEC | O_PATH));
+        if (fd == -ENOENT) {
+                c->x11_stat = (struct stat) {};
                 context_free_x11(c);
                 return 0;
         }
+        if (fd < 0)
+                return fd;
 
-        /* If mtime is not changed, then we do not need to re-read */
-        t = timespec_load(&st.st_mtim);
-        if (c->x11_mtime != USEC_INFINITY && t == c->x11_mtime)
+        if (fstat(fd, &st) < 0)
+                return -errno;
+
+        /* If the file is not changed, then we do not need to re-read */
+        if (stat_inode_unmodified(&c->x11_stat, &st))
                 return 0;
 
-        c->x11_mtime = t;
+        c->x11_stat = st;
         context_free_x11(c);
 
-        f = fopen("/etc/X11/xorg.conf.d/00-keyboard.conf", "re");
+        fd_ro = fd_reopen(fd, O_CLOEXEC | O_RDONLY);
+        if (fd_ro < 0)
+                return fd_ro;
+
+        f = fdopen(fd_ro, "re");
         if (!f)
                 return -errno;
 
+        TAKE_FD(fd_ro);
+
         for (;;) {
                 _cleanup_free_ char *line = NULL;
                 char *l;
@@ -219,7 +230,6 @@ int x11_read_data(Context *c, sd_bus_message *m) {
 
 int vconsole_write_data(Context *c) {
         _cleanup_strv_free_ char **l = NULL;
-        struct stat st;
         int r;
 
         r = load_env_file(NULL, "/etc/vconsole.conf", &l);
@@ -238,7 +248,7 @@ int vconsole_write_data(Context *c) {
                 if (unlink("/etc/vconsole.conf") < 0)
                         return errno == ENOENT ? 0 : -errno;
 
-                c->vc_mtime = USEC_INFINITY;
+                c->vc_stat = (struct stat) {};
                 return 0;
         }
 
@@ -246,8 +256,8 @@ int vconsole_write_data(Context *c) {
         if (r < 0)
                 return r;
 
-        if (stat("/etc/vconsole.conf", &st) >= 0)
-                c->vc_mtime = timespec_load(&st.st_mtim);
+        if (stat("/etc/vconsole.conf", &c->vc_stat) < 0)
+                return -errno;
 
         return 0;
 }
@@ -255,7 +265,6 @@ int vconsole_write_data(Context *c) {
 int x11_write_data(Context *c) {
         _cleanup_fclose_ FILE *f = NULL;
         _cleanup_(unlink_and_freep) char *temp_path = NULL;
-        struct stat st;
         int r;
 
         if (isempty(c->x11_layout) &&
@@ -266,7 +275,7 @@ int x11_write_data(Context *c) {
                 if (unlink("/etc/X11/xorg.conf.d/00-keyboard.conf") < 0)
                         return errno == ENOENT ? 0 : -errno;
 
-                c->vc_mtime = USEC_INFINITY;
+                c->x11_stat = (struct stat) {};
                 return 0;
         }
 
@@ -305,8 +314,8 @@ int x11_write_data(Context *c) {
         if (rename(temp_path, "/etc/X11/xorg.conf.d/00-keyboard.conf") < 0)
                 return -errno;
 
-        if (stat("/etc/X11/xorg.conf.d/00-keyboard.conf", &st) >= 0)
-                c->x11_mtime = timespec_load(&st.st_mtim);
+        if (stat("/etc/X11/xorg.conf.d/00-keyboard.conf", &c->x11_stat) < 0)
+                return -errno;
 
         return 0;
 }
diff --git a/src/locale/localed-util.h b/src/locale/localed-util.h
index 5470d1bb9b..b09ed80b83 100644
--- a/src/locale/localed-util.h
+++ b/src/locale/localed-util.h
@@ -1,25 +1,26 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
+#include <sys/stat.h>
+
 #include "sd-bus.h"
 
 #include "hashmap.h"
 #include "locale-setup.h"
-#include "time-util.h"
 
 typedef struct Context {
         sd_bus_message *locale_cache;
         LocaleContext locale_context;
 
         sd_bus_message *x11_cache;
-        usec_t x11_mtime;
+        struct stat x11_stat;
         char *x11_layout;
         char *x11_model;
         char *x11_variant;
         char *x11_options;
 
         sd_bus_message *vc_cache;
-        usec_t vc_mtime;
+        struct stat vc_stat;
         char *vc_keymap;
         char *vc_keymap_toggle;
 
diff --git a/src/locale/localed.c b/src/locale/localed.c
index 9b6890ebb4..8394751536 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -760,11 +760,7 @@ static int connect_bus(Context *c, sd_event *event, sd_bus **_bus) {
 }
 
 static int run(int argc, char *argv[]) {
-        _cleanup_(context_clear) Context context = {
-                .locale_context.mtime = USEC_INFINITY,
-                .vc_mtime = USEC_INFINITY,
-                .x11_mtime = USEC_INFINITY,
-        };
+        _cleanup_(context_clear) Context context = {};
         _cleanup_(sd_event_unrefp) sd_event *event = NULL;
         _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
         int r;