From: Lennart Poettering Date: Fri, 6 Oct 2023 15:50:48 +0000 (+0200) Subject: serialize: add new helper deserialize_fd() X-Git-Tag: v255-rc1~212^2~6 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=dff9808a628c31b7ecb1f1aba8fdc3be06ce8372;p=thirdparty%2Fsystemd.git 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 a34bfae3ac8..8df715b1afd 100644 --- 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 dc349afecfa..a70f64f7ff6 100644 --- 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 147bbd60edd..1886a59b84c 100644 --- 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 06fcc741f87..d9c8a19c380 100644 --- 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 09c49b6d6ca..1d7a1bed359 100644 --- 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 b4bd343bc94..8846d04d230 100644 --- 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 be041010312..0ffcceaf53d 100644 --- 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 ee35055aeae..d62dfaa2d7a 100644 --- 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 5122c427936..decad81de7d 100644 --- 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);