core: Serialize both pid and pidfd to keep downgrades working

Currently, when downgrading from a version with pidfd support to a
version without pidfd support, all information about running processes
is lost as the newer systemd will serialized pidfds which are not recognized
by the older systemd when deserializing.

To improve the situation, let's serialize both the pid and the pidfd.
This is safe because existing versions will either replace the first
deserialized pidref with the second one or discard the second one in
favor of the first one depending on the unit and field. Older versions
that don't support pidfd's will silently discard any fields that contain
a pidfd as those will try to parse the field as a pid and since a pidfd
field will start with '@', those versions will debug error log and ignore
the value.

To make sure we reuse the existing pidfd as much as possible, the pidfd
is serialized first. Both for scopes and service main pids, if the same
pid is seen multiple times, the first pidref is kept. So by serializing
the pidfd first we make sure the original pidfd is used instead of the
new one which is opened when deserializing the first pid field.

For other control units, older versions with pidfd support will discard
the first pidfd and replace it with a new pidfd from the second pid field.
This is a slight regression on downgrades, but we make sure it doesn't
happen for future versions (and older versions when this commit is
backported) by modifying the logic to only use the first successfully
deserialized pidref so that the raw pid without pidfd is discarded instead
of it replacing the existing pidfd.

diff --git a/src/core/mount.c b/src/core/mount.c
index b90b76d3ac05fd7b22d0ed43693a48bcdff43170..886ef8974cb00ee4f47b9dd1a394c5cf23ae033f 100644 (file)
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -1438,8 +1438,8 @@ static int mount_deserialize_item(Unit *u, const char *key, const char *value, F
 
         } else if (streq(key, "control-pid")) {
 
-                pidref_done(&m->control_pid);
-                (void) deserialize_pidref(fds, value, &m->control_pid);
+                if (!pidref_is_set(&m->control_pid))
+                        (void) deserialize_pidref(fds, value, &m->control_pid);
 
         } else if (streq(key, "control-command")) {
                 MountExecCommand id;

diff --git a/src/core/service.c b/src/core/service.c
index 0a3717e34e95a055bfb86938af2420297b4e0326..37025b4be279ff06fed4d2fa8be29486a1d115ab 100644 (file)
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -3188,9 +3188,9 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
                         s->reload_result = f;
 
         } else if (streq(key, "control-pid")) {
-                pidref_done(&s->control_pid);
 
-                (void) deserialize_pidref(fds, value, &s->control_pid);
+                if (!pidref_is_set(&s->control_pid))
+                        (void) deserialize_pidref(fds, value, &s->control_pid);
 
         } else if (streq(key, "main-pid")) {
                 _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL;

diff --git a/src/core/socket.c b/src/core/socket.c
index c9db6973f03485ed2cd098b6ad80a6a391b91bdf..36d458f35c01ef14317d321ffd5d3bf99d8006e0 100644 (file)
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -2631,8 +2631,9 @@ static int socket_deserialize_item(Unit *u, const char *key, const char *value,
                 else
                         s->n_refused += k;
         } else if (streq(key, "control-pid")) {
-                pidref_done(&s->control_pid);
-                (void) deserialize_pidref(fds, value, &s->control_pid);
+
+                if (!pidref_is_set(&s->control_pid))
+                        (void) deserialize_pidref(fds, value, &s->control_pid);
 
         } else if (streq(key, "control-command")) {
                 SocketExecCommand id;

diff --git a/src/core/swap.c b/src/core/swap.c
index 5601b832b2df0b202308fb842cff781c8dd6f9c5..820dfdff81c9999fca9df44702f0bee44107809d 100644 (file)
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -955,8 +955,8 @@ static int swap_deserialize_item(Unit *u, const char *key, const char *value, FD
                         s->result = f;
         } else if (streq(key, "control-pid")) {
 
-                pidref_done(&s->control_pid);
-                (void) deserialize_pidref(fds, value, &s->control_pid);
+                if (!pidref_is_set(&s->control_pid))
+                        (void) deserialize_pidref(fds, value, &s->control_pid);
 
         } else if (streq(key, "control-command")) {
                 SwapExecCommand id;

diff --git a/src/shared/serialize.c b/src/shared/serialize.c
index 483cbc7419b25e55364fd928de94226e3ae19040..d1f41ce4c821b2db2a1776ffb32133824bc24d2a 100644 (file)
--- a/src/shared/serialize.c
+++ b/src/shared/serialize.c
@@ -180,7 +180,7 @@ int serialize_strv(FILE *f, const char *key, char **l) {
 }
 
 int serialize_pidref(FILE *f, FDSet *fds, const char *key, PidRef *pidref) {
-        int copy;
+        int r;
 
         assert(f);
         assert(fds);
@@ -188,17 +188,21 @@ int serialize_pidref(FILE *f, FDSet *fds, const char *key, PidRef *pidref) {
         if (!pidref_is_set(pidref))
                 return 0;
 
-        /* If we have a pidfd we serialize the fd and encode the fd number prefixed by "@" in the
-         * serialization. Otherwise we serialize the numeric PID as it is. */
+        /* We always serialize the pid, to keep downgrades mostly working (older versions will deserialize
+         * the pid and silently fail to deserialize the pidfd). If we also have a pidfd, we serialize it
+         * first and encode the fd number prefixed by "@" in the serialization. */
 
-        if (pidref->fd < 0)
-                return serialize_item_format(f, key, PID_FMT, pidref->pid);
+        if (pidref->fd >= 0) {
+                int copy = fdset_put_dup(fds, pidref->fd);
+                if (copy < 0)
+                        return log_error_errno(copy, "Failed to add file descriptor to serialization set: %m");
 
-        copy = fdset_put_dup(fds, pidref->fd);
-        if (copy < 0)
-                return log_error_errno(copy, "Failed to add file descriptor to serialization set: %m");
+                r = serialize_item_format(f, key, "@%i", copy);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to serialize PID file descriptor: %m");
+        }
 
-        return serialize_item_format(f, key, "@%i", copy);
+        return serialize_item_format(f, key, PID_FMT, pidref->pid);
 }
 
 int serialize_ratelimit(FILE *f, const char *key, const RateLimit *rl) {