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 8adef4773281f50f8e8e55d96a80502b7824e8df..2b697c03b346066ec42ab754b3e9843ccd448afa 100644 (file)
--- 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 11fc0b62de674d957855c01ffdcdd58eb8aab9a5..83ef44391d526f5221d00bdfb6d99c40e22216ff 100644 (file)
--- 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 06c61e5c07c68bdd9dcdd7bce0ed143e8919afc5..720e09a70f12323fc6e6349948bfc98d0f7330b8 100644 (file)
--- 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 8380d674c50d84afad7614a97c5d967ce9f1293f..ccd30eea95a1c767e22769340b0423d9a5c601e5 100644 (file)
--- 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;
 }