From a2840b9599f2c764600c3168017918c2cf213ead Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 4 Dec 2024 05:02:53 +0900 Subject: [PATCH 1/2] udev: move listen_fds() to udev-manager.c Also - drop redundant error message when manager_init() failed, - close unexpected fds. No functional change, just refactoring. --- src/udev/udev-manager.c | 57 ++++++++++++++++++++++++++++++++++++++++- src/udev/udev-manager.h | 2 +- src/udev/udevd.c | 45 ++------------------------------ 3 files changed, 59 insertions(+), 45 deletions(-) diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index 32bc0db345..5a8f1613e1 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -1227,15 +1227,69 @@ void manager_adjust_arguments(Manager *manager) { } } -int manager_init(Manager *manager, int fd_ctrl, int fd_uevent) { +static int listen_fds(int *ret_ctrl, int *ret_netlink) { + _cleanup_strv_free_ char **names = NULL; + _cleanup_close_ int ctrl_fd = -EBADF, netlink_fd = -EBADF; + + assert(ret_ctrl); + assert(ret_netlink); + + int n = sd_listen_fds_with_names(/* unset_environment = */ true, &names); + if (n < 0) + return n; + + if (strv_length(names) != (size_t) n) + return -EIO; + + for (int i = 0; i < n; i++) { + int fd = SD_LISTEN_FDS_START + i; + + if (sd_is_socket(fd, AF_UNIX, SOCK_SEQPACKET, -1) > 0) { + if (ctrl_fd >= 0) { + log_debug("Received multiple seqpacket socket (%s), ignoring.", names[i]); + goto unused; + } + + ctrl_fd = fd; + continue; + } + + if (sd_is_socket(fd, AF_NETLINK, SOCK_RAW, -1) > 0) { + if (netlink_fd >= 0) { + log_debug("Received multiple netlink socket (%s), ignoring.", names[i]); + goto unused; + } + + netlink_fd = fd; + continue; + } + + log_debug("Received unexpected fd (%s), ignoring.", names[i]); + + unused: + close_and_notify_warn(fd, names[i]); + } + + *ret_ctrl = TAKE_FD(ctrl_fd); + *ret_netlink = TAKE_FD(netlink_fd); + return 0; +} + +int manager_init(Manager *manager) { + _cleanup_close_ int fd_ctrl = -EBADF, fd_uevent = -EBADF; _cleanup_free_ char *cgroup = NULL; int r; assert(manager); + r = listen_fds(&fd_ctrl, &fd_uevent); + if (r < 0) + return log_error_errno(r, "Failed to listen on fds: %m"); + r = udev_ctrl_new_from_fd(&manager->ctrl, fd_ctrl); if (r < 0) return log_error_errno(r, "Failed to initialize udev control socket: %m"); + TAKE_FD(fd_ctrl); r = udev_ctrl_enable_receiving(manager->ctrl); if (r < 0) @@ -1244,6 +1298,7 @@ int manager_init(Manager *manager, int fd_ctrl, int fd_uevent) { r = device_monitor_new_full(&manager->monitor, MONITOR_GROUP_KERNEL, fd_uevent); if (r < 0) return log_error_errno(r, "Failed to initialize device monitor: %m"); + TAKE_FD(fd_uevent); (void) sd_device_monitor_set_description(manager->monitor, "manager"); diff --git a/src/udev/udev-manager.h b/src/udev/udev-manager.h index 5921d333d1..9e96e3ff28 100644 --- a/src/udev/udev-manager.h +++ b/src/udev/udev-manager.h @@ -53,7 +53,7 @@ Manager* manager_free(Manager *manager); DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free); void manager_adjust_arguments(Manager *manager); -int manager_init(Manager *manager, int fd_ctrl, int fd_uevent); +int manager_init(Manager *manager); int manager_main(Manager *manager); bool devpath_conflict(const char *a, const char *b); diff --git a/src/udev/udevd.c b/src/udev/udevd.c index c46fcaa038..c5b5f652a7 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -8,8 +8,6 @@ #include #include -#include "sd-daemon.h" - #include "conf-parser.h" #include "env-file.h" #include "errno-util.h" @@ -31,40 +29,6 @@ static bool arg_debug = false; static int arg_daemonize = false; -static int listen_fds(int *ret_ctrl, int *ret_netlink) { - int ctrl_fd = -EBADF, netlink_fd = -EBADF; - - assert(ret_ctrl); - assert(ret_netlink); - - int n = sd_listen_fds(true); - if (n < 0) - return n; - - for (int fd = SD_LISTEN_FDS_START; fd < n + SD_LISTEN_FDS_START; fd++) { - if (sd_is_socket(fd, AF_UNIX, SOCK_SEQPACKET, -1) > 0) { - if (ctrl_fd >= 0) - return -EINVAL; - ctrl_fd = fd; - continue; - } - - if (sd_is_socket(fd, AF_NETLINK, SOCK_RAW, -1) > 0) { - if (netlink_fd >= 0) - return -EINVAL; - netlink_fd = fd; - continue; - } - - return -EINVAL; - } - - *ret_ctrl = ctrl_fd; - *ret_netlink = netlink_fd; - - return 0; -} - static DEFINE_CONFIG_PARSE_ENUM(config_parse_resolve_name_timing, resolve_name_timing, ResolveNameTiming); static int manager_parse_udev_config(Manager *manager) { @@ -286,7 +250,6 @@ static int parse_argv(int argc, char *argv[], Manager *manager) { int run_udevd(int argc, char *argv[]) { _cleanup_(manager_freep) Manager *manager = NULL; - int fd_ctrl = -EBADF, fd_uevent = -EBADF; int r; log_setup(); @@ -330,13 +293,9 @@ int run_udevd(int argc, char *argv[]) { if (r < 0 && r != -EEXIST) return log_error_errno(r, "Failed to create /run/udev: %m"); - r = listen_fds(&fd_ctrl, &fd_uevent); + r = manager_init(manager); if (r < 0) - return log_error_errno(r, "Failed to listen on fds: %m"); - - r = manager_init(manager, fd_ctrl, fd_uevent); - if (r < 0) - return log_error_errno(r, "Failed to create manager: %m"); + return r; if (arg_daemonize) { pid_t pid; From 0bdcca5aee60422167065be156990f2a88e0ed95 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 4 Dec 2024 06:05:38 +0900 Subject: [PATCH 2/2] udev: move enums to udev-def.h No functional change, just refactoring and preparation for later commits. --- src/udev/test-udev-rule-runner.c | 1 + src/udev/udev-builtin.h | 24 +------------- src/udev/udev-def.h | 57 ++++++++++++++++++++++++++++++++ src/udev/udev-event.c | 2 ++ src/udev/udev-event.h | 12 ++----- src/udev/udev-format.c | 1 + src/udev/udev-manager.c | 1 + src/udev/udev-manager.h | 3 +- src/udev/udev-rules.c | 1 + src/udev/udev-rules.h | 21 +----------- src/udev/udev-spawn.c | 1 + src/udev/udev-worker.c | 1 + src/udev/udevadm-test.c | 1 + src/udev/udevd.c | 1 + 14 files changed, 74 insertions(+), 53 deletions(-) create mode 100644 src/udev/udev-def.h diff --git a/src/udev/test-udev-rule-runner.c b/src/udev/test-udev-rule-runner.c index db41813511..d123c8ad1b 100644 --- a/src/udev/test-udev-rule-runner.c +++ b/src/udev/test-udev-rule-runner.c @@ -25,6 +25,7 @@ #include "string-util.h" #include "tests.h" #include "udev-event.h" +#include "udev-rules.h" #include "udev-spawn.h" #include "version.h" diff --git a/src/udev/udev-builtin.h b/src/udev/udev-builtin.h index b4ddba095a..2433dfa4d4 100644 --- a/src/udev/udev-builtin.h +++ b/src/udev/udev-builtin.h @@ -7,31 +7,9 @@ #include "sd-netlink.h" #include "macro.h" +#include "udev-def.h" #include "udev-event.h" -typedef enum UdevBuiltinCommand { -#if HAVE_BLKID - UDEV_BUILTIN_BLKID, -#endif - UDEV_BUILTIN_BTRFS, - UDEV_BUILTIN_HWDB, - UDEV_BUILTIN_INPUT_ID, - UDEV_BUILTIN_KEYBOARD, -#if HAVE_KMOD - UDEV_BUILTIN_KMOD, -#endif - UDEV_BUILTIN_NET_DRIVER, - UDEV_BUILTIN_NET_ID, - UDEV_BUILTIN_NET_LINK, - UDEV_BUILTIN_PATH_ID, - UDEV_BUILTIN_USB_ID, -#if HAVE_ACL - UDEV_BUILTIN_UACCESS, -#endif - _UDEV_BUILTIN_MAX, - _UDEV_BUILTIN_INVALID = -EINVAL, -} UdevBuiltinCommand; - typedef struct UdevBuiltin { const char *name; int (*cmd)(UdevEvent *event, int argc, char *argv[]); diff --git a/src/udev/udev-def.h b/src/udev/udev-def.h new file mode 100644 index 0000000000..6ff3feacec --- /dev/null +++ b/src/udev/udev-def.h @@ -0,0 +1,57 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +#pragma once + +#include + +#define UDEV_NAME_SIZE 512 +#define UDEV_PATH_SIZE 1024 +#define UDEV_LINE_SIZE 16384 + +typedef enum EventMode { + EVENT_UDEV_WORKER, + EVENT_UDEVADM_TEST, + EVENT_UDEVADM_TEST_BUILTIN, + EVENT_TEST_RULE_RUNNER, + EVENT_TEST_SPAWN, + _EVENT_MODE_MAX, + _EVENT_MODE_INVALID = -EINVAL, +} EventMode; + +typedef enum UdevRuleEscapeType { + ESCAPE_UNSET, + ESCAPE_NONE, /* OPTIONS="string_escape=none" */ + ESCAPE_REPLACE, /* OPTIONS="string_escape=replace" */ + _ESCAPE_TYPE_MAX, + _ESCAPE_TYPE_INVALID = -EINVAL, +} UdevRuleEscapeType; + +typedef enum ResolveNameTiming { + RESOLVE_NAME_NEVER, + RESOLVE_NAME_LATE, + RESOLVE_NAME_EARLY, + _RESOLVE_NAME_TIMING_MAX, + _RESOLVE_NAME_TIMING_INVALID = -EINVAL, +} ResolveNameTiming; + +typedef enum UdevBuiltinCommand { +#if HAVE_BLKID + UDEV_BUILTIN_BLKID, +#endif + UDEV_BUILTIN_BTRFS, + UDEV_BUILTIN_HWDB, + UDEV_BUILTIN_INPUT_ID, + UDEV_BUILTIN_KEYBOARD, +#if HAVE_KMOD + UDEV_BUILTIN_KMOD, +#endif + UDEV_BUILTIN_NET_DRIVER, + UDEV_BUILTIN_NET_ID, + UDEV_BUILTIN_NET_LINK, + UDEV_BUILTIN_PATH_ID, + UDEV_BUILTIN_USB_ID, +#if HAVE_ACL + UDEV_BUILTIN_UACCESS, +#endif + _UDEV_BUILTIN_MAX, + _UDEV_BUILTIN_INVALID = -EINVAL, +} UdevBuiltinCommand; diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 2b4e5b2301..9141a9d2a7 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -12,8 +12,10 @@ #include "strv.h" #include "udev-event.h" #include "udev-node.h" +#include "udev-rules.h" #include "udev-trace.h" #include "udev-util.h" +#include "udev-worker.h" #include "user-util.h" UdevEvent* udev_event_new(sd_device *dev, UdevWorker *worker, EventMode mode) { diff --git a/src/udev/udev-event.h b/src/udev/udev-event.h index cc3b59415e..873e7ca421 100644 --- a/src/udev/udev-event.h +++ b/src/udev/udev-event.h @@ -14,17 +14,11 @@ #include "hashmap.h" #include "macro.h" #include "time-util.h" -#include "udev-rules.h" -#include "udev-worker.h" +#include "udev-def.h" #include "user-util.h" -typedef enum EventMode { - EVENT_UDEV_WORKER, - EVENT_UDEVADM_TEST, - EVENT_UDEVADM_TEST_BUILTIN, - EVENT_TEST_RULE_RUNNER, - EVENT_TEST_SPAWN, -} EventMode; +typedef struct UdevRules UdevRules; +typedef struct UdevWorker UdevWorker; typedef struct UdevEvent { UdevWorker *worker; diff --git a/src/udev/udev-format.c b/src/udev/udev-format.c index 05842fdc1c..c09ea44d91 100644 --- a/src/udev/udev-format.c +++ b/src/udev/udev-format.c @@ -8,6 +8,7 @@ #include "udev-event.h" #include "udev-format.h" #include "udev-util.h" +#include "udev-worker.h" typedef enum { FORMAT_SUBST_DEVNODE, diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index 5a8f1613e1..6e1935a731 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -29,6 +29,7 @@ #include "udev-event.h" #include "udev-manager.h" #include "udev-node.h" +#include "udev-rules.h" #include "udev-spawn.h" #include "udev-trace.h" #include "udev-util.h" diff --git a/src/udev/udev-manager.h b/src/udev/udev-manager.h index 9e96e3ff28..7c20e29594 100644 --- a/src/udev/udev-manager.h +++ b/src/udev/udev-manager.h @@ -10,9 +10,10 @@ #include "macro.h" #include "time-util.h" #include "udev-ctrl.h" -#include "udev-rules.h" +#include "udev-def.h" typedef struct Event Event; +typedef struct UdevRules UdevRules; typedef struct Worker Worker; typedef struct Manager { diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 2555f3723a..8b1f476bc0 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -39,6 +39,7 @@ #include "udev-spawn.h" #include "udev-trace.h" #include "udev-util.h" +#include "udev-worker.h" #include "user-util.h" #include "virt.h" diff --git a/src/udev/udev-rules.h b/src/udev/udev-rules.h index 57447de8c5..67d7e5b178 100644 --- a/src/udev/udev-rules.h +++ b/src/udev/udev-rules.h @@ -4,31 +4,12 @@ #include "alloc-util.h" #include "hashmap.h" #include "time-util.h" - -#define UDEV_NAME_SIZE 512 -#define UDEV_PATH_SIZE 1024 -#define UDEV_LINE_SIZE 16384 +#include "udev-def.h" typedef struct UdevRuleFile UdevRuleFile; typedef struct UdevRules UdevRules; typedef struct UdevEvent UdevEvent; -typedef enum { - ESCAPE_UNSET, - ESCAPE_NONE, /* OPTIONS="string_escape=none" */ - ESCAPE_REPLACE, /* OPTIONS="string_escape=replace" */ - _ESCAPE_TYPE_MAX, - _ESCAPE_TYPE_INVALID = -EINVAL, -} UdevRuleEscapeType; - -typedef enum ResolveNameTiming { - RESOLVE_NAME_NEVER, - RESOLVE_NAME_LATE, - RESOLVE_NAME_EARLY, - _RESOLVE_NAME_TIMING_MAX, - _RESOLVE_NAME_TIMING_INVALID = -EINVAL, -} ResolveNameTiming; - int udev_rule_parse_value(char *str, char **ret_value, char **ret_endpos, bool *ret_is_case_insensitive); int udev_rules_parse_file(UdevRules *rules, const char *filename, bool extra_checks, UdevRuleFile **ret); unsigned udev_rule_file_get_issues(UdevRuleFile *rule_file); diff --git a/src/udev/udev-spawn.c b/src/udev/udev-spawn.c index d2a422f7e8..b95141cf21 100644 --- a/src/udev/udev-spawn.c +++ b/src/udev/udev-spawn.c @@ -14,6 +14,7 @@ #include "udev-event.h" #include "udev-spawn.h" #include "udev-trace.h" +#include "udev-worker.h" typedef struct Spawn { sd_device *device; diff --git a/src/udev/udev-worker.c b/src/udev/udev-worker.c index 59f56f653c..28cf26f1ea 100644 --- a/src/udev/udev-worker.c +++ b/src/udev/udev-worker.c @@ -18,6 +18,7 @@ #include "signal-util.h" #include "string-util.h" #include "udev-event.h" +#include "udev-rules.h" #include "udev-spawn.h" #include "udev-trace.h" #include "udev-util.h" diff --git a/src/udev/udevadm-test.c b/src/udev/udevadm-test.c index 748bcaf364..48714de33b 100644 --- a/src/udev/udevadm-test.c +++ b/src/udev/udevadm-test.c @@ -26,6 +26,7 @@ #include "udev-builtin.h" #include "udev-event.h" #include "udev-format.h" +#include "udev-rules.h" #include "udevadm-util.h" #include "udevadm.h" #include "user-util.h" diff --git a/src/udev/udevd.c b/src/udev/udevd.c index c5b5f652a7..ef1c07a2ca 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -22,6 +22,7 @@ #include "signal-util.h" #include "syslog-util.h" #include "udev-manager.h" +#include "udev-rules.h" #include "udev-util.h" #include "udevd.h" #include "version.h"