picnoir/Systemd
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

core: create socket service instances with the correct name from the start

Browse source 
Upon an incoming connection for an accepting socket, we'd create a unit like
foo@0.service, then figure out that the instance name should be e.g. "0-41-0",
and then add the name foo@0-41-0.service to the unit. This obviously violates
the rule that any service needs to have a constance instance part.

So let's reverse the order: we first determine the instance name and then
create the unit with the correct name from the start.

There are two cases where we don't know the instance name:
- analyze-verify: we just do a quick check that the instance unit can be
  created. So let's use a bogus instance string.
- selinux: the code wants to load the service unit to extract the ExecStart path
  and query it for the selinux label. Do the same as above.

Note that in both cases it is possible that the real unit that is loaded could
be different than the one with the bogus instance value, for example if there
is a dropin for a specific instance name. We can't do much about this, since we
can't figure out the instance name in advance. The old code had the same
shortcoming.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2020-05-29 14:24:12 +02:00
parent ada4b34ec7
commit 934ef6a522
3 changed files with 82 additions and 86 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
src/analyze/analyze-verify.c
View file

@ -94,6 +94,7 @@ static int generate_path(char **var, char **filenames) {
}
static int verify_socket(Unit *u) {
Unit *service;
int r;
assert(u);
@ -101,26 +102,15 @@ static int verify_socket(Unit *u) {
if (u->type != UNIT_SOCKET)
return 0;
/* Cannot run this without the service being around */
/* This makes sure instance is created if necessary. */
r = socket_instantiate_service(SOCKET(u));
r = socket_load_service_unit(SOCKET(u), -1, &service);
if (r < 0)
return log_unit_error_errno(u, r, "Socket cannot be started, failed to create instance: %m");
return log_unit_error_errno(u, r, "service unit for the socket cannot be loaded: %m");
/* This checks both type of sockets */
if (UNIT_ISSET(SOCKET(u)->service)) {
Service *service;
service = SERVICE(UNIT_DEREF(SOCKET(u)->service));
log_unit_debug(u, "Using %s", UNIT(service)->id);
if (UNIT(service)->load_state != UNIT_LOADED) {
log_unit_error(u, "Service %s not loaded, %s cannot be started.", UNIT(service)->id, u->id);
return -ENOENT;
}
}
if (service->load_state != UNIT_LOADED)
return log_unit_error_errno(u, SYNTHETIC_ERRNO(ENOENT),
"service %s not loaded, socket cannot be started.", service->id);
log_unit_debug(u, "using service unit %s.", service->id);
return 0;
}

142
src/core/socket.c
View file

@ -205,38 +205,25 @@ static int socket_arm_timer(Socket *s, usec_t usec) {
return 0;
}
int socket_instantiate_service(Socket *s) {
_cleanup_free_ char *prefix = NULL, *name = NULL;
static int socket_instantiate_service(Socket *s, int cfd) {
Unit *service;
int r;
Unit *u;
assert(s);
assert(cfd >= 0);
/* This fills in s->service if it isn't filled in yet. For
* Accept=yes sockets we create the next connection service
* here. For Accept=no this is mostly a NOP since the service
* is figured out at load time anyway. */
/* This fills in s->service if it isn't filled in yet. For Accept=yes sockets we create the next
* connection service here. For Accept=no this is mostly a NOP since the service is figured out at
* load time anyway. */
if (UNIT_DEREF(s->service))
return 0;
if (!s->accept)
return 0;
r = unit_name_to_prefix(UNIT(s)->id, &prefix);
r = socket_load_service_unit(s, cfd, &service);
if (r < 0)
return r;
if (asprintf(&name, "%s@%u.service", prefix, s->n_accepted) < 0)
return -ENOMEM;
unit_ref_set(&s->service, UNIT(s), service);
r = manager_load_unit(UNIT(s)->manager, name, NULL, NULL, &u);
if (r < 0)
return r;
unit_ref_set(&s->service, UNIT(s), u);
return unit_add_two_dependencies(UNIT(s), UNIT_BEFORE, UNIT_TRIGGERS, u, false, UNIT_DEPENDENCY_IMPLICIT);
return unit_add_two_dependencies(UNIT(s), UNIT_BEFORE, UNIT_TRIGGERS, service,
false, UNIT_DEPENDENCY_IMPLICIT);
}
static bool have_non_accept_socket(Socket *s) {
@ -1398,37 +1385,81 @@ clear:
return r;
}
int socket_load_service_unit(Socket *s, int cfd, Unit **ret) {
/* Figure out what the unit that will be used to handle the connections on the socket looks like.
*
* If cfd < 0, then we don't have a connection yet. In case of Accept=yes sockets, use a fake
* instance name.
*/
if (UNIT_ISSET(s->service)) {
*ret = UNIT_DEREF(s->service);
return 0;
}
if (!s->accept)
return -ENODATA;
/* Build the instance name and load the unit */
_cleanup_free_ char *prefix = NULL, *instance = NULL, *name = NULL;
int r;
r = unit_name_to_prefix(UNIT(s)->id, &prefix);
if (r < 0)
return r;
if (cfd >= 0) {
r = instance_from_socket(cfd, s->n_accepted, &instance);
if (r == -ENOTCONN)
/* ENOTCONN is legitimate if TCP RST was received.
* This connection is over, but the socket unit lives on. */
return log_unit_debug_errno(UNIT(s), r,
"Got ENOTCONN on incoming socket, assuming aborted connection attempt, ignoring.");
if (r < 0)
return r;
}
/* For accepting sockets, we don't know how the instance will be called until we get a connection and
* can figure out what the peer name is. So let's use "internal" as the instance to make it clear
* that this is not an actual peer name. We use "unknown" when we cannot figure out the peer. */
r = unit_name_build(prefix, instance ?: "internal", ".service", &name);
if (r < 0)
return r;
return manager_load_unit(UNIT(s)->manager, name, NULL, NULL, ret);
}
static int socket_determine_selinux_label(Socket *s, char **ret) {
Service *service;
ExecCommand *c;
_cleanup_free_ char *path = NULL;
int r;
assert(s);
assert(ret);
if (s->selinux_context_from_net) {
/* If this is requested, get label from the network label */
/* If this is requested, get the label from the network label */
r = mac_selinux_get_our_label(ret);
if (r == -EOPNOTSUPP)
goto no_label;
} else {
/* Otherwise, get it from the executable we are about to start */
r = socket_instantiate_service(s);
/* Otherwise, get it from the executable we are about to start. */
Unit *service;
ExecCommand *c;
_cleanup_free_ char *path = NULL;
r = socket_load_service_unit(s, -1, &service);
if (r == -ENODATA)
goto no_label;
if (r < 0)
return r;
if (!UNIT_ISSET(s->service))
goto no_label;
service = SERVICE(UNIT_DEREF(s->service));
c = service->exec_command[SERVICE_EXEC_START];
c = SERVICE(service)->exec_command[SERVICE_EXEC_START];
if (!c)
goto no_label;
r = chase_symlinks(c->path, service->exec_context.root_directory, CHASE_PREFIX_ROOT, &path, NULL);
r = chase_symlinks(c->path, SERVICE(service)->exec_context.root_directory, CHASE_PREFIX_ROOT, &path, NULL);
if (r < 0)
goto no_label;
@ -1614,8 +1645,8 @@ static int socket_open_fds(Socket *_s) {
case SOCKET_SOCKET:
if (!know_label) {
/* Figure out label, if we don't it know yet. We do it once, for the first socket where
* we need this and remember it for the rest. */
/* Figure out the label, if we don't it know yet. We do it once for the first
* socket where we need this and remember it for the rest. */
r = socket_determine_selinux_label(s, &label);
if (r < 0)
@ -2332,7 +2363,6 @@ static void socket_enter_running(Socket *s, int cfd) {
socket_set_state(s, SOCKET_RUNNING);
} else {
_cleanup_free_ char *prefix = NULL, *instance = NULL, *name = NULL;
_cleanup_(socket_peer_unrefp) SocketPeer *p = NULL;
Service *service;
@ -2344,9 +2374,9 @@ static void socket_enter_running(Socket *s, int cfd) {
if (s->max_connections_per_source > 0) {
r = socket_acquire_peer(s, cfd, &p);
if (r < 0) {
if (r < 0)
goto refuse;
} else if (r > 0 && p->n_ref > s->max_connections_per_source) {
if (r > 0 && p->n_ref > s->max_connections_per_source) {
_cleanup_free_ char *t = NULL;
(void) sockaddr_pretty(&p->peer.sa, p->peer_salen, true, false, &t);
@ -2358,30 +2388,7 @@ static void socket_enter_running(Socket *s, int cfd) {
}
}
r = socket_instantiate_service(s);
if (r < 0)
goto fail;
r = instance_from_socket(cfd, s->n_accepted, &instance);
if (r < 0) {
if (r != -ENOTCONN)
goto fail;
/* ENOTCONN is legitimate if TCP RST was received.
* This connection is over, but the socket unit lives on. */
log_unit_debug(UNIT(s), "Got ENOTCONN on incoming socket, assuming aborted connection attempt, ignoring.");
goto refuse;
}
r = unit_name_to_prefix(UNIT(s)->id, &prefix);
if (r < 0)
goto fail;
r = unit_name_build(prefix, instance, ".service", &name);
if (r < 0)
goto fail;
r = unit_add_name(UNIT_DEREF(s->service), name);
r = socket_instantiate_service(s, cfd);
if (r < 0)
goto fail;
@ -2389,21 +2396,20 @@ static void socket_enter_running(Socket *s, int cfd) {
unit_ref_unset(&s->service);
s->n_accepted++;
unit_choose_id(UNIT(service), name);
r = service_set_socket_fd(service, cfd, s, s->selinux_context_from_net);
if (r < 0)
goto fail;
cfd = -1; /* We passed ownership of the fd to the service now. Forget it here. */
TAKE_FD(cfd); /* We passed ownership of the fd to the service now. Forget it here. */
s->n_connections++;
service->peer = TAKE_PTR(p); /* Pass ownership of the peer reference */
r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT(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 service
* closes and forgets the connection fd again, immediately. */
/* We failed to activate the new service, but it still exists. Let's make sure the
* service closes and forgets the connection fd again, immediately. */
service_close_socket_fd(service);
goto fail;
}

2
src/core/socket.h
View file

@ -165,7 +165,7 @@ void socket_connection_unref(Socket *s);
void socket_free_ports(Socket *s);
int socket_instantiate_service(Socket *s);
int socket_load_service_unit(Socket *s, int cfd, Unit **ret);
char *socket_fdname(Socket *s);