From 58f4458afd0b9dce77c814d2ff9c73ca80651ecd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 4 Jul 2018 10:53:21 +0200 Subject: [PATCH 1/7] man: fix typo --- man/os-release.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/os-release.xml b/man/os-release.xml index a51edf3b8a..560d095a14 100644 --- a/man/os-release.xml +++ b/man/os-release.xml @@ -234,7 +234,7 @@ if there is any. This is primarily intended for operating systems that rely on community QA. PRIVACY_POLICY_URL= should refer to the - main privacy policy page for the operation system, if there is + main privacy policy page for the operating system, if there is any. These settings are optional, and providing only some of these settings is common. These URLs are intended to be exposed in "About this system" UIs behind links with captions From 6755bb5548532f728ca06742d7e7abad14d41e7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 6 Jul 2018 11:51:04 +0200 Subject: [PATCH 2/7] core/kmod-setup: restore comments They were removed in 7491e6e7c5fcb3c445a0656a440bd1551adb6ba1, but the original version is more informative. Let's add them back. --- src/core/kmod-setup.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/core/kmod-setup.c b/src/core/kmod-setup.c index 9251929558..b8292e77fd 100644 --- a/src/core/kmod-setup.c +++ b/src/core/kmod-setup.c @@ -76,13 +76,15 @@ int kmod_setup(void) { bool warn_if_module:1; bool (*condition_fn)(void); } kmod_table[] = { - /* auto-loading on use doesn't work before udev is up */ + /* This one we need to load explicitly, since auto-loading on use doesn't work + * before udev created the ghost device nodes, and we need it earlier than that. */ { "autofs4", "/sys/class/misc/autofs", true, false, NULL }, - /* early configure of ::1 on the loopback device */ + /* This one we need to load explicitly, since auto-loading of IPv6 is not done when + * we try to configure ::1 on the loopback device. */ { "ipv6", "/sys/module/ipv6", false, true, NULL }, - /* this should never be a module */ + /* This should never be a module */ { "unix", "/proc/net/unix", true, true, NULL }, #if HAVE_LIBIPTC From 3cb9b42af3b205fba176ebf51ce0e07739698278 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 6 Jul 2018 11:57:54 +0200 Subject: [PATCH 3/7] Move module-util.h to src/shared/ and load_module() to libshared Unfortunately this needs libshared to link to libkmod. Before it was linked into systemd-udevd, udevadm, and systemd each seperately. On most systems this doesn't make much difference, because at least systemd would be installed, but it might not be in small chroots. It is a small library, so I hope this is not a big issue. --- src/basic/meson.build | 1 - src/modules-load/modules-load.c | 63 +--------------------------- src/shared/meson.build | 3 ++ src/shared/module-util.c | 64 +++++++++++++++++++++++++++++ src/{basic => shared}/module-util.h | 2 + 5 files changed, 71 insertions(+), 62 deletions(-) create mode 100644 src/shared/module-util.c rename src/{basic => shared}/module-util.h (81%) diff --git a/src/basic/meson.build b/src/basic/meson.build index 31625b1785..2fa2681b47 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -121,7 +121,6 @@ basic_sources = files(''' mkdir-label.c mkdir.c mkdir.h - module-util.h mount-util.c mount-util.h nss-util.h diff --git a/src/modules-load/modules-load.c b/src/modules-load/modules-load.c index c49a6b7a76..b3a4e818b6 100644 --- a/src/modules-load/modules-load.c +++ b/src/modules-load/modules-load.c @@ -59,65 +59,6 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat return 0; } -static int load_module(struct kmod_ctx *ctx, const char *m) { - const int probe_flags = KMOD_PROBE_APPLY_BLACKLIST; - struct kmod_list *itr; - _cleanup_(kmod_module_unref_listp) struct kmod_list *modlist = NULL; - int r = 0; - - log_debug("load: %s", m); - - r = kmod_module_new_from_lookup(ctx, m, &modlist); - if (r < 0) - return log_error_errno(r, "Failed to lookup alias '%s': %m", m); - - if (!modlist) { - log_error("Failed to find module '%s'", m); - return -ENOENT; - } - - kmod_list_foreach(itr, modlist) { - _cleanup_(kmod_module_unrefp) struct kmod_module *mod = NULL; - int state, err; - - mod = kmod_module_get_module(itr); - state = kmod_module_get_initstate(mod); - - switch (state) { - case KMOD_MODULE_BUILTIN: - log_info("Module '%s' is builtin", kmod_module_get_name(mod)); - break; - - case KMOD_MODULE_LIVE: - log_debug("Module '%s' is already loaded", kmod_module_get_name(mod)); - break; - - default: - err = kmod_module_probe_insert_module(mod, probe_flags, - NULL, NULL, NULL, NULL); - - if (err == 0) - log_info("Inserted module '%s'", kmod_module_get_name(mod)); - else if (err == KMOD_PROBE_APPLY_BLACKLIST) - log_info("Module '%s' is blacklisted", kmod_module_get_name(mod)); - else { - assert(err < 0); - - log_full_errno(err == ENODEV ? LOG_NOTICE : - err == ENOENT ? LOG_WARNING : - LOG_ERR, - err, - "Failed to insert '%s': %m", - kmod_module_get_name(mod)); - if (!IN_SET(err, ENODEV, ENOENT)) - r = err; - } - } - } - - return r; -} - static int apply_file(struct kmod_ctx *ctx, const char *path, bool ignore_enoent) { _cleanup_fclose_ FILE *f = NULL; int r; @@ -151,7 +92,7 @@ static int apply_file(struct kmod_ctx *ctx, const char *path, bool ignore_enoent if (strchr(COMMENTS "\n", *l)) continue; - k = load_module(ctx, l); + k = module_load_and_warn(ctx, l); if (k < 0 && r == 0) r = k; } @@ -248,7 +189,7 @@ int main(int argc, char *argv[]) { char **fn, **i; STRV_FOREACH(i, arg_proc_cmdline_modules) { - k = load_module(ctx, *i); + k = module_load_and_warn(ctx, *i); if (k < 0 && r == 0) r = k; } diff --git a/src/shared/meson.build b/src/shared/meson.build index 54e77e9af6..9c80f2b855 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -63,6 +63,8 @@ shared_sources = files(''' machine-image.h machine-pool.c machine-pool.h + module-util.h + module-util.c nsflags.c nsflags.h output-mode.c @@ -132,6 +134,7 @@ libshared_deps = [threads, libcryptsetup, libgcrypt, libiptc, + libkmod, libseccomp, libselinux, libidn, diff --git a/src/shared/module-util.c b/src/shared/module-util.c new file mode 100644 index 0000000000..36f4f364c1 --- /dev/null +++ b/src/shared/module-util.c @@ -0,0 +1,64 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + +#include + +#include "module-util.h" + +int module_load_and_warn(struct kmod_ctx *ctx, const char *module) { + const int probe_flags = KMOD_PROBE_APPLY_BLACKLIST; + struct kmod_list *itr; + _cleanup_(kmod_module_unref_listp) struct kmod_list *modlist = NULL; + int r = 0; + + log_debug("Loading module: %s", module); + + r = kmod_module_new_from_lookup(ctx, module, &modlist); + if (r < 0) + return log_error_errno(r, "Failed to lookup module alias '%s': %m", module); + + if (!modlist) { + log_error("Failed to find module '%s'", module); + return -ENOENT; + } + + kmod_list_foreach(itr, modlist) { + _cleanup_(kmod_module_unrefp) struct kmod_module *mod = NULL; + int state, err; + + mod = kmod_module_get_module(itr); + state = kmod_module_get_initstate(mod); + + switch (state) { + case KMOD_MODULE_BUILTIN: + log_info("Module '%s' is builtin", kmod_module_get_name(mod)); + break; + + case KMOD_MODULE_LIVE: + log_debug("Module '%s' is already loaded", kmod_module_get_name(mod)); + break; + + default: + err = kmod_module_probe_insert_module(mod, probe_flags, + NULL, NULL, NULL, NULL); + + if (err == 0) + log_info("Inserted module '%s'", kmod_module_get_name(mod)); + else if (err == KMOD_PROBE_APPLY_BLACKLIST) + log_info("Module '%s' is blacklisted", kmod_module_get_name(mod)); + else { + assert(err < 0); + + log_full_errno(err == ENODEV ? LOG_NOTICE : + err == ENOENT ? LOG_WARNING : + LOG_ERR, + err, + "Failed to insert module '%s': %m", + kmod_module_get_name(mod)); + if (!IN_SET(err, ENODEV, ENOENT)) + r = err; + } + } + } + + return r; +} diff --git a/src/basic/module-util.h b/src/shared/module-util.h similarity index 81% rename from src/basic/module-util.h rename to src/shared/module-util.h index 8fa121ed98..16cac90258 100644 --- a/src/basic/module-util.h +++ b/src/shared/module-util.h @@ -8,3 +8,5 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(struct kmod_ctx*, kmod_unref); DEFINE_TRIVIAL_CLEANUP_FUNC(struct kmod_module*, kmod_module_unref); DEFINE_TRIVIAL_CLEANUP_FUNC(struct kmod_list*, kmod_module_unref_list); + +int module_load_and_warn(struct kmod_ctx *ctx, const char *module); From c3ad978633cd46491ebe1065fd4884adb13a8f5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 6 Jul 2018 12:28:09 +0200 Subject: [PATCH 4/7] udev-builtin-kmod: use the generic module_load() function There should be no functional change. --- src/modules-load/modules-load.c | 4 ++-- src/shared/module-util.c | 24 ++++++++++++++++-------- src/shared/module-util.h | 2 +- src/udev/udev-builtin-kmod.c | 32 +------------------------------- 4 files changed, 20 insertions(+), 42 deletions(-) diff --git a/src/modules-load/modules-load.c b/src/modules-load/modules-load.c index b3a4e818b6..4b0b9f4c80 100644 --- a/src/modules-load/modules-load.c +++ b/src/modules-load/modules-load.c @@ -92,7 +92,7 @@ static int apply_file(struct kmod_ctx *ctx, const char *path, bool ignore_enoent if (strchr(COMMENTS "\n", *l)) continue; - k = module_load_and_warn(ctx, l); + k = module_load_and_warn(ctx, l, true); if (k < 0 && r == 0) r = k; } @@ -189,7 +189,7 @@ int main(int argc, char *argv[]) { char **fn, **i; STRV_FOREACH(i, arg_proc_cmdline_modules) { - k = module_load_and_warn(ctx, *i); + k = module_load_and_warn(ctx, *i, true); if (k < 0 && r == 0) r = k; } diff --git a/src/shared/module-util.c b/src/shared/module-util.c index 36f4f364c1..af6a9b01e7 100644 --- a/src/shared/module-util.c +++ b/src/shared/module-util.c @@ -4,20 +4,25 @@ #include "module-util.h" -int module_load_and_warn(struct kmod_ctx *ctx, const char *module) { +int module_load_and_warn(struct kmod_ctx *ctx, const char *module, bool verbose) { const int probe_flags = KMOD_PROBE_APPLY_BLACKLIST; struct kmod_list *itr; _cleanup_(kmod_module_unref_listp) struct kmod_list *modlist = NULL; int r = 0; + /* verbose==true means we should log at non-debug level if we + * fail to find or load the module. */ + log_debug("Loading module: %s", module); r = kmod_module_new_from_lookup(ctx, module, &modlist); if (r < 0) - return log_error_errno(r, "Failed to lookup module alias '%s': %m", module); + return log_full_errno(verbose ? LOG_ERR : LOG_DEBUG, r, + "Failed to lookup module alias '%s': %m", module); if (!modlist) { - log_error("Failed to find module '%s'", module); + log_full_errno(verbose ? LOG_ERR : LOG_DEBUG, r, + "Failed to find module '%s'", module); return -ENOENT; } @@ -30,7 +35,8 @@ int module_load_and_warn(struct kmod_ctx *ctx, const char *module) { switch (state) { case KMOD_MODULE_BUILTIN: - log_info("Module '%s' is builtin", kmod_module_get_name(mod)); + log_full(verbose ? LOG_INFO : LOG_DEBUG, + "Module '%s' is builtin", kmod_module_get_name(mod)); break; case KMOD_MODULE_LIVE: @@ -40,15 +46,17 @@ int module_load_and_warn(struct kmod_ctx *ctx, const char *module) { default: err = kmod_module_probe_insert_module(mod, probe_flags, NULL, NULL, NULL, NULL); - if (err == 0) - log_info("Inserted module '%s'", kmod_module_get_name(mod)); + log_full(verbose ? LOG_INFO : LOG_DEBUG, + "Inserted module '%s'", kmod_module_get_name(mod)); else if (err == KMOD_PROBE_APPLY_BLACKLIST) - log_info("Module '%s' is blacklisted", kmod_module_get_name(mod)); + log_full(verbose ? LOG_INFO : LOG_DEBUG, + "Module '%s' is blacklisted", kmod_module_get_name(mod)); else { assert(err < 0); - log_full_errno(err == ENODEV ? LOG_NOTICE : + log_full_errno(!verbose ? LOG_DEBUG : + err == ENODEV ? LOG_NOTICE : err == ENOENT ? LOG_WARNING : LOG_ERR, err, diff --git a/src/shared/module-util.h b/src/shared/module-util.h index 16cac90258..c386c5b459 100644 --- a/src/shared/module-util.h +++ b/src/shared/module-util.h @@ -9,4 +9,4 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(struct kmod_ctx*, kmod_unref); DEFINE_TRIVIAL_CLEANUP_FUNC(struct kmod_module*, kmod_module_unref); DEFINE_TRIVIAL_CLEANUP_FUNC(struct kmod_list*, kmod_module_unref_list); -int module_load_and_warn(struct kmod_ctx *ctx, const char *module); +int module_load_and_warn(struct kmod_ctx *ctx, const char *module, bool verbose); diff --git a/src/udev/udev-builtin-kmod.c b/src/udev/udev-builtin-kmod.c index e24e8e55e2..f5e09cebde 100644 --- a/src/udev/udev-builtin-kmod.c +++ b/src/udev/udev-builtin-kmod.c @@ -18,41 +18,11 @@ static struct kmod_ctx *ctx = NULL; -static int load_module(struct udev *udev, const char *alias) { - _cleanup_(kmod_module_unref_listp) struct kmod_list *list = NULL; - struct kmod_list *l; - int err; - - err = kmod_module_new_from_lookup(ctx, alias, &list); - if (err < 0) - return err; - - if (list == NULL) - log_debug("No module matches '%s'", alias); - - kmod_list_foreach(l, list) { - _cleanup_(kmod_module_unrefp) struct kmod_module *mod = NULL; - - mod = kmod_module_get_module(l); - - err = kmod_module_probe_insert_module(mod, KMOD_PROBE_APPLY_BLACKLIST, NULL, NULL, NULL, NULL); - if (err == KMOD_PROBE_APPLY_BLACKLIST) - log_debug("Module '%s' is blacklisted", kmod_module_get_name(mod)); - else if (err == 0) - log_debug("Inserted '%s'", kmod_module_get_name(mod)); - else - log_debug("Failed to insert '%s'", kmod_module_get_name(mod)); - } - - return err; -} - _printf_(6,0) static void udev_kmod_log(void *data, int priority, const char *file, int line, const char *fn, const char *format, va_list args) { log_internalv(priority, 0, file, line, fn, format, args); } static int builtin_kmod(struct udev_device *dev, int argc, char *argv[], bool test) { - struct udev *udev = udev_device_get_udev(dev); int i; if (!ctx) @@ -65,7 +35,7 @@ static int builtin_kmod(struct udev_device *dev, int argc, char *argv[], bool te for (i = 2; argv[i]; i++) { log_debug("Execute '%s' '%s'", argv[1], argv[i]); - load_module(udev, argv[i]); + (void) module_load_and_warn(ctx, argv[i], false); } return EXIT_SUCCESS; From 9b38ec87da26821db13a77cbed17d7b45485d044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 19 Jul 2018 18:27:22 +0200 Subject: [PATCH 5/7] shared/module-util: fix preexisting mixup with errno sign --- src/shared/module-util.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/shared/module-util.c b/src/shared/module-util.c index af6a9b01e7..a34fe8fb95 100644 --- a/src/shared/module-util.c +++ b/src/shared/module-util.c @@ -56,13 +56,13 @@ int module_load_and_warn(struct kmod_ctx *ctx, const char *module, bool verbose) assert(err < 0); log_full_errno(!verbose ? LOG_DEBUG : - err == ENODEV ? LOG_NOTICE : - err == ENOENT ? LOG_WARNING : - LOG_ERR, + err == -ENODEV ? LOG_NOTICE : + err == -ENOENT ? LOG_WARNING : + LOG_ERR, err, "Failed to insert module '%s': %m", kmod_module_get_name(mod)); - if (!IN_SET(err, ENODEV, ENOENT)) + if (!IN_SET(err, -ENODEV, -ENOENT)) r = err; } } From 81d7c696573672213a345a09de3199f392cd3162 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 6 Jul 2018 12:41:50 +0200 Subject: [PATCH 6/7] core: use the generic module_load() function This allows aliases to be used for the basic modules we load from pid1 before udev is started. In #9501 the kernel renamed autofs4 to autofs, with "autofs4" as alias, but we wouldn't load the module, because we didn't follow aliases. The kernel change was reverted, but it's probably better to support aliases. --- src/core/kmod-setup.c | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/src/core/kmod-setup.c b/src/core/kmod-setup.c index b8292e77fd..5feb9962ba 100644 --- a/src/core/kmod-setup.c +++ b/src/core/kmod-setup.c @@ -96,14 +96,11 @@ int kmod_setup(void) { }; _cleanup_(kmod_unrefp) struct kmod_ctx *ctx = NULL; unsigned int i; - int r; if (have_effective_cap(CAP_SYS_MODULE) == 0) return 0; for (i = 0; i < ELEMENTSOF(kmod_table); i++) { - _cleanup_(kmod_module_unrefp) struct kmod_module *mod = NULL; - if (kmod_table[i].path && access(kmod_table[i].path, F_OK) >= 0) continue; @@ -124,23 +121,7 @@ int kmod_setup(void) { kmod_load_resources(ctx); } - r = kmod_module_new_from_name(ctx, kmod_table[i].module, &mod); - if (r < 0) { - log_error("Failed to lookup module '%s'", kmod_table[i].module); - continue; - } - - r = kmod_module_probe_insert_module(mod, KMOD_PROBE_APPLY_BLACKLIST, NULL, NULL, NULL, NULL); - if (r == 0) - log_debug("Inserted module '%s'", kmod_module_get_name(mod)); - else if (r == KMOD_PROBE_APPLY_BLACKLIST) - log_info("Module '%s' is blacklisted", kmod_module_get_name(mod)); - else { - bool print_warning = kmod_table[i].warn_if_unavailable || (r < 0 && r != -ENOENT); - - log_full_errno(print_warning ? LOG_WARNING : LOG_DEBUG, r, - "Failed to insert module '%s': %m", kmod_module_get_name(mod)); - } + (void) module_load_and_warn(ctx, kmod_table[i].module, kmod_table[i].warn_if_unavailable); } #endif From 4fdf69078a777fdb34a36b0fd83a459192ea63dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 6 Jul 2018 12:47:39 +0200 Subject: [PATCH 7/7] udev-builtin-kmod: adjust logging I guess the one about "execute" was from the time when modprobe was called directly. --- src/udev/udev-builtin-kmod.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/udev/udev-builtin-kmod.c b/src/udev/udev-builtin-kmod.c index f5e09cebde..5e9fd0ce43 100644 --- a/src/udev/udev-builtin-kmod.c +++ b/src/udev/udev-builtin-kmod.c @@ -29,14 +29,12 @@ static int builtin_kmod(struct udev_device *dev, int argc, char *argv[], bool te return 0; if (argc < 3 || !streq(argv[1], "load")) { - log_error("expect: %s load ", argv[0]); + log_error("%s: expected: load ", argv[0]); return EXIT_FAILURE; } - for (i = 2; argv[i]; i++) { - log_debug("Execute '%s' '%s'", argv[1], argv[i]); + for (i = 2; argv[i]; i++) (void) module_load_and_warn(ctx, argv[i], false); - } return EXIT_SUCCESS; }