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.

Author: DaanDeMeyer

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;

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;

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;

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;

src/shared/serialize.c

@@ -180,25 +180,29 @@ 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);
 
         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) {