diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c index 179a1fdec3..2b697c03b3 100644 --- a/src/udev/udev-ctrl.c +++ b/src/udev/udev-ctrl.c @@ -12,6 +12,7 @@ #include "alloc-util.h" #include "errno-util.h" +#include "event-util.h" #include "fd-util.h" #include "format-util.h" #include "io-util.h" @@ -105,6 +106,13 @@ static void udev_ctrl_disconnect(UdevCtrl *uctrl) { uctrl->sock_connect = safe_close(uctrl->sock_connect); } +int udev_ctrl_is_connected(UdevCtrl *uctrl) { + if (!uctrl) + return false; + + return event_source_is_enabled(uctrl->event_source_connect); +} + static UdevCtrl *udev_ctrl_free(UdevCtrl *uctrl) { assert(uctrl); @@ -291,7 +299,7 @@ int udev_ctrl_start(UdevCtrl *uctrl, udev_ctrl_handler_t callback, void *userdat return 0; } -int udev_ctrl_send(UdevCtrl *uctrl, UdevCtrlMessageType type, int intval, const char *buf) { +int udev_ctrl_send(UdevCtrl *uctrl, UdevCtrlMessageType type, const void *data) { UdevCtrlMessageWire ctrl_msg_wire = { .version = "udev-" STRINGIFY(PROJECT_VERSION), .magic = UDEV_CTRL_MAGIC, @@ -301,10 +309,13 @@ int udev_ctrl_send(UdevCtrl *uctrl, UdevCtrlMessageType type, int intval, const if (uctrl->maybe_disconnected) return -ENOANO; /* to distinguish this from other errors. */ - if (buf) - strscpy(ctrl_msg_wire.value.buf, sizeof(ctrl_msg_wire.value.buf), buf); - else - ctrl_msg_wire.value.intval = intval; + if (type == UDEV_CTRL_SET_ENV) { + assert(data); + strscpy(ctrl_msg_wire.value.buf, sizeof(ctrl_msg_wire.value.buf), data); + } else if (IN_SET(type, UDEV_CTRL_SET_LOG_LEVEL, UDEV_CTRL_SET_CHILDREN_MAX)) + ctrl_msg_wire.value.intval = PTR_TO_INT(data); + else if (type == UDEV_CTRL_SENDER_PID) + ctrl_msg_wire.value.pid = PTR_TO_PID(data); if (!uctrl->connected) { if (connect(uctrl->sock, &uctrl->saddr.sa, uctrl->addrlen) < 0) @@ -333,7 +344,7 @@ int udev_ctrl_wait(UdevCtrl *uctrl, usec_t timeout) { return 0; if (!uctrl->maybe_disconnected) { - r = udev_ctrl_send(uctrl, _UDEV_CTRL_END_MESSAGES, 0, NULL); + r = udev_ctrl_send(uctrl, _UDEV_CTRL_END_MESSAGES, NULL); if (r < 0) return r; } diff --git a/src/udev/udev-ctrl.h b/src/udev/udev-ctrl.h index d4bc068127..83ef44391d 100644 --- a/src/udev/udev-ctrl.h +++ b/src/udev/udev-ctrl.h @@ -4,6 +4,7 @@ #include "sd-event.h" #include "macro.h" +#include "process-util.h" #include "time-util.h" typedef struct UdevCtrl UdevCtrl; @@ -18,10 +19,12 @@ typedef enum UdevCtrlMessageType { UDEV_CTRL_SET_CHILDREN_MAX, UDEV_CTRL_PING, UDEV_CTRL_EXIT, + UDEV_CTRL_SENDER_PID, } UdevCtrlMessageType; typedef union UdevCtrlMessageValue { int intval; + pid_t pid; char buf[256]; } UdevCtrlMessageValue; @@ -39,40 +42,45 @@ UdevCtrl *udev_ctrl_unref(UdevCtrl *uctrl); int udev_ctrl_attach_event(UdevCtrl *uctrl, sd_event *event); int udev_ctrl_start(UdevCtrl *uctrl, udev_ctrl_handler_t callback, void *userdata); sd_event_source *udev_ctrl_get_event_source(UdevCtrl *uctrl); +int udev_ctrl_is_connected(UdevCtrl *uctrl); int udev_ctrl_wait(UdevCtrl *uctrl, usec_t timeout); -int udev_ctrl_send(UdevCtrl *uctrl, UdevCtrlMessageType type, int intval, const char *buf); +int udev_ctrl_send(UdevCtrl *uctrl, UdevCtrlMessageType type, const void *data); static inline int udev_ctrl_send_set_log_level(UdevCtrl *uctrl, int priority) { - return udev_ctrl_send(uctrl, UDEV_CTRL_SET_LOG_LEVEL, priority, NULL); + return udev_ctrl_send(uctrl, UDEV_CTRL_SET_LOG_LEVEL, INT_TO_PTR(priority)); } static inline int udev_ctrl_send_stop_exec_queue(UdevCtrl *uctrl) { - return udev_ctrl_send(uctrl, UDEV_CTRL_STOP_EXEC_QUEUE, 0, NULL); + return udev_ctrl_send(uctrl, UDEV_CTRL_STOP_EXEC_QUEUE, NULL); } static inline int udev_ctrl_send_start_exec_queue(UdevCtrl *uctrl) { - return udev_ctrl_send(uctrl, UDEV_CTRL_START_EXEC_QUEUE, 0, NULL); + return udev_ctrl_send(uctrl, UDEV_CTRL_START_EXEC_QUEUE, NULL); } static inline int udev_ctrl_send_reload(UdevCtrl *uctrl) { - return udev_ctrl_send(uctrl, UDEV_CTRL_RELOAD, 0, NULL); + return udev_ctrl_send(uctrl, UDEV_CTRL_RELOAD, NULL); } static inline int udev_ctrl_send_set_env(UdevCtrl *uctrl, const char *key) { - return udev_ctrl_send(uctrl, UDEV_CTRL_SET_ENV, 0, key); + return udev_ctrl_send(uctrl, UDEV_CTRL_SET_ENV, key); } static inline int udev_ctrl_send_set_children_max(UdevCtrl *uctrl, int count) { - return udev_ctrl_send(uctrl, UDEV_CTRL_SET_CHILDREN_MAX, count, NULL); + return udev_ctrl_send(uctrl, UDEV_CTRL_SET_CHILDREN_MAX, INT_TO_PTR(count)); } static inline int udev_ctrl_send_ping(UdevCtrl *uctrl) { - return udev_ctrl_send(uctrl, UDEV_CTRL_PING, 0, NULL); + return udev_ctrl_send(uctrl, UDEV_CTRL_PING, NULL); } static inline int udev_ctrl_send_exit(UdevCtrl *uctrl) { - return udev_ctrl_send(uctrl, UDEV_CTRL_EXIT, 0, NULL); + return udev_ctrl_send(uctrl, UDEV_CTRL_EXIT, NULL); +} + +static inline int udev_ctrl_send_pid(UdevCtrl *uctrl) { + return udev_ctrl_send(uctrl, UDEV_CTRL_SENDER_PID, PID_TO_PTR(getpid_cached())); } DEFINE_TRIVIAL_CLEANUP_FUNC(UdevCtrl*, udev_ctrl_unref); diff --git a/src/udev/udevadm-control.c b/src/udev/udevadm-control.c index 06c61e5c07..720e09a70f 100644 --- a/src/udev/udevadm-control.c +++ b/src/udev/udevadm-control.c @@ -87,6 +87,11 @@ int control_main(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to initialize udev control: %m"); + /* See comments in on_post() in udevd.c. */ + r = udev_ctrl_send_pid(uctrl); + if (r < 0) + return log_error_errno(r, "Failed to send pid of this process: %m"); + while ((c = getopt_long(argc, argv, "el:sSRp:m:t:Vh", options, NULL)) >= 0) switch (c) { case 'e': diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 8380d674c5..ccd30eea95 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -108,6 +108,8 @@ typedef struct Manager { bool stop_exec_queue; bool exit; + + pid_t pid_udevadm; /* pid of 'udevadm control' */ } Manager; typedef enum EventState { @@ -1215,6 +1217,10 @@ static int on_ctrl_msg(UdevCtrl *uctrl, UdevCtrlMessageType type, const UdevCtrl log_debug("Received udev control message (EXIT)"); manager_exit(manager); break; + case UDEV_CTRL_SENDER_PID: + log_debug("Received udev control message (SENDER_PID)"); + manager->pid_udevadm = value->pid; + break; default: log_debug("Received unknown udev control message, ignoring"); } @@ -1490,9 +1496,35 @@ static int on_post(sd_event_source *s, void *userdata) { if (manager->exit) return sd_event_exit(manager->event, 0); - if (manager->cgroup) - /* cleanup possible left-over processes in our cgroup */ - (void) cg_kill(SYSTEMD_CGROUP_CONTROLLER, manager->cgroup, SIGKILL, CGROUP_IGNORE_SELF, NULL, NULL, NULL); + if (!manager->cgroup) + return 1; + + /* Cleanup possible left-over processes in our cgroup. But do not kill udevadm called by + * ExecReload= in systemd-udevd.service. See #16867. To protect it, the following two ways are + * introduced: + * + * 1. After the connection is accepted, but the PID of udevadm is not received, do not call + * cg_kill(). So, in this period, unwanted process or threads may exist in our cgroup. + * But, it is expected that the PID of the udevadm will be received soon. So, this time + * period should be short enough. + * 2. After the PID of udevadm is received, check the process is active or not, and if it is + * still active, set the PID to the deny list for cg_kill(). Why udev_ctrl_is_connected() is + * not enough? Because the udevadm may be still active after the control socket is + * disconnected. If the process is not active, then clear the PID for later connections. + */ + + if (udev_ctrl_is_connected(manager->ctrl) >= 0 && !pid_is_valid(manager->pid_udevadm)) + return 1; + + _cleanup_set_free_ Set *pids = NULL; + if (pid_is_valid(manager->pid_udevadm)) { + if (pid_is_alive(manager->pid_udevadm)) + (void) set_ensure_put(&pids, NULL, PID_TO_PTR(manager->pid_udevadm)); + else + manager->pid_udevadm = 0; + } + + (void) cg_kill(SYSTEMD_CGROUP_CONTROLLER, manager->cgroup, SIGKILL, CGROUP_IGNORE_SELF, pids, NULL, NULL); return 1; }