From 42670846427c3e00288c1e14afb305965234e51f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 15 Mar 2022 21:12:40 +0900 Subject: [PATCH] Revert "udev: do not kill "udevadm control" process in the same cgroup" This reverts commit ccadf9ac0d6d206767294b3f96f41eb42b48d1b0. The fix is not insufficient. See #22686. --- src/udev/udev-ctrl.c | 10 ---------- src/udev/udev-ctrl.h | 8 -------- src/udev/udevadm-control.c | 5 ----- src/udev/udevd.c | 38 +++----------------------------------- 4 files changed, 3 insertions(+), 58 deletions(-) diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c index 2b697c03b3..8adef47732 100644 --- a/src/udev/udev-ctrl.c +++ b/src/udev/udev-ctrl.c @@ -12,7 +12,6 @@ #include "alloc-util.h" #include "errno-util.h" -#include "event-util.h" #include "fd-util.h" #include "format-util.h" #include "io-util.h" @@ -106,13 +105,6 @@ 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); @@ -314,8 +306,6 @@ int udev_ctrl_send(UdevCtrl *uctrl, UdevCtrlMessageType type, const void *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) diff --git a/src/udev/udev-ctrl.h b/src/udev/udev-ctrl.h index 83ef44391d..11fc0b62de 100644 --- a/src/udev/udev-ctrl.h +++ b/src/udev/udev-ctrl.h @@ -4,7 +4,6 @@ #include "sd-event.h" #include "macro.h" -#include "process-util.h" #include "time-util.h" typedef struct UdevCtrl UdevCtrl; @@ -19,12 +18,10 @@ 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; @@ -42,7 +39,6 @@ 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); @@ -79,8 +75,4 @@ static inline int udev_ctrl_send_exit(UdevCtrl *uctrl) { 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 720e09a70f..06c61e5c07 100644 --- a/src/udev/udevadm-control.c +++ b/src/udev/udevadm-control.c @@ -87,11 +87,6 @@ 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 ccd30eea95..8380d674c5 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -108,8 +108,6 @@ typedef struct Manager { bool stop_exec_queue; bool exit; - - pid_t pid_udevadm; /* pid of 'udevadm control' */ } Manager; typedef enum EventState { @@ -1217,10 +1215,6 @@ 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"); } @@ -1496,35 +1490,9 @@ static int on_post(sd_event_source *s, void *userdata) { if (manager->exit) return sd_event_exit(manager->event, 0); - 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); + 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); return 1; } -- 2.25.1