From dd57a22e6a677d8684302eb1beac8803a850c55d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 12 Dec 2022 15:53:50 +0900 Subject: [PATCH 1/7] env-file: several cleanups - add missing assertions, - rename function arguments for storing results, - add typedef for callback. --- src/basic/env-file.c | 47 ++++++++++++++++++++++++++++++++++++++------ src/basic/env-file.h | 4 ++-- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/basic/env-file.c b/src/basic/env-file.c index e363bc80bfe..af82ddfb630 100644 --- a/src/basic/env-file.c +++ b/src/basic/env-file.c @@ -12,11 +12,17 @@ #include "tmpfile-util.h" #include "utf8.h" +typedef int (*push_env_func_t)( + const char *filename, + unsigned line, + const char *key, + char *value, + void *userdata); + static int parse_env_file_internal( FILE *f, const char *fname, - int (*push) (const char *filename, unsigned line, - const char *key, char *value, void *userdata), + push_env_func_t push, void *userdata) { size_t n_key = 0, n_value = 0, last_value_whitespace = SIZE_MAX, last_key_whitespace = SIZE_MAX; @@ -37,6 +43,9 @@ static int parse_env_file_internal( COMMENT_ESCAPE } state = PRE_KEY; + assert(f || fname); + assert(push); + if (f) r = read_full_stream(f, &contents, NULL); else @@ -274,6 +283,8 @@ static int check_utf8ness_and_warn( const char *filename, unsigned line, const char *key, char *value) { + assert(key); + if (!utf8_is_valid(key)) { _cleanup_free_ char *p = NULL; @@ -304,6 +315,8 @@ static int parse_env_file_push( va_list aq, *ap = userdata; int r; + assert(key); + r = check_utf8ness_and_warn(filename, line, key, value); if (r < 0) return r; @@ -338,6 +351,8 @@ int parse_env_filev( int r; va_list aq; + assert(f || fname); + va_copy(aq, ap); r = parse_env_file_internal(f, fname, parse_env_file_push, &aq); va_end(aq); @@ -352,6 +367,8 @@ int parse_env_file_sentinel( va_list ap; int r; + assert(f || fname); + va_start(ap, fname); r = parse_env_filev(f, fname, ap); va_end(ap); @@ -363,10 +380,13 @@ static int load_env_file_push( const char *filename, unsigned line, const char *key, char *value, void *userdata) { + char ***m = userdata; char *p; int r; + assert(key); + r = check_utf8ness_and_warn(filename, line, key, value); if (r < 0) return r; @@ -383,15 +403,18 @@ static int load_env_file_push( return 0; } -int load_env_file(FILE *f, const char *fname, char ***rl) { +int load_env_file(FILE *f, const char *fname, char ***ret) { _cleanup_strv_free_ char **m = NULL; int r; + assert(f || fname); + assert(ret); + r = parse_env_file_internal(f, fname, load_env_file_push, &m); if (r < 0) return r; - *rl = TAKE_PTR(m); + *ret = TAKE_PTR(m); return 0; } @@ -403,6 +426,8 @@ static int load_env_file_push_pairs( char ***m = ASSERT_PTR(userdata); int r; + assert(key); + r = check_utf8ness_and_warn(filename, line, key, value); if (r < 0) return r; @@ -426,15 +451,17 @@ static int load_env_file_push_pairs( return strv_extend(m, ""); } -int load_env_file_pairs(FILE *f, const char *fname, char ***rl) { +int load_env_file_pairs(FILE *f, const char *fname, char ***ret) { _cleanup_strv_free_ char **m = NULL; int r; + assert(f || fname); + r = parse_env_file_internal(f, fname, load_env_file_push_pairs, &m); if (r < 0) return r; - *rl = TAKE_PTR(m); + *ret = TAKE_PTR(m); return 0; } @@ -446,6 +473,8 @@ static int merge_env_file_push( char ***env = ASSERT_PTR(userdata); char *expanded_value; + assert(key); + if (!value) { log_error("%s:%u: invalid syntax (around \"%s\"), ignoring.", strna(filename), line, key); return 0; @@ -476,6 +505,9 @@ int merge_env_file( FILE *f, const char *fname) { + assert(env); + assert(f || fname); + /* NOTE: this function supports braceful and braceless variable expansions, * plus "extended" substitutions, unlike other exported parsing functions. */ @@ -486,6 +518,9 @@ int merge_env_file( static void write_env_var(FILE *f, const char *v) { const char *p; + assert(f); + assert(v); + p = strchr(v, '='); if (!p) { /* Fallback */ diff --git a/src/basic/env-file.h b/src/basic/env-file.h index de475885ace..8da451c74a5 100644 --- a/src/basic/env-file.h +++ b/src/basic/env-file.h @@ -9,8 +9,8 @@ int parse_env_filev(FILE *f, const char *fname, va_list ap); int parse_env_file_sentinel(FILE *f, const char *fname, ...) _sentinel_; #define parse_env_file(f, fname, ...) parse_env_file_sentinel(f, fname, __VA_ARGS__, NULL) -int load_env_file(FILE *f, const char *fname, char ***l); -int load_env_file_pairs(FILE *f, const char *fname, char ***l); +int load_env_file(FILE *f, const char *fname, char ***ret); +int load_env_file_pairs(FILE *f, const char *fname, char ***ret); int merge_env_file(char ***env, FILE *f, const char *fname); From 649512b934f4a762c14380dea4a096263b291feb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 13 Dec 2022 17:24:19 +0900 Subject: [PATCH 2/7] env-file: introduce parse_env_file_fd() --- src/basic/env-file.c | 29 +++++++++++++++++++++++++++++ src/basic/env-file.h | 2 ++ 2 files changed, 31 insertions(+) diff --git a/src/basic/env-file.c b/src/basic/env-file.c index af82ddfb630..5a4b21ee6aa 100644 --- a/src/basic/env-file.c +++ b/src/basic/env-file.c @@ -376,6 +376,35 @@ int parse_env_file_sentinel( return r; } +int parse_env_file_fd_sentinel( + int fd, + const char *fname, /* only used for logging */ + ...) { + + _cleanup_close_ int fd_ro = -EBADFD; + _cleanup_fclose_ FILE *f = NULL; + va_list ap; + int r; + + assert(fd >= 0); + + 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); + + va_start(ap, fname); + r = parse_env_filev(f, fname, ap); + va_end(ap); + + return r; +} + static int load_env_file_push( const char *filename, unsigned line, const char *key, char *value, diff --git a/src/basic/env-file.h b/src/basic/env-file.h index 8da451c74a5..2448d943cd4 100644 --- a/src/basic/env-file.h +++ b/src/basic/env-file.h @@ -9,6 +9,8 @@ int parse_env_filev(FILE *f, const char *fname, va_list ap); int parse_env_file_sentinel(FILE *f, const char *fname, ...) _sentinel_; #define parse_env_file(f, fname, ...) parse_env_file_sentinel(f, fname, __VA_ARGS__, NULL) +int parse_env_file_fd_sentinel(int fd, const char *fname, ...) _sentinel_; +#define parse_env_file_fd(fd, fname, ...) parse_env_file_fd_sentinel(fd, fname, __VA_ARGS__, NULL) int load_env_file(FILE *f, const char *fname, char ***ret); int load_env_file_pairs(FILE *f, const char *fname, char ***ret); From 7294912f1819fca6cb8b017343d90036ae9a0107 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 13 Dec 2022 17:30:12 +0900 Subject: [PATCH 3/7] localed: avoid TOCTOU in loading config --- src/locale/localed-util.c | 73 ++++++++++++++++++++++----------------- src/locale/localed-util.h | 7 ++-- src/locale/localed.c | 2 -- 3 files changed, 45 insertions(+), 37 deletions(-) diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c index 8fb65b96997..cace820bddb 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 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 (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->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 5470d1bb9b6..b09ed80b83f 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 + #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 a2014e3da0f..ebce509b306 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -766,8 +766,6 @@ 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_(sd_event_unrefp) sd_event *event = NULL; _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; From 018befcff6b51f8a50ca232e1984d34526037241 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 13 Dec 2022 17:59:35 +0900 Subject: [PATCH 4/7] locale-setup: split locale_context_load() into small pieces No functional change, just refactoring and preparation for later commits. --- src/shared/locale-setup.c | 195 +++++++++++++++++++++++--------------- 1 file changed, 116 insertions(+), 79 deletions(-) diff --git a/src/shared/locale-setup.c b/src/shared/locale-setup.c index b8c6647e7c5..4b0d46995ba 100644 --- a/src/shared/locale-setup.c +++ b/src/shared/locale-setup.c @@ -19,94 +19,131 @@ void locale_context_clear(LocaleContext *c) { c->locale[i] = mfree(c->locale[i]); } +static int locale_context_load_proc(LocaleContext *c, LocaleLoadFlag flag) { + int r; + + assert(c); + + if (!FLAGS_SET(flag, LOCALE_LOAD_PROC_CMDLINE)) + return 0; + + locale_context_clear(c); + + r = proc_cmdline_get_key_many(PROC_CMDLINE_STRIP_RD_PREFIX, + "locale.LANG", &c->locale[VARIABLE_LANG], + "locale.LANGUAGE", &c->locale[VARIABLE_LANGUAGE], + "locale.LC_CTYPE", &c->locale[VARIABLE_LC_CTYPE], + "locale.LC_NUMERIC", &c->locale[VARIABLE_LC_NUMERIC], + "locale.LC_TIME", &c->locale[VARIABLE_LC_TIME], + "locale.LC_COLLATE", &c->locale[VARIABLE_LC_COLLATE], + "locale.LC_MONETARY", &c->locale[VARIABLE_LC_MONETARY], + "locale.LC_MESSAGES", &c->locale[VARIABLE_LC_MESSAGES], + "locale.LC_PAPER", &c->locale[VARIABLE_LC_PAPER], + "locale.LC_NAME", &c->locale[VARIABLE_LC_NAME], + "locale.LC_ADDRESS", &c->locale[VARIABLE_LC_ADDRESS], + "locale.LC_TELEPHONE", &c->locale[VARIABLE_LC_TELEPHONE], + "locale.LC_MEASUREMENT", &c->locale[VARIABLE_LC_MEASUREMENT], + "locale.LC_IDENTIFICATION", &c->locale[VARIABLE_LC_IDENTIFICATION]); + if (r == -ENOENT) + return 0; + if (r < 0) + return log_debug_errno(r, "Failed to read /proc/cmdline: %m"); + return r; +} + +static int locale_context_load_conf(LocaleContext *c, LocaleLoadFlag flag) { + struct stat st; + usec_t t; + int r; + + assert(c); + + if (!FLAGS_SET(flag, LOCALE_LOAD_LOCALE_CONF)) + return 0; + + r = stat("/etc/locale.conf", &st); + if (r < 0) { + if (errno == ENOENT) + return 0; + return log_debug_errno(errno, "Failed to stat /etc/locale.conf: %m"); + } + + /* If mtime is not changed, then we do not need to re-read the file. */ + t = timespec_load(&st.st_mtim); + if (c->mtime != USEC_INFINITY && t == c->mtime) + return 0; + + locale_context_clear(c); + c->mtime = t; + + r = parse_env_file(NULL, "/etc/locale.conf", + "LANG", &c->locale[VARIABLE_LANG], + "LANGUAGE", &c->locale[VARIABLE_LANGUAGE], + "LC_CTYPE", &c->locale[VARIABLE_LC_CTYPE], + "LC_NUMERIC", &c->locale[VARIABLE_LC_NUMERIC], + "LC_TIME", &c->locale[VARIABLE_LC_TIME], + "LC_COLLATE", &c->locale[VARIABLE_LC_COLLATE], + "LC_MONETARY", &c->locale[VARIABLE_LC_MONETARY], + "LC_MESSAGES", &c->locale[VARIABLE_LC_MESSAGES], + "LC_PAPER", &c->locale[VARIABLE_LC_PAPER], + "LC_NAME", &c->locale[VARIABLE_LC_NAME], + "LC_ADDRESS", &c->locale[VARIABLE_LC_ADDRESS], + "LC_TELEPHONE", &c->locale[VARIABLE_LC_TELEPHONE], + "LC_MEASUREMENT", &c->locale[VARIABLE_LC_MEASUREMENT], + "LC_IDENTIFICATION", &c->locale[VARIABLE_LC_IDENTIFICATION]); + if (r < 0) + return log_debug_errno(r, "Failed to read /etc/locale.conf: %m"); + + return 1; /* loaded */ +} + +static int locale_context_load_env(LocaleContext *c, LocaleLoadFlag flag) { + int r; + + assert(c); + + if (!FLAGS_SET(flag, LOCALE_LOAD_ENVIRONMENT)) + return 0; + + locale_context_clear(c); + + /* Fill in what we got passed from systemd. */ + for (LocaleVariable p = 0; p < _VARIABLE_LC_MAX; p++) { + const char *name = ASSERT_PTR(locale_variable_to_string(p)); + + r = free_and_strdup(&c->locale[p], empty_to_null(getenv(name))); + if (r < 0) + return log_oom_debug(); + } + + return 1; /* loaded */ +} + int locale_context_load(LocaleContext *c, LocaleLoadFlag flag) { int r; assert(c); - if (FLAGS_SET(flag, LOCALE_LOAD_PROC_CMDLINE)) { - locale_context_clear(c); - - r = proc_cmdline_get_key_many(PROC_CMDLINE_STRIP_RD_PREFIX, - "locale.LANG", &c->locale[VARIABLE_LANG], - "locale.LANGUAGE", &c->locale[VARIABLE_LANGUAGE], - "locale.LC_CTYPE", &c->locale[VARIABLE_LC_CTYPE], - "locale.LC_NUMERIC", &c->locale[VARIABLE_LC_NUMERIC], - "locale.LC_TIME", &c->locale[VARIABLE_LC_TIME], - "locale.LC_COLLATE", &c->locale[VARIABLE_LC_COLLATE], - "locale.LC_MONETARY", &c->locale[VARIABLE_LC_MONETARY], - "locale.LC_MESSAGES", &c->locale[VARIABLE_LC_MESSAGES], - "locale.LC_PAPER", &c->locale[VARIABLE_LC_PAPER], - "locale.LC_NAME", &c->locale[VARIABLE_LC_NAME], - "locale.LC_ADDRESS", &c->locale[VARIABLE_LC_ADDRESS], - "locale.LC_TELEPHONE", &c->locale[VARIABLE_LC_TELEPHONE], - "locale.LC_MEASUREMENT", &c->locale[VARIABLE_LC_MEASUREMENT], - "locale.LC_IDENTIFICATION", &c->locale[VARIABLE_LC_IDENTIFICATION]); - if (r < 0 && r != -ENOENT) - log_debug_errno(r, "Failed to read /proc/cmdline, ignoring: %m"); - if (r > 0) - goto finalize; - } - - if (FLAGS_SET(flag, LOCALE_LOAD_LOCALE_CONF)) { - struct stat st; - usec_t t; - - r = stat("/etc/locale.conf", &st); - if (r < 0 && errno != ENOENT) - return log_debug_errno(errno, "Failed to stat /etc/locale.conf: %m"); - - if (r >= 0) { - /* If mtime is not changed, then we do not need to re-read the file. */ - t = timespec_load(&st.st_mtim); - if (c->mtime != USEC_INFINITY && t == c->mtime) - return 0; - - locale_context_clear(c); - c->mtime = t; - - r = parse_env_file(NULL, "/etc/locale.conf", - "LANG", &c->locale[VARIABLE_LANG], - "LANGUAGE", &c->locale[VARIABLE_LANGUAGE], - "LC_CTYPE", &c->locale[VARIABLE_LC_CTYPE], - "LC_NUMERIC", &c->locale[VARIABLE_LC_NUMERIC], - "LC_TIME", &c->locale[VARIABLE_LC_TIME], - "LC_COLLATE", &c->locale[VARIABLE_LC_COLLATE], - "LC_MONETARY", &c->locale[VARIABLE_LC_MONETARY], - "LC_MESSAGES", &c->locale[VARIABLE_LC_MESSAGES], - "LC_PAPER", &c->locale[VARIABLE_LC_PAPER], - "LC_NAME", &c->locale[VARIABLE_LC_NAME], - "LC_ADDRESS", &c->locale[VARIABLE_LC_ADDRESS], - "LC_TELEPHONE", &c->locale[VARIABLE_LC_TELEPHONE], - "LC_MEASUREMENT", &c->locale[VARIABLE_LC_MEASUREMENT], - "LC_IDENTIFICATION", &c->locale[VARIABLE_LC_IDENTIFICATION]); - if (r < 0) - return log_debug_errno(r, "Failed to read /etc/locale.conf: %m"); - - goto finalize; - } - } - - if (FLAGS_SET(flag, LOCALE_LOAD_ENVIRONMENT)) { - locale_context_clear(c); - - /* Fill in what we got passed from systemd. */ - for (LocaleVariable p = 0; p < _VARIABLE_LC_MAX; p++) { - const char *name = ASSERT_PTR(locale_variable_to_string(p)); - - r = free_and_strdup(&c->locale[p], empty_to_null(getenv(name))); - if (r < 0) - return log_oom_debug(); - } - + r = locale_context_load_proc(c, flag); + if (r > 0) goto finalize; - } - /* Nothing loaded. */ - locale_context_clear(c); - return 0; + r = locale_context_load_conf(c, flag); + if (r != 0) + goto finalize; + + r = locale_context_load_env(c, flag); finalize: + if (r < 0) + return r; + + if (r == 0) { + /* Nothing loaded. */ + locale_context_clear(c); + return 0; + } + if (FLAGS_SET(flag, LOCALE_LOAD_SIMPLIFY)) locale_variables_simplify(c->locale); From 50359113e9fa9c48731efa7dbd725fe11b22cab0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 13 Dec 2022 18:01:36 +0900 Subject: [PATCH 5/7] locale-setup: clear partially loaded context on failure --- src/shared/locale-setup.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/shared/locale-setup.c b/src/shared/locale-setup.c index 4b0d46995ba..1b85ec41358 100644 --- a/src/shared/locale-setup.c +++ b/src/shared/locale-setup.c @@ -135,13 +135,10 @@ int locale_context_load(LocaleContext *c, LocaleLoadFlag flag) { r = locale_context_load_env(c, flag); finalize: - if (r < 0) - return r; - - if (r == 0) { - /* Nothing loaded. */ + if (r <= 0) { + /* Nothing loaded, or error. */ locale_context_clear(c); - return 0; + return r; } if (FLAGS_SET(flag, LOCALE_LOAD_SIMPLIFY)) From 00afa6a38fbd05f79ffc1b1365ad68f63e7ac335 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 13 Dec 2022 18:02:57 +0900 Subject: [PATCH 6/7] locale-setup: make failure in stat() critical The file was created by this function. Hence, the failure is something critical. --- src/shared/locale-setup.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/shared/locale-setup.c b/src/shared/locale-setup.c index 1b85ec41358..cd994cffb7d 100644 --- a/src/shared/locale-setup.c +++ b/src/shared/locale-setup.c @@ -208,8 +208,10 @@ int locale_context_save(LocaleContext *c, char ***ret_set, char ***ret_unset) { if (r < 0) return r; - if (stat("/etc/locale.conf", &st) >= 0) - c->mtime = timespec_load(&st.st_mtim); + if (stat("/etc/locale.conf", &st) < 0) + return -errno; + + c->mtime = timespec_load(&st.st_mtim); if (ret_set) *ret_set = TAKE_PTR(set); From 56b7f112ed7e3e5e6f495e8d01a89c292c9f8847 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 13 Dec 2022 18:04:48 +0900 Subject: [PATCH 7/7] locale-setup: avoid TOCTOU in reading locale.conf --- src/locale/localectl.c | 2 +- src/locale/localed.c | 4 +-- src/shared/locale-setup.c | 68 ++++++++++++++++++++------------------- src/shared/locale-setup.h | 5 +-- 4 files changed, 40 insertions(+), 39 deletions(-) diff --git a/src/locale/localectl.c b/src/locale/localectl.c index 966f07d0832..fb83881cc71 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.c b/src/locale/localed.c index ebce509b306..737cc690795 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -764,9 +764,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, - }; + _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; diff --git a/src/shared/locale-setup.c b/src/shared/locale-setup.c index cd994cffb7d..64af9d57b8f 100644 --- a/src/shared/locale-setup.c +++ b/src/shared/locale-setup.c @@ -6,14 +6,17 @@ #include "env-file-label.h" #include "env-file.h" #include "env-util.h" +#include "errno-util.h" +#include "fd-util.h" #include "locale-setup.h" #include "proc-cmdline.h" +#include "stat-util.h" #include "strv.h" void locale_context_clear(LocaleContext *c) { assert(c); - c->mtime = USEC_INFINITY; + c->st = (struct stat) {}; for (LocaleVariable i = 0; i < _VARIABLE_LC_MAX; i++) c->locale[i] = mfree(c->locale[i]); @@ -52,8 +55,8 @@ static int locale_context_load_proc(LocaleContext *c, LocaleLoadFlag flag) { } static int locale_context_load_conf(LocaleContext *c, LocaleLoadFlag flag) { + _cleanup_close_ int fd = -EBADFD; struct stat st; - usec_t t; int r; assert(c); @@ -61,36 +64,37 @@ static int locale_context_load_conf(LocaleContext *c, LocaleLoadFlag flag) { if (!FLAGS_SET(flag, LOCALE_LOAD_LOCALE_CONF)) return 0; - r = stat("/etc/locale.conf", &st); - if (r < 0) { - if (errno == ENOENT) - return 0; - return log_debug_errno(errno, "Failed to stat /etc/locale.conf: %m"); - } + fd = RET_NERRNO(open("/etc/locale.conf", O_CLOEXEC | O_PATH)); + if (fd == -ENOENT) + return 0; + if (fd < 0) + return log_debug_errno(errno, "Failed to open /etc/locale.conf: %m"); - /* If mtime is not changed, then we do not need to re-read the file. */ - t = timespec_load(&st.st_mtim); - if (c->mtime != USEC_INFINITY && t == c->mtime) + if (fstat(fd, &st) < 0) + return log_debug_errno(errno, "Failed to stat /etc/locale.conf: %m"); + + /* If the file is not changed, then we do not need to re-read the file. */ + if (stat_inode_unmodified(&c->st, &st)) return 0; + c->st = st; locale_context_clear(c); - c->mtime = t; - r = parse_env_file(NULL, "/etc/locale.conf", - "LANG", &c->locale[VARIABLE_LANG], - "LANGUAGE", &c->locale[VARIABLE_LANGUAGE], - "LC_CTYPE", &c->locale[VARIABLE_LC_CTYPE], - "LC_NUMERIC", &c->locale[VARIABLE_LC_NUMERIC], - "LC_TIME", &c->locale[VARIABLE_LC_TIME], - "LC_COLLATE", &c->locale[VARIABLE_LC_COLLATE], - "LC_MONETARY", &c->locale[VARIABLE_LC_MONETARY], - "LC_MESSAGES", &c->locale[VARIABLE_LC_MESSAGES], - "LC_PAPER", &c->locale[VARIABLE_LC_PAPER], - "LC_NAME", &c->locale[VARIABLE_LC_NAME], - "LC_ADDRESS", &c->locale[VARIABLE_LC_ADDRESS], - "LC_TELEPHONE", &c->locale[VARIABLE_LC_TELEPHONE], - "LC_MEASUREMENT", &c->locale[VARIABLE_LC_MEASUREMENT], - "LC_IDENTIFICATION", &c->locale[VARIABLE_LC_IDENTIFICATION]); + r = parse_env_file_fd(fd, "/etc/locale.conf", + "LANG", &c->locale[VARIABLE_LANG], + "LANGUAGE", &c->locale[VARIABLE_LANGUAGE], + "LC_CTYPE", &c->locale[VARIABLE_LC_CTYPE], + "LC_NUMERIC", &c->locale[VARIABLE_LC_NUMERIC], + "LC_TIME", &c->locale[VARIABLE_LC_TIME], + "LC_COLLATE", &c->locale[VARIABLE_LC_COLLATE], + "LC_MONETARY", &c->locale[VARIABLE_LC_MONETARY], + "LC_MESSAGES", &c->locale[VARIABLE_LC_MESSAGES], + "LC_PAPER", &c->locale[VARIABLE_LC_PAPER], + "LC_NAME", &c->locale[VARIABLE_LC_NAME], + "LC_ADDRESS", &c->locale[VARIABLE_LC_ADDRESS], + "LC_TELEPHONE", &c->locale[VARIABLE_LC_TELEPHONE], + "LC_MEASUREMENT", &c->locale[VARIABLE_LC_MEASUREMENT], + "LC_IDENTIFICATION", &c->locale[VARIABLE_LC_IDENTIFICATION]); if (r < 0) return log_debug_errno(r, "Failed to read /etc/locale.conf: %m"); @@ -181,7 +185,6 @@ int locale_context_build_env(const LocaleContext *c, char ***ret_set, char ***re int locale_context_save(LocaleContext *c, char ***ret_set, char ***ret_unset) { _cleanup_strv_free_ char **set = NULL, **unset = NULL; - struct stat st; int r; assert(c); @@ -196,7 +199,8 @@ int locale_context_save(LocaleContext *c, char ***ret_set, char ***ret_unset) { if (unlink("/etc/locale.conf") < 0) return errno == ENOENT ? 0 : -errno; - c->mtime = USEC_INFINITY; + c->st = (struct stat) {}; + if (ret_set) *ret_set = NULL; if (ret_unset) @@ -208,11 +212,9 @@ int locale_context_save(LocaleContext *c, char ***ret_set, char ***ret_unset) { if (r < 0) return r; - if (stat("/etc/locale.conf", &st) < 0) + if (stat("/etc/locale.conf", &c->st) < 0) return -errno; - c->mtime = timespec_load(&st.st_mtim); - if (ret_set) *ret_set = TAKE_PTR(set); if (ret_unset) @@ -254,7 +256,7 @@ bool locale_context_equal(const LocaleContext *c, char *l[_VARIABLE_LC_MAX]) { } int locale_setup(char ***environment) { - _cleanup_(locale_context_clear) LocaleContext c = { .mtime = USEC_INFINITY }; + _cleanup_(locale_context_clear) LocaleContext c = {}; _cleanup_strv_free_ char **add = NULL; int r; diff --git a/src/shared/locale-setup.h b/src/shared/locale-setup.h index ec3fc8c364a..537acc72df8 100644 --- a/src/shared/locale-setup.h +++ b/src/shared/locale-setup.h @@ -1,11 +1,12 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include + #include "locale-util.h" -#include "time-util.h" typedef struct LocaleContext { - usec_t mtime; + struct stat st; char *locale[_VARIABLE_LC_MAX]; } LocaleContext;