From: Yu Watanabe Date: Mon, 13 Dec 2021 20:30:58 +0000 (+0900) Subject: udev: do not kill "udevadm control" process in the same cgroup X-Git-Tag: v251-rc1~650^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ccadf9ac0d6d206767294b3f96f41eb42b48d1b0;p=thirdparty%2Fsystemd.git udev: do not kill "udevadm control" process in the same cgroup Fixes #16867. --- diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c index 8adef477328..2b697c03b34 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); @@ -306,6 +314,8 @@ 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 11fc0b62de6..83ef44391d5 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,6 +42,7 @@ 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); @@ -75,4 +79,8 @@ 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 06c61e5c07c..720e09a70f1 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 8380d674c50..ccd30eea95a 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; }