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
This commit is contained in:
Lennart Poettering
2021-11-25 00:05:03 +01:00
parent bc6b326d48
commit 3fabebf45e
3 changed files with 24 additions and 16 deletions
+18 -9
View File
@@ -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));
+5 -4
View File
@@ -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_;
+1 -3
View File
@@ -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