socket: always pass socket, fd and SocketPeer ownership to service together

Per-connection socket instances we currently maintain three fields
related to the socket: a reference to the Socket unit, the connection fd,
and a reference to the SocketPeer object that counts socket peers.

Let's synchronize their lifetime, i.e. always set them all three
together or unset them together, so that their reference counters stay
synchronous.

THis will in particuar ensure that we'll drop the SocketPeer reference
whenever we leave an active state of the service unit, i.e. at the same
time we close the fd for it.

Fixes: #20685

3 files changed, 24 insertions, 16 deletions

diff --git a/src/core/service.c b/src/core/service.c
index 17c19a2c4a..49579f7998 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -190,6 +190,8 @@ void service_close_socket_fd(Service *s) {
                 socket_connection_unref(SOCKET(UNIT_DEREF(s->accept_socket)));
                 unit_ref_unset(&s->accept_socket);
         }
+
+        s->socket_peer = socket_peer_unref(s->socket_peer);
 }
 
 static void service_stop_watchdog(Service *s) {
@@ -388,7 +390,6 @@ static void service_done(Unit *u) {
         s->usb_function_strings = mfree(s->usb_function_strings);
 
         service_close_socket_fd(s);
-        s->peer = socket_peer_unref(s->peer);
 
         unit_ref_unset(&s->accept_socket);
 
@@ -1237,8 +1238,8 @@ static int service_coldplug(Unit *u) {
 
                         /* Make a best-effort attempt at bumping the connection count */
                         if (socket_acquire_peer(socket, s->socket_fd, &peer) > 0) {
-                                socket_peer_unref(s->peer);
-                                s->peer = peer;
+                                socket_peer_unref(s->socket_peer);
+                                s->socket_peer = peer;
                         }
                 }
         }
@@ -4285,8 +4286,14 @@ static void service_bus_name_owner_change(Unit *u, const char *new_owner) {
         }
 }
 
-int service_set_socket_fd(Service *s, int fd, Socket *sock, bool selinux_context_net) {
-        _cleanup_free_ char *peer = NULL;
+int service_set_socket_fd(
+                Service *s,
+                int fd,
+                Socket *sock,
+                SocketPeer *peer,
+                bool selinux_context_net) {
+
+        _cleanup_free_ char *peer_text = NULL;
         int r;
 
         assert(s);
@@ -4301,22 +4308,23 @@ int service_set_socket_fd(Service *s, int fd, Socket *sock, bool selinux_context
         if (s->socket_fd >= 0)
                 return -EBUSY;
 
+        assert(!s->socket_peer);
+
         if (s->state != SERVICE_DEAD)
                 return -EAGAIN;
 
-        if (getpeername_pretty(fd, true, &peer) >= 0) {
+        if (getpeername_pretty(fd, true, &peer_text) >= 0) {
 
                 if (UNIT(s)->description) {
                         _cleanup_free_ char *a = NULL;
 
-                        a = strjoin(UNIT(s)->description, " (", peer, ")");
+                        a = strjoin(UNIT(s)->description, " (", peer_text, ")");
                         if (!a)
                                 return -ENOMEM;
 
                         r = unit_set_description(UNIT(s), a);
                 }  else
-                        r = unit_set_description(UNIT(s), peer);
-
+                        r = unit_set_description(UNIT(s), peer_text);
                 if (r < 0)
                         return r;
         }
@@ -4326,6 +4334,7 @@ int service_set_socket_fd(Service *s, int fd, Socket *sock, bool selinux_context
                 return r;
 
         s->socket_fd = fd;
+        s->socket_peer = socket_peer_ref(peer);
         s->socket_fd_selinux_context_net = selinux_context_net;
 
         unit_ref_set(&s->accept_socket, UNIT(s), UNIT(sock));
diff --git a/src/core/service.h b/src/core/service.h
index 70ce70fba5..778551d844 100644
--- a/src/core/service.h
+++ b/src/core/service.h
@@ -157,8 +157,11 @@ struct Service {
         DynamicCreds dynamic_creds;
 
         pid_t main_pid, control_pid;
+
+        /* if we are a socket activated service instance, store information of the connection/peer/socket */
         int socket_fd;
-        SocketPeer *peer;
+        SocketPeer *socket_peer;
+        UnitRef accept_socket;
         bool socket_fd_selinux_context_net;
 
         bool permissions_start_only;
@@ -186,8 +189,6 @@ struct Service {
         char *status_text;
         int status_errno;
 
-        UnitRef accept_socket;
-
         sd_event_source *timer_event_source;
         PathSpec *pid_file_pathspec;
 
@@ -226,7 +227,7 @@ static inline usec_t service_get_watchdog_usec(Service *s) {
 
 extern const UnitVTable service_vtable;
 
-int service_set_socket_fd(Service *s, int fd, struct Socket *socket, bool selinux_context_net);
+int service_set_socket_fd(Service *s, int fd, struct Socket *socket, struct SocketPeer *peer, bool selinux_context_net);
 void service_close_socket_fd(Service *s);
 
 const char* service_restart_to_string(ServiceRestart i) _const_;
diff --git a/src/core/socket.c b/src/core/socket.c
index e6d168188a..d9db1edd3c 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -2409,7 +2409,7 @@ static void socket_enter_running(Socket *s, int cfd_in) {
 
                 s->n_accepted++;
 
-                r = service_set_socket_fd(SERVICE(service), cfd, s, s->selinux_context_from_net);
+                r = service_set_socket_fd(SERVICE(service), cfd, s, p, s->selinux_context_from_net);
                 if (ERRNO_IS_DISCONNECT(r))
                         return;
                 if (r < 0)
@@ -2418,8 +2418,6 @@ static void socket_enter_running(Socket *s, int cfd_in) {
                 TAKE_FD(cfd); /* We passed ownership of the fd to the service now. Forget it here. */
                 s->n_connections++;
 
-                SERVICE(service)->peer = TAKE_PTR(p); /* Pass ownership of the peer reference */
-
                 r = manager_add_job(UNIT(s)->manager, JOB_START, service, JOB_REPLACE, NULL, &error, NULL);
                 if (r < 0) {
                         /* We failed to activate the new service, but it still exists. Let's make sure the