Merge pull request #18875 from keszybz/localed-error

localed: return error when setting a non-installed keymap

TODO

@@ -7,6 +7,8 @@ Bugfixes:
 * userdbctl: "Password OK: yes" is shown even when there are no passwords
   or the password is locked.
 
+* Get rid of nftw(). We should refuse to use such useless APIs on principle.
+
 External:
 
 * Fedora: add an rpmlint check that verifies that all unit files in the RPM are listed in %systemd_post macros.

src/basic/kbd-util.c

@@ -1,109 +0,0 @@
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-
-#include <ftw.h>
-
-#include "kbd-util.h"
-#include "log.h"
-#include "nulstr-util.h"
-#include "path-util.h"
-#include "set.h"
-#include "string-util.h"
-#include "strv.h"
-#include "utf8.h"
-
-static thread_local Set *keymaps = NULL;
-
-static int nftw_cb(
-                const char *fpath,
-                const struct stat *sb,
-                int tflag,
-                struct FTW *ftwbuf) {
-
-        _cleanup_free_ char *p = NULL;
-        char *e;
-        int r;
-
-        if (tflag != FTW_F)
-                return 0;
-
-        if (!endswith(fpath, ".map") &&
-            !endswith(fpath, ".map.gz"))
-                return 0;
-
-        p = strdup(basename(fpath));
-        if (!p)
-                return FTW_STOP;
-
-        e = endswith(p, ".map");
-        if (e)
-                *e = 0;
-
-        e = endswith(p, ".map.gz");
-        if (e)
-                *e = 0;
-
-        if (!keymap_is_valid(p))
-                return 0;
-
-        r = set_consume(keymaps, TAKE_PTR(p));
-        if (r < 0 && r != -EEXIST)
-                return r;
-
-        return 0;
-}
-
-int get_keymaps(char ***ret) {
-        _cleanup_strv_free_ char **l = NULL;
-        const char *dir;
-        int r;
-
-        keymaps = set_new(&string_hash_ops);
-        if (!keymaps)
-                return -ENOMEM;
-
-        NULSTR_FOREACH(dir, KBD_KEYMAP_DIRS) {
-                r = nftw(dir, nftw_cb, 20, FTW_PHYS|FTW_ACTIONRETVAL);
-
-                if (r == FTW_STOP)
-                        log_debug("Directory not found %s", dir);
-                else if (r < 0)
-                        log_debug_errno(r, "Can't add keymap: %m");
-        }
-
-        l = set_get_strv(keymaps);
-        if (!l) {
-                set_free_free(keymaps);
-                return -ENOMEM;
-        }
-
-        set_free(keymaps);
-
-        if (strv_isempty(l))
-                return -ENOENT;
-
-        strv_sort(l);
-
-        *ret = TAKE_PTR(l);
-
-        return 0;
-}
-
-bool keymap_is_valid(const char *name) {
-
-        if (isempty(name))
-                return false;
-
-        if (strlen(name) >= 128)
-                return false;
-
-        if (!utf8_is_valid(name))
-                return false;
-
-        if (!filename_is_valid(name))
-                return false;
-
-        if (!string_is_safe(name))
-                return false;
-
-        return true;
-}

src/basic/meson.build

@@ -83,8 +83,6 @@ basic_sources = files('''
         io-util.c
         io-util.h
         ioprio.h
-        kbd-util.c
-        kbd-util.h
         khash.c
         khash.h
         label.c

src/locale/localed.c

@@ -19,6 +19,7 @@
 #include "bus-polkit.h"
 #include "def.h"
 #include "dlfcn-util.h"
+#include "kbd-util.h"
 #include "keymap-util.h"
 #include "locale-util.h"
 #include "macro.h"
@@ -474,7 +475,7 @@ static int method_set_locale(sd_bus_message *m, void *userdata, sd_bus_error *er
 
 static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_error *error) {
         Context *c = userdata;
-        const char *keymap, *keymap_toggle;
+        const char *name, *keymap, *keymap_toggle;
         int convert, interactive, r;
 
         assert(m);
@@ -490,17 +491,23 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro
         r = vconsole_read_data(c, m);
         if (r < 0) {
                 log_error_errno(r, "Failed to read virtual console keymap data: %m");
-                return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Failed to read virtual console keymap data");
+                return sd_bus_error_set_errnof(error, r, "Failed to read virtual console keymap data: %m");
+        }
+
+        FOREACH_STRING(name, keymap ?: keymap_toggle, keymap ? keymap_toggle : NULL) {
+                r = keymap_exists(name); /* This also verifies that the keymap name is kosher. */
+                if (r < 0) {
+                        log_error_errno(r, "Failed to check keymap %s: %m", name);
+                        return sd_bus_error_set_errnof(error, r, "Failed to check keymap %s: %m", name);
+                }
+                if (r == 0)
+                        return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Keymap %s is not installed.", name);
         }
 
         if (streq_ptr(keymap, c->vc_keymap) &&
             streq_ptr(keymap_toggle, c->vc_keymap_toggle))
                 return sd_bus_reply_method_return(m, NULL);
 
-        if ((keymap && (!filename_is_valid(keymap) || !string_is_safe(keymap))) ||
-            (keymap_toggle && (!filename_is_valid(keymap_toggle) || !string_is_safe(keymap_toggle))))
-                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Received invalid keymap data");
-
         r = bus_verify_polkit_async(
                         m,
                         CAP_SYS_ADMIN,

src/shared/kbd-util.c

@@ -0,0 +1,134 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include <ftw.h>
+
+#include "errno-util.h"
+#include "kbd-util.h"
+#include "log.h"
+#include "nulstr-util.h"
+#include "path-util.h"
+#include "set.h"
+#include "string-util.h"
+#include "strv.h"
+#include "utf8.h"
+
+static thread_local const char *keymap_name = NULL;
+static thread_local Set *keymaps = NULL;
+
+static int nftw_cb(
+                const char *fpath,
+                const struct stat *sb,
+                int tflag,
+                struct FTW *ftwbuf) {
+
+        _cleanup_free_ char *p = NULL;
+        int r;
+
+        /* If keymap_name is non-null, return true if keymap keymap_name is found.
+         * Otherwise, add all keymaps to keymaps. */
+
+        if (tflag != FTW_F)
+                return 0;
+
+        fpath = basename(fpath);
+
+        const char *e = endswith(fpath, ".map") ?: endswith(fpath, ".map.gz");
+        if (!e)
+                return 0;
+
+        p = strndup(fpath, e - fpath);
+        if (!p) {
+                errno = ENOMEM;
+                return -1;
+        }
+
+        if (keymap_name)
+                return streq(p, keymap_name);
+
+        if (!keymap_is_valid(p))
+                return 0;
+
+        r = set_consume(keymaps, TAKE_PTR(p));
+        if (r < 0 && r != -EEXIST) {
+                errno = -r;
+                return -1;
+        }
+
+        return 0;
+}
+
+int get_keymaps(char ***ret) {
+        keymaps = set_new(&string_hash_ops);
+        if (!keymaps)
+                return -ENOMEM;
+
+        const char *dir;
+        NULSTR_FOREACH(dir, KBD_KEYMAP_DIRS)
+                if (nftw(dir, nftw_cb, 20, FTW_PHYS) < 0) {
+                        if (errno == ENOENT)
+                                continue;
+                        if (ERRNO_IS_RESOURCE(errno)) {
+                                keymaps = set_free_free(keymaps);
+                                return log_warning_errno(errno, "Failed to read keymap list from %s: %m", dir);
+                        }
+                        log_debug_errno(errno, "Failed to read keymap list from %s, ignoring: %m", dir);
+                }
+
+        _cleanup_strv_free_ char **l = set_get_strv(keymaps);
+        if (!l) {
+                keymaps = set_free_free(keymaps);
+                return -ENOMEM;
+        }
+
+        keymaps = set_free(keymaps);
+
+        if (strv_isempty(l))
+                return -ENOENT;
+
+        strv_sort(l);
+
+        *ret = TAKE_PTR(l);
+
+        return 0;
+}
+
+bool keymap_is_valid(const char *name) {
+        if (isempty(name))
+                return false;
+
+        if (strlen(name) >= 128)
+                return false;
+
+        if (!utf8_is_valid(name))
+                return false;
+
+        if (!filename_is_valid(name))
+                return false;
+
+        if (!string_is_safe(name))
+                return false;
+
+        return true;
+}
+
+int keymap_exists(const char *name) {
+        int r = 0;
+
+        if (!keymap_is_valid(name))
+                return -EINVAL;
+
+        keymap_name = name;
+
+        const char *dir;
+        NULSTR_FOREACH(dir, KBD_KEYMAP_DIRS) {
+                r = nftw(dir, nftw_cb, 20, FTW_PHYS);
+                if (r > 0)
+                        break;
+                if (r < 0 && errno != ENOENT)
+                        log_debug_errno(errno, "Failed to read keymap list from %s, ignoring: %m", dir);
+        }
+
+        keymap_name = NULL;
+
+        return r > 0;
+}

src/shared/kbd-util.h

@@ -18,3 +18,4 @@
 
 int get_keymaps(char ***l);
 bool keymap_is_valid(const char *name);
+int keymap_exists(const char *name);

src/shared/meson.build

@@ -149,6 +149,8 @@ shared_sources = files('''
         json-internal.h
         json.c
         json.h
+        kbd-util.c
+        kbd-util.h
         killall.c
         killall.h
         libcrypt-util.c