serialize: add new helper deserialize_fd()

Currently, when we deserialize an fd we do a lot of manual work. Add a
common helper that makes this more robust and uniform.

Note that this sometimes changes behaviour slightly, but in ways that
shouldn't really matter: if we fail to deserialize an fd correctly we'll
unset (i.e. set to -EBADF) the fd in the deserialized data structure.
Previously, we'd leave the old value in place.

This should not change effective result (as in either case we'll be in a
bad state afterwards, just once we mix old/invalidated state with new
state, while now we'll reset the state explicitly to invalidated state
on failure). In particular as deserialization starts from an empty
structure generally, hence the old value should be unset anyway.

Another slight change is that if we fail to deserialize some object half
way, and we already have taken out one fd from the serialized fdset
we'll now just close it instead of returning it to/leaving it in the
fdset. Given that such "orphaned" fds are blanket closed after
deserialization finishes this also shouldn't change behaviour IRL.

Also, the idle_pipe was previously incorrectly serialized: we'd
serialize invalidated fds, which would fail, but because parsing errors
on this were ignored on the deserializatin noone noticed. This is fixed.

diff --git a/src/core/automount.c b/src/core/automount.c
index a34bfae3ac819e1a5804d46c411730aa4b290de7..8df715b1afde60787a90c92f8c234e99043af8a2 100644 (file)
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -925,14 +925,8 @@ static int automount_deserialize_item(Unit *u, const char *key, const char *valu
                                 log_unit_error_errno(u, r, "Failed to add expire token to set: %m");
                 }
         } else if (streq(key, "pipe-fd")) {
-                int fd;
-
-                if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd))
-                        log_unit_debug(u, "Failed to parse pipe-fd value: %s", value);
-                else {
-                        safe_close(a->pipe_fd);
-                        a->pipe_fd = fdset_remove(fds, fd);
-                }
+                safe_close(a->pipe_fd);
+                a->pipe_fd = deserialize_fd(fds, value);
         } else
                 log_unit_debug(u, "Unknown serialization key: %s", key);
 

diff --git a/src/core/dynamic-user.c b/src/core/dynamic-user.c
index dc349afecfac0d429d9fd25a870c27d208213bfa..a70f64f7ff61d02dcf9de4d5d7e053c8ef6417bc 100644 (file)
--- a/src/core/dynamic-user.c
+++ b/src/core/dynamic-user.c
@@ -648,7 +648,8 @@ int dynamic_user_serialize(Manager *m, FILE *f, FDSet *fds) {
 
 void dynamic_user_deserialize_one(Manager *m, const char *value, FDSet *fds, DynamicUser **ret) {
         _cleanup_free_ char *name = NULL, *s0 = NULL, *s1 = NULL;
-        int r, fd0, fd1;
+        _cleanup_close_ int fd0 = -EBADF, fd1 = -EBADF;
+        int r;
 
         assert(value);
         assert(fds);
@@ -661,15 +662,13 @@ void dynamic_user_deserialize_one(Manager *m, const char *value, FDSet *fds, Dyn
                 return;
         }
 
-        if ((fd0 = parse_fd(s0)) < 0 || !fdset_contains(fds, fd0)) {
-                log_debug("Unable to process dynamic user fd specification.");
+        fd0 = deserialize_fd(fds, s0);
+        if (fd0 < 0)
                 return;
-        }
 
-        if ((fd1 = parse_fd(s1)) < 0 || !fdset_contains(fds, fd1)) {
-                log_debug("Unable to process dynamic user fd specification.");
+        fd1 = deserialize_fd(fds, s1);
+        if (fd1 < 0)
                 return;
-        }
 
         r = dynamic_user_add(m, name, (int[]) { fd0, fd1 }, ret);
         if (r < 0) {
@@ -677,8 +676,8 @@ void dynamic_user_deserialize_one(Manager *m, const char *value, FDSet *fds, Dyn
                 return;
         }
 
-        (void) fdset_remove(fds, fd0);
-        (void) fdset_remove(fds, fd1);
+        TAKE_FD(fd0);
+        TAKE_FD(fd1);
 
         if (ret) /* If the caller uses it directly, increment the refcount */
                 (*ret)->n_ref++;

diff --git a/src/core/execute-serialize.c b/src/core/execute-serialize.c
index 147bbd60edd118446e816fd1e33ac454a5324a9f..1886a59b84cdf6b2d2e3459ff4da8e3f90e4e6f6 100644 (file)
--- a/src/core/execute-serialize.c
+++ b/src/core/execute-serialize.c
@@ -1232,17 +1232,11 @@ static int exec_runtime_deserialize(ExecRuntime *rt, FILE *f, FDSet *fds) {
                                 if (r == 0)
                                         break;
 
-                                if ((fd = parse_fd(w)) < 0 || !fdset_contains(fds, fd))
-                                        log_debug("Failed to parse %s value: %s, ignoring.", l, w);
-                                else {
-                                        r = fdset_remove(fds, fd);
-                                        if (r < 0) {
-                                                log_debug_errno(r, "Failed to remove %s value=%d from fdset, ignoring: %m", l, fd);
-                                                continue;
-                                        }
+                                fd = deserialize_fd(fds, w);
+                                if (fd < 0)
+                                        continue;
 
-                                        rt->shared->netns_storage_socket[i] = fd;
-                                }
+                                rt->shared->netns_storage_socket[i] = fd;
                         }
                 } else if ((val = startswith(l, "exec-runtime-ipcns-storage-socket="))) {
                         for (size_t i = 0; i < 2; ++i) {
@@ -1255,17 +1249,11 @@ static int exec_runtime_deserialize(ExecRuntime *rt, FILE *f, FDSet *fds) {
                                 if (r == 0)
                                         break;
 
-                                if ((fd = parse_fd(w)) < 0 || !fdset_contains(fds, fd))
-                                        log_debug("Failed to parse %s value: %s, ignoring.", l, w);
-                                else {
-                                        r = fdset_remove(fds, fd);
-                                        if (r < 0) {
-                                                log_debug_errno(r, "Failed to remove %s value=%d from fdset, ignoring: %m", l, fd);
-                                                continue;
-                                        }
+                                fd = deserialize_fd(fds, w);
+                                if (fd < 0)
+                                        continue;
 
-                                        rt->shared->ipcns_storage_socket[i] = fd;
-                                }
+                                rt->shared->ipcns_storage_socket[i] = fd;
                         }
                 } else if ((val = startswith(l, "exec-runtime-dynamic-creds-user=")))
                         dynamic_user_deserialize_one(/* m= */ NULL, val, fds, &rt->dynamic_creds->user);
@@ -1297,17 +1285,11 @@ static int exec_runtime_deserialize(ExecRuntime *rt, FILE *f, FDSet *fds) {
                                 if (r == 0)
                                         break;
 
-                                if ((fd = parse_fd(w)) < 0 || !fdset_contains(fds, fd))
-                                        log_debug("Failed to parse %s value: %s, ignoring.", l, w);
-                                else {
-                                        r = fdset_remove(fds, fd);
-                                        if (r < 0) {
-                                                log_debug_errno(r, "Failed to remove %s value=%d from fdset, ignoring: %m", l, fd);
-                                                continue;
-                                        }
+                                fd = deserialize_fd(fds, w);
+                                if (fd < 0)
+                                        continue;
 
-                                        rt->ephemeral_storage_socket[i] = fd;
-                                }
+                                rt->ephemeral_storage_socket[i] = fd;
                         }
                 } else
                         log_warning("Failed to parse serialized line, ignoring: %s", l);
@@ -1316,6 +1298,16 @@ static int exec_runtime_deserialize(ExecRuntime *rt, FILE *f, FDSet *fds) {
         return 0;
 }
 
+static bool exec_parameters_is_idle_pipe_set(const ExecParameters *p) {
+        assert(p);
+
+        return p->idle_pipe &&
+                p->idle_pipe[0] >= 0 &&
+                p->idle_pipe[1] >= 0 &&
+                p->idle_pipe[2] >= 0 &&
+                p->idle_pipe[3] >= 0;
+}
+
 static int exec_parameters_serialize(const ExecParameters *p, FILE *f, FDSet *fds) {
         int r;
 
@@ -1434,17 +1426,13 @@ static int exec_parameters_serialize(const ExecParameters *p, FILE *f, FDSet *fd
                         return r;
         }
 
-        if (p->idle_pipe) {
+        if (exec_parameters_is_idle_pipe_set(p)) {
                 _cleanup_free_ char *serialized_fds = NULL;
 
                 for (size_t i = 0; i < 4; ++i) {
-                        int copy = -EBADF;
-
-                        if (p->idle_pipe[i] >= 0) {
-                                copy = fdset_put_dup(fds, p->idle_pipe[i]);
-                                if (copy < 0)
-                                        return copy;
-                        }
+                        int copy = fdset_put_dup(fds, p->idle_pipe[i]);
+                        if (copy < 0)
+                                return copy;
 
                         r = strextendf(&serialized_fds, "%d ", copy);
                         if (r < 0)
@@ -1609,17 +1597,11 @@ static int exec_parameters_deserialize(ExecParameters *p, FILE *f, FDSet *fds) {
                                 if (r == 0)
                                         break;
 
-                                if ((fd = parse_fd(w)) < 0 || !fdset_contains(fds, fd))
-                                        log_debug("Failed to parse %s value: %s, ignoring.", l, w);
-                                else {
-                                        r = fdset_remove(fds, fd);
-                                        if (r < 0) {
-                                                log_debug_errno(r, "Failed to remove %s value=%d from fdset, ignoring: %m", l, fd);
-                                                continue;
-                                        }
+                                fd = deserialize_fd(fds, w);
+                                if (fd < 0)
+                                        continue;
 
-                                        p->fds[i] = fd;
-                                }
+                                p->fds[i] = fd;
                         }
                 } else if ((val = startswith(l, "exec-parameters-fd-names="))) {
                         r = deserialize_strv(&p->fd_names, val);
@@ -1719,94 +1701,60 @@ static int exec_parameters_deserialize(ExecParameters *p, FILE *f, FDSet *fds) {
                                 if (r == 0)
                                         break;
 
-                                if ((fd = parse_fd(w)) < 0 || !fdset_contains(fds, fd))
-                                        log_debug("Failed to parse %s value: %s, ignoring.", l, w);
-                                else {
-                                        r = fdset_remove(fds, fd);
-                                        if (r < 0) {
-                                                log_debug_errno(r, "Failed to remove %s value=%d from fdset, ignoring: %m", l, fd);
-                                                continue;
-                                        }
+                                fd = deserialize_fd(fds, w);
+                                if (fd < 0)
+                                        continue;
 
-                                        p->idle_pipe[i] = fd;
-                                }
+                                p->idle_pipe[i] = fd;
                         }
                 } else if ((val = startswith(l, "exec-parameters-stdin-fd="))) {
                         int fd;
 
-                        if ((fd = parse_fd(val)) < 0 || !fdset_contains(fds, fd))
-                                log_debug("Failed to parse %s value: %s, ignoring.", l, val);
-                        else {
-                                r = fdset_remove(fds, fd);
-                                if (r < 0) {
-                                        log_debug_errno(r, "Failed to remove %s value=%d from fdset, ignoring: %m", l, fd);
-                                        continue;
-                                }
+                        fd = deserialize_fd(fds, val);
+                        if (fd < 0)
+                                continue;
+
+                        p->stdin_fd = fd;
 
-                                p->stdin_fd = fd;
-                        }
                 } else if ((val = startswith(l, "exec-parameters-stdout-fd="))) {
                         int fd;
 
-                        if ((fd = parse_fd(val)) < 0 || !fdset_contains(fds, fd))
-                                log_debug("Failed to parse %s value: %s, ignoring.", l, val);
-                        else {
-                                r = fdset_remove(fds, fd);
-                                if (r < 0) {
-                                        log_debug_errno(r, "Failed to remove %s value=%d from fdset, ignoring: %m", l, fd);
-                                        continue;
-                                }
+                        fd = deserialize_fd(fds, val);
+                        if (fd < 0)
+                                continue;
+
+                        p->stdout_fd = fd;
 
-                                p->stdout_fd = fd;
-                        }
                 } else if ((val = startswith(l, "exec-parameters-stderr-fd="))) {
                         int fd;
 
-                        if ((fd = parse_fd(val)) < 0 || !fdset_contains(fds, fd))
-                                log_debug("Failed to parse %s value: %s, ignoring.", l, val);
-                        else {
-                                r = fdset_remove(fds, fd);
-                                if (r < 0) {
-                                        log_debug_errno(r, "Failed to remove %s value=%d from fdset, ignoring: %m", l, fd);
-                                        continue;
-                                }
+                        fd = deserialize_fd(fds, val);
+                        if (fd < 0)
+                                continue;
 
-                                p->stderr_fd = fd;
-                        }
+                        p->stderr_fd = fd;
                 } else if ((val = startswith(l, "exec-parameters-exec-fd="))) {
                         int fd;
 
-                        if ((fd = parse_fd(val)) < 0 || !fdset_contains(fds, fd))
-                                log_debug("Failed to parse %s value: %s, ignoring.", l, val);
-                        else {
-                                r = fdset_remove(fds, fd);
-                                if (r < 0) {
-                                        log_debug_errno(r, "Failed to remove %s value=%d from fdset, ignoring: %m", l, fd);
-                                        continue;
-                                }
+                        fd = deserialize_fd(fds, val);
+                        if (fd < 0)
+                                continue;
 
-                                /* This is special and relies on close-on-exec semantics, make sure it's
-                                 * there */
-                                r = fd_cloexec(fd, true);
-                                if (r < 0)
-                                        return r;
+                        /* This is special and relies on close-on-exec semantics, make sure it's
+                         * there */
+                        r = fd_cloexec(fd, true);
+                        if (r < 0)
+                                return r;
 
-                                p->exec_fd = fd;
-                        }
+                        p->exec_fd = fd;
                 } else if ((val = startswith(l, "exec-parameters-bpf-outer-map-fd="))) {
                         int fd;
 
-                        if ((fd = parse_fd(val)) < 0 || !fdset_contains(fds, fd))
-                                log_debug("Failed to parse %s value: %s, ignoring.", l, val);
-                        else {
-                                r = fdset_remove(fds, fd);
-                                if (r < 0) {
-                                        log_debug_errno(r, "Failed to remove %s value=%d from fdset, ignoring: %m", l, fd);
-                                        continue;
-                                }
+                        fd = deserialize_fd(fds, val);
+                        if (fd < 0)
+                                continue;
 
-                                p->bpf_outer_map_fd = fd;
-                        }
+                        p->bpf_outer_map_fd = fd;
                 } else if ((val = startswith(l, "exec-parameters-notify-socket="))) {
                         r = free_and_strdup(&p->notify_socket, val);
                         if (r < 0)
@@ -1826,17 +1774,11 @@ static int exec_parameters_deserialize(ExecParameters *p, FILE *f, FDSet *fds) {
                 } else if ((val = startswith(l, "exec-parameters-user-lookup-fd="))) {
                         int fd;
 
-                        if ((fd = parse_fd(val)) < 0 || !fdset_contains(fds, fd))
-                                log_debug("Failed to parse %s value: %s, ignoring.", l, val);
-                        else {
-                                r = fdset_remove(fds, fd);
-                                if (r < 0) {
-                                        log_debug_errno(r, "Failed to remove %s value=%d from fdset, ignoring: %m", l, fd);
-                                        continue;
-                                }
+                        fd = deserialize_fd(fds, val);
+                        if (fd < 0)
+                                continue;
 
-                                p->user_lookup_fd = fd;
-                        }
+                        p->user_lookup_fd = fd;
                 } else if ((val = startswith(l, "exec-parameters-files-env="))) {
                         r = deserialize_strv(&p->files_env, val);
                         if (r < 0)

diff --git a/src/core/execute.c b/src/core/execute.c
index 06fcc741f87bbb070b71f597cac1746592c5c226..d9c8a19c380700c9f758ea898257009a1bba4bef 100644 (file)
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -2002,27 +2002,18 @@ int exec_shared_runtime_deserialize_compat(Unit *u, const char *key, const char
                         return -ENOMEM;
 
         } else if (streq(key, "netns-socket-0")) {
-                int fd;
-
-                if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd)) {
-                        log_unit_debug(u, "Failed to parse netns socket value: %s", value);
-                        return 0;
-                }
 
                 safe_close(rt->netns_storage_socket[0]);
-                rt->netns_storage_socket[0] = fdset_remove(fds, fd);
+                rt->netns_storage_socket[0] = deserialize_fd(fds, value);
+                if (rt->netns_storage_socket[0] < 0)
+                        return 0;
 
         } else if (streq(key, "netns-socket-1")) {
-                int fd;
-
-                if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd)) {
-                        log_unit_debug(u, "Failed to parse netns socket value: %s", value);
-                        return 0;
-                }
 
                 safe_close(rt->netns_storage_socket[1]);
-                rt->netns_storage_socket[1] = fdset_remove(fds, fd);
-
+                rt->netns_storage_socket[1] = deserialize_fd(fds, value);
+                if (rt->netns_storage_socket[1] < 0)
+                        return 0;
         } else
                 return 0;
 
@@ -2088,13 +2079,9 @@ int exec_shared_runtime_deserialize_one(Manager *m, const char *value, FDSet *fd
                 n = strcspn(v, " ");
                 buf = strndupa_safe(v, n);
 
-                netns_fdpair[0] = parse_fd(buf);
+                netns_fdpair[0] = deserialize_fd(fds, buf);
                 if (netns_fdpair[0] < 0)
-                        return log_debug_errno(netns_fdpair[0], "Unable to parse exec-runtime specification netns-socket-0=%s: %m", buf);
-                if (!fdset_contains(fds, netns_fdpair[0]))
-                        return log_debug_errno(SYNTHETIC_ERRNO(EBADF),
-                                               "exec-runtime specification netns-socket-0= refers to unknown fd %d: %m", netns_fdpair[0]);
-                netns_fdpair[0] = fdset_remove(fds, netns_fdpair[0]);
+                        return netns_fdpair[0];
                 if (v[n] != ' ')
                         goto finalize;
                 p = v + n + 1;
@@ -2107,13 +2094,9 @@ int exec_shared_runtime_deserialize_one(Manager *m, const char *value, FDSet *fd
                 n = strcspn(v, " ");
                 buf = strndupa_safe(v, n);
 
-                netns_fdpair[1] = parse_fd(buf);
+                netns_fdpair[1] = deserialize_fd(fds, buf);
                 if (netns_fdpair[1] < 0)
-                        return log_debug_errno(netns_fdpair[1], "Unable to parse exec-runtime specification netns-socket-1=%s: %m", buf);
-                if (!fdset_contains(fds, netns_fdpair[1]))
-                        return log_debug_errno(SYNTHETIC_ERRNO(EBADF),
-                                               "exec-runtime specification netns-socket-1= refers to unknown fd %d: %m", netns_fdpair[1]);
-                netns_fdpair[1] = fdset_remove(fds, netns_fdpair[1]);
+                        return netns_fdpair[1];
                 if (v[n] != ' ')
                         goto finalize;
                 p = v + n + 1;
@@ -2126,13 +2109,9 @@ int exec_shared_runtime_deserialize_one(Manager *m, const char *value, FDSet *fd
                 n = strcspn(v, " ");
                 buf = strndupa_safe(v, n);
 
-                ipcns_fdpair[0] = parse_fd(buf);
+                ipcns_fdpair[0] = deserialize_fd(fds, buf);
                 if (ipcns_fdpair[0] < 0)
-                        return log_debug_errno(ipcns_fdpair[0], "Unable to parse exec-runtime specification ipcns-socket-0=%s: %m", buf);
-                if (!fdset_contains(fds, ipcns_fdpair[0]))
-                        return log_debug_errno(SYNTHETIC_ERRNO(EBADF),
-                                               "exec-runtime specification ipcns-socket-0= refers to unknown fd %d: %m", ipcns_fdpair[0]);
-                ipcns_fdpair[0] = fdset_remove(fds, ipcns_fdpair[0]);
+                        return ipcns_fdpair[0];
                 if (v[n] != ' ')
                         goto finalize;
                 p = v + n + 1;
@@ -2145,13 +2124,9 @@ int exec_shared_runtime_deserialize_one(Manager *m, const char *value, FDSet *fd
                 n = strcspn(v, " ");
                 buf = strndupa_safe(v, n);
 
-                ipcns_fdpair[1] = parse_fd(buf);
+                ipcns_fdpair[1] = deserialize_fd(fds, buf);
                 if (ipcns_fdpair[1] < 0)
-                        return log_debug_errno(ipcns_fdpair[1], "Unable to parse exec-runtime specification ipcns-socket-1=%s: %m", buf);
-                if (!fdset_contains(fds, ipcns_fdpair[1]))
-                        return log_debug_errno(SYNTHETIC_ERRNO(EBADF),
-                                               "exec-runtime specification ipcns-socket-1= refers to unknown fd %d: %m", ipcns_fdpair[1]);
-                ipcns_fdpair[1] = fdset_remove(fds, ipcns_fdpair[1]);
+                        return ipcns_fdpair[1];
         }
 
 finalize:

diff --git a/src/core/manager-serialize.c b/src/core/manager-serialize.c
index 09c49b6d6ca1331f0457752dc636467a46fbd1ed..1d7a1bed35995faa4a881296c97d2003706e76c1 100644 (file)
--- a/src/core/manager-serialize.c
+++ b/src/core/manager-serialize.c
@@ -456,12 +456,11 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                 } else if ((val = startswith(l, "notify-fd="))) {
                         int fd;
 
-                        if ((fd = parse_fd(val)) < 0 || !fdset_contains(fds, fd))
-                                log_notice("Failed to parse notify fd, ignoring: \"%s\"", val);
-                        else {
+                        fd = deserialize_fd(fds, val);
+                        if (fd >= 0) {
                                 m->notify_event_source = sd_event_source_disable_unref(m->notify_event_source);
                                 safe_close(m->notify_fd);
-                                m->notify_fd = fdset_remove(fds, fd);
+                                m->notify_fd = fd;
                         }
 
                 } else if ((val = startswith(l, "notify-socket="))) {
@@ -472,12 +471,11 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                 } else if ((val = startswith(l, "cgroups-agent-fd="))) {
                         int fd;
 
-                        if ((fd = parse_fd(val)) < 0 || !fdset_contains(fds, fd))
-                                log_notice("Failed to parse cgroups agent fd, ignoring.: %s", val);
-                        else {
+                        fd = deserialize_fd(fds, val);
+                        if (fd >= 0) {
                                 m->cgroups_agent_event_source = sd_event_source_disable_unref(m->cgroups_agent_event_source);
                                 safe_close(m->cgroups_agent_fd);
-                                m->cgroups_agent_fd = fdset_remove(fds, fd);
+                                m->cgroups_agent_fd = fd;
                         }
 
                 } else if ((val = startswith(l, "user-lookup="))) {

diff --git a/src/core/service.c b/src/core/service.c
index b4bd343bc9486160b12ea0b98b9234d59e2966e9..8846d04d2301d5fa62b3cf489fc7003303bdda82 100644 (file)
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -3223,27 +3223,27 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
                 }
 
         } else if (streq(key, "socket-fd")) {
-                int fd;
+                asynchronous_close(s->socket_fd);
+                s->socket_fd = deserialize_fd(fds, value);
 
-                if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd))
-                        log_unit_debug(u, "Failed to parse socket-fd value: %s", value);
-                else {
-                        asynchronous_close(s->socket_fd);
-                        s->socket_fd = fdset_remove(fds, fd);
-                }
         } else if (streq(key, "fd-store-fd")) {
                 _cleanup_free_ char *fdv = NULL, *fdn = NULL, *fdp = NULL;
-                int fd, do_poll;
+                _cleanup_close_ int fd = -EBADF;
+                int do_poll;
 
                 r = extract_first_word(&value, &fdv, NULL, 0);
-                if (r <= 0 || (fd = parse_fd(fdv)) < 0 || !fdset_contains(fds, fd)) {
-                        log_unit_debug(u, "Failed to parse fd-store-fd value: %s", value);
+                if (r <= 0) {
+                        log_unit_debug(u, "Failed to parse fd-store-fd value, ignoring: %s", value);
                         return 0;
                 }
 
+                fd = deserialize_fd(fds, fdv);
+                if (fd < 0)
+                        return 0;
+
                 r = extract_first_word(&value, &fdn, NULL, EXTRACT_CUNESCAPE | EXTRACT_UNQUOTE);
                 if (r <= 0) {
-                        log_unit_debug(u, "Failed to parse fd-store-fd value: %s", value);
+                        log_unit_debug(u, "Failed to parse fd-store-fd value, ignoring: %s", value);
                         return 0;
                 }
 
@@ -3251,23 +3251,18 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
                 if (r == 0) {
                         /* If the value is not present, we assume the default */
                         do_poll = 1;
-                } else if (r < 0 || safe_atoi(fdp, &do_poll) < 0) {
-                        log_unit_debug_errno(u, r, "Failed to parse fd-store-fd value \"%s\": %m", value);
+                } else if (r < 0 || (r = safe_atoi(fdp, &do_poll)) < 0) {
+                        log_unit_debug_errno(u, r, "Failed to parse fd-store-fd value \"%s\", ignoring: %m", value);
                         return 0;
                 }
 
-                r = fdset_remove(fds, fd);
-                if (r < 0) {
-                        log_unit_error_errno(u, r, "Could not find deserialized fd %i in fdset: %m", fd);
-                        return 0;
-                }
-                assert(r == fd);
-
                 r = service_add_fd_store(s, fd, fdn, do_poll);
                 if (r < 0) {
-                        log_unit_error_errno(u, r, "Failed to store deserialized fd %i: %m", fd);
+                        log_unit_debug_errno(u, r, "Failed to store deserialized fd %i, ignoring: %m", fd);
                         return 0;
                 }
+
+                TAKE_FD(fd);
         } else if (streq(key, "main-exec-status-pid")) {
                 pid_t pid;
 
@@ -3312,47 +3307,37 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
                 else
                         s->forbid_restart = b;
         } else if (streq(key, "stdin-fd")) {
-                int fd;
 
-                if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd))
-                        log_unit_debug(u, "Failed to parse stdin-fd value: %s", value);
-                else {
-                        asynchronous_close(s->stdin_fd);
-                        s->stdin_fd = fdset_remove(fds, fd);
+                asynchronous_close(s->stdin_fd);
+                s->stdin_fd = deserialize_fd(fds, value);
+                if (s->stdin_fd >= 0)
                         s->exec_context.stdio_as_fds = true;
-                }
+
         } else if (streq(key, "stdout-fd")) {
-                int fd;
 
-                if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd))
-                        log_unit_debug(u, "Failed to parse stdout-fd value: %s", value);
-                else {
-                        asynchronous_close(s->stdout_fd);
-                        s->stdout_fd = fdset_remove(fds, fd);
+                asynchronous_close(s->stdout_fd);
+                s->stdout_fd = deserialize_fd(fds, value);
+                if (s->stdout_fd >= 0)
                         s->exec_context.stdio_as_fds = true;
-                }
+
         } else if (streq(key, "stderr-fd")) {
-                int fd;
 
-                if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd))
-                        log_unit_debug(u, "Failed to parse stderr-fd value: %s", value);
-                else {
-                        asynchronous_close(s->stderr_fd);
-                        s->stderr_fd = fdset_remove(fds, fd);
+                asynchronous_close(s->stderr_fd);
+                s->stderr_fd = deserialize_fd(fds, value);
+                if (s->stderr_fd >= 0)
                         s->exec_context.stdio_as_fds = true;
-                }
+
         } else if (streq(key, "exec-fd")) {
-                int fd;
+                _cleanup_close_ int fd = -EBADF;
 
-                if ((fd = parse_fd(value)) < 0 || !fdset_contains(fds, fd))
-                        log_unit_debug(u, "Failed to parse exec-fd value: %s", value);
-                else {
+                fd = deserialize_fd(fds, value);
+                if (fd >= 0) {
                         s->exec_fd_event_source = sd_event_source_disable_unref(s->exec_fd_event_source);
 
-                        fd = fdset_remove(fds, fd);
-                        if (service_allocate_exec_fd_event_source(s, fd, &s->exec_fd_event_source) < 0)
-                                safe_close(fd);
+                        if (service_allocate_exec_fd_event_source(s, fd, &s->exec_fd_event_source) >= 0)
+                                TAKE_FD(fd);
                 }
+
         } else if (streq(key, "watchdog-override-usec")) {
                 if (deserialize_usec(value, &s->watchdog_override_usec) < 0)
                         log_unit_debug(u, "Failed to parse watchdog_override_usec value: %s", value);

diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c
index be041010312a47505d8631cf03b1240b763b20ec..0ffcceaf53d7deb54d92c5645e96eb271b6a5011 100644 (file)
--- a/src/core/unit-serialize.c
+++ b/src/core/unit-serialize.c
@@ -387,16 +387,9 @@ int unit_deserialize_state(Unit *u, FILE *f, FDSet *fds) {
                 else if (STR_IN_SET(l, "ipv4-socket-bind-bpf-link-fd", "ipv6-socket-bind-bpf-link-fd")) {
                         int fd;
 
-                        if ((fd = parse_fd(v)) < 0 || !fdset_contains(fds, fd))
-                                log_unit_debug(u, "Failed to parse %s value: %s, ignoring.", l, v);
-                        else {
-                                if (fdset_remove(fds, fd) < 0) {
-                                        log_unit_debug(u, "Failed to remove %s value=%d from fdset", l, fd);
-                                        continue;
-                                }
-
+                        fd = deserialize_fd(fds, v);
+                        if (fd >= 0)
                                 (void) bpf_socket_bind_add_initial_link_fd(u, fd);
-                        }
                         continue;
 
                 } else if (streq(l, "ip-bpf-ingress-installed")) {
@@ -419,16 +412,10 @@ int unit_deserialize_state(Unit *u, FILE *f, FDSet *fds) {
                 } else if (streq(l, "restrict-ifaces-bpf-fd")) {
                         int fd;
 
-                        if ((fd = parse_fd(v)) < 0 || !fdset_contains(fds, fd)) {
-                                log_unit_debug(u, "Failed to parse restrict-ifaces-bpf-fd value: %s", v);
-                                continue;
-                        }
-                        if (fdset_remove(fds, fd) < 0) {
-                                log_unit_debug(u, "Failed to remove restrict-ifaces-bpf-fd %d from fdset", fd);
-                                continue;
-                        }
+                        fd = deserialize_fd(fds, v);
+                        if (fd >= 0)
+                                (void) restrict_network_interfaces_add_initial_link_fd(u, fd);
 
-                        (void) restrict_network_interfaces_add_initial_link_fd(u, fd);
                         continue;
 
                 } else if (streq(l, "ref-uid")) {

diff --git a/src/shared/serialize.c b/src/shared/serialize.c
index ee35055aeaeb02460030768b7302478c34dbdf7d..d62dfaa2d7a7f7752f025d3a51bb8f62d45319f4 100644 (file)
--- a/src/shared/serialize.c
+++ b/src/shared/serialize.c
@@ -285,6 +285,23 @@ int deserialize_read_line(FILE *f, char **ret) {
         return 1;
 }
 
+int deserialize_fd(FDSet *fds, const char *value) {
+        _cleanup_close_ int our_fd = -EBADF;
+        int parsed_fd;
+
+        assert(value);
+
+        parsed_fd = parse_fd(value);
+        if (parsed_fd < 0)
+                return log_debug_errno(parsed_fd, "Failed to parse file descriptor serialization: %s", value);
+
+        our_fd = fdset_remove(fds, parsed_fd); /* Take possession of the fd */
+        if (our_fd < 0)
+                return log_debug_errno(our_fd, "Failed to acquire fd from serialization fds: %m");
+
+        return TAKE_FD(our_fd);
+}
+
 int deserialize_strv(char ***l, const char *value) {
         ssize_t unescaped_len;
         char *unescaped;
@@ -372,18 +389,12 @@ int deserialize_pidref(FDSet *fds, const char *value, PidRef *ret) {
 
         e = startswith(value, "@");
         if (e) {
-                _cleanup_close_ int our_fd = -EBADF;
-                int parsed_fd;
-
-                parsed_fd = parse_fd(e);
-                if (parsed_fd < 0)
-                        return log_debug_errno(parsed_fd, "Failed to parse file descriptor specification: %s", e);
+                int fd = deserialize_fd(fds, e);
 
-                our_fd = fdset_remove(fds, parsed_fd); /* Take possession of the fd */
-                if (our_fd < 0)
-                        return log_debug_errno(our_fd, "Failed to acquire pidfd from serialization fds: %m");
+                if (fd < 0)
+                        return fd;
 
-                r = pidref_set_pidfd_consume(ret, TAKE_FD(our_fd));
+                r = pidref_set_pidfd_consume(ret, fd);
         } else {
                 pid_t pid;
 

diff --git a/src/shared/serialize.h b/src/shared/serialize.h
index 5122c42793677963c564c303b68ac38ab4fa0af5..decad81de7d4fcf35e210594d92af6582dc06125 100644 (file)
--- a/src/shared/serialize.h
+++ b/src/shared/serialize.h
@@ -37,6 +37,7 @@ static inline int serialize_item_tristate(FILE *f, const char *key, int value) {
 
 int deserialize_read_line(FILE *f, char **ret);
 
+int deserialize_fd(FDSet *fds, const char *value);
 int deserialize_usec(const char *value, usec_t *timestamp);
 int deserialize_dual_timestamp(const char *value, dual_timestamp *t);
 int deserialize_environment(const char *value, char ***environment);