core/service: use array rather than list for extra fds, limit max number

Follow-up for 3543456f84

I don't think list is particularly useful here. The passed fds are
constant for the lifetime of service, and with this commit we track
the number of extra fds in a dedicated var anyway.
This commit is contained in:
Mike Yuan
2024-10-08 14:53:14 +02:00
parent 6286f213f5
commit 32af4dd80f
3 changed files with 76 additions and 118 deletions

View File

@@ -78,7 +78,7 @@ static int property_get_extra_file_descriptors(
void *userdata,
sd_bus_error *error) {
ServiceExtraFD **extra_fds = ASSERT_PTR(userdata);
Service *s = ASSERT_PTR(userdata);
int r;
assert(bus);
@@ -88,8 +88,8 @@ static int property_get_extra_file_descriptors(
if (r < 0)
return r;
LIST_FOREACH(extra_fd, efd, *extra_fds) {
r = sd_bus_message_append_basic(reply, 's', efd->fdname);
FOREACH_ARRAY(i, s->extra_fds, s->n_extra_fds) {
r = sd_bus_message_append_basic(reply, 's', i->fdname);
if (r < 0)
return r;
}
@@ -367,7 +367,7 @@ const sd_bus_vtable bus_service_vtable[] = {
SD_BUS_PROPERTY("NRestarts", "u", bus_property_get_unsigned, offsetof(Service, n_restarts), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
SD_BUS_PROPERTY("OOMPolicy", "s", bus_property_get_oom_policy, offsetof(Service, oom_policy), SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY("OpenFile", "a(sst)", property_get_open_files, offsetof(Service, open_files), SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY("ExtraFileDescriptorNames", "as", property_get_extra_file_descriptors, offsetof(Service, extra_fds), SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY("ExtraFileDescriptorNames", "as", property_get_extra_file_descriptors, 0, SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY("ReloadSignal", "i", bus_property_get_int, offsetof(Service, reload_signal), SD_BUS_VTABLE_PROPERTY_CONST),
BUS_EXEC_STATUS_VTABLE("ExecMain", offsetof(Service, main_exec_status), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
@@ -748,15 +748,13 @@ static int bus_service_set_transient_property(
return bus_set_transient_reload_signal(u, name, &s->reload_signal, message, flags, error);
if (streq(name, "ExtraFileDescriptors")) {
int fd;
const char *fdname;
r = sd_bus_message_enter_container(message, 'a', "(hs)");
if (r < 0)
return r;
for (;;) {
_cleanup_(service_extra_fd_freep) ServiceExtraFD *efd = NULL;
const char *fdname;
int fd;
r = sd_bus_message_read(message, "(hs)", &fd, &fdname);
if (r < 0)
@@ -767,29 +765,30 @@ static int bus_service_set_transient_property(
/* Disallow empty string for ExtraFileDescriptors.
* Unlike OpenFile, StandardInput and friends, there isn't a good sane
* default for an arbitrary FD. */
if (fd < 0 || isempty(fdname) || !fdname_is_valid(fdname))
return -EINVAL;
if (isempty(fdname) || !fdname_is_valid(fdname))
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid extra fd name: %s", fdname);
if (s->n_extra_fds >= NOTIFY_FD_MAX)
return sd_bus_error_set(error, SD_BUS_ERROR_LIMITS_EXCEEDED, "Too many extra fds sent");
if (UNIT_WRITE_FLAGS_NOOP(flags))
continue;
efd = new(ServiceExtraFD, 1);
if (!efd)
if (!GREEDY_REALLOC(s->extra_fds, s->n_extra_fds + 1))
return -ENOMEM;
*efd = (ServiceExtraFD) {
.fd = -EBADF,
.fdname = strdup(fdname),
};
if (!efd->fdname)
_cleanup_free_ char *fdname_dup = strdup(fdname);
if (!fdname_dup)
return -ENOMEM;
efd->fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
if (efd->fd < 0)
_cleanup_close_ int fd_dup = fcntl(fd, F_DUPFD_CLOEXEC, 3);
if (fd_dup < 0)
return -errno;
LIST_APPEND(extra_fd, s->extra_fds, TAKE_PTR(efd));
s->extra_fds[s->n_extra_fds++] = (ServiceExtraFD) {
.fd = TAKE_FD(fd_dup),
.fdname = TAKE_PTR(fdname_dup),
};
}
r = sd_bus_message_exit_container(message);

View File

@@ -446,7 +446,7 @@ static void service_release_fd_store(Service *s) {
if (!s->fd_store)
return;
log_unit_debug(UNIT(s), "Releasing all stored fds");
log_unit_debug(UNIT(s), "Releasing all stored fds.");
while (s->fd_store)
service_fd_store_unlink(s->fd_store);
@@ -454,19 +454,21 @@ static void service_release_fd_store(Service *s) {
assert(s->n_fd_store == 0);
}
ServiceExtraFD* service_extra_fd_free(ServiceExtraFD *efd) {
if (!efd)
return NULL;
efd->fd = asynchronous_close(efd->fd);
free(efd->fdname);
return mfree(efd);
}
static void service_release_extra_fds(Service *s) {
assert(s);
LIST_CLEAR(extra_fd, s->extra_fds, service_extra_fd_free);
if (!s->extra_fds)
return;
log_unit_debug(UNIT(s), "Releasing extra file descriptors.");
FOREACH_ARRAY(i, s->extra_fds, s->n_extra_fds) {
asynchronous_close(i->fd);
free(i->fdname);
}
s->extra_fds = mfree(s->extra_fds);
s->n_extra_fds = 0;
}
static void service_release_stdio_fd(Service *s) {
@@ -924,12 +926,18 @@ static int service_dump_fd(int fd, const char *fdname, const char *header, FILE
struct stat st;
int flags;
assert(fd >= 0);
assert(fdname);
assert(header);
assert(f);
assert(prefix);
if (fstat(fd, &st) < 0)
return log_debug_errno(errno, "Failed to stat fdstore entry: %m");
return log_debug_errno(errno, "Failed to stat service fd: %m");
flags = fcntl(fd, F_GETFL);
if (flags < 0)
return log_debug_errno(errno, "Failed to get fdstore entry flags: %m");
return log_debug_errno(errno, "Failed to get service fd flags: %m");
(void) fd_get_path(fd, &path);
@@ -948,32 +956,6 @@ static int service_dump_fd(int fd, const char *fdname, const char *header, FILE
return 0;
}
static void service_dump_fdstore(Service *s, FILE *f, const char *prefix) {
assert(s);
assert(f);
assert(prefix);
LIST_FOREACH(fd_store, i, s->fd_store)
(void) service_dump_fd(i->fd,
i->fdname,
i == s->fd_store ? "File Descriptor Store Entry:" : " ",
f,
prefix);
}
static void service_dump_extra_fds(Service *s, FILE *f, const char *prefix) {
assert(s);
assert(f);
assert(prefix);
LIST_FOREACH(extra_fd, i, s->extra_fds)
(void) service_dump_fd(i->fd,
i->fdname,
i == s->extra_fds ? "Extra File Descriptor Entry:" : " ",
f,
prefix);
}
static void service_dump(Unit *u, FILE *f, const char *prefix) {
Service *s = ASSERT_PTR(SERVICE(u));
const char *prefix2;
@@ -1102,7 +1084,7 @@ static void service_dump(Unit *u, FILE *f, const char *prefix) {
fprintf(f, "%sStatus Varlink Error: %s\n",
prefix, s->status_varlink_error);
if (s->n_fd_store_max > 0)
if (s->n_fd_store_max > 0) {
fprintf(f,
"%sFile Descriptor Store Max: %u\n"
"%sFile Descriptor Store Pin: %s\n"
@@ -1111,7 +1093,20 @@ static void service_dump(Unit *u, FILE *f, const char *prefix) {
prefix, exec_preserve_mode_to_string(s->fd_store_preserve_mode),
prefix, s->n_fd_store);
service_dump_fdstore(s, f, prefix);
LIST_FOREACH(fd_store, i, s->fd_store)
(void) service_dump_fd(i->fd,
i->fdname,
i == s->fd_store ? "File Descriptor Store Entry:" : " ",
f,
prefix);
}
FOREACH_ARRAY(i, s->extra_fds, s->n_extra_fds)
(void) service_dump_fd(i->fd,
i->fdname,
i == s->extra_fds ? "Extra File Descriptor Entry:" : " ",
f,
prefix);
if (s->open_files)
LIST_FOREACH(open_files, of, s->open_files) {
@@ -1128,8 +1123,6 @@ static void service_dump(Unit *u, FILE *f, const char *prefix) {
fprintf(f, "%sOpen File: %s\n", prefix, ofs);
}
service_dump_extra_fds(s, f, prefix);
cgroup_context_dump(UNIT(s), f, prefix);
}
@@ -1465,7 +1458,7 @@ static int service_collect_fds(
_cleanup_strv_free_ char **rfd_names = NULL;
_cleanup_free_ int *rfds = NULL;
size_t rn_socket_fds = 0, rn_storage_fds = 0, rn_extra_fds = 0;
size_t rn_socket_fds = 0;
int r;
assert(s);
@@ -1520,61 +1513,31 @@ static int service_collect_fds(
}
}
if (s->n_fd_store > 0) {
size_t n_fds;
char **nl;
int *t;
t = reallocarray(rfds, rn_socket_fds + s->n_fd_store, sizeof(int));
if (s->n_fd_store + s->n_extra_fds > 0) {
int *t = reallocarray(rfds, rn_socket_fds + s->n_fd_store + s->n_extra_fds, sizeof(int));
if (!t)
return -ENOMEM;
rfds = t;
nl = reallocarray(rfd_names, rn_socket_fds + s->n_fd_store + 1, sizeof(char *));
char **nl = reallocarray(rfd_names, rn_socket_fds + s->n_fd_store + s->n_extra_fds + 1, sizeof(char *));
if (!nl)
return -ENOMEM;
rfd_names = nl;
n_fds = rn_socket_fds;
size_t n_fds = rn_socket_fds;
LIST_FOREACH(fd_store, fs, s->fd_store) {
rfds[n_fds] = fs->fd;
rfd_names[n_fds] = strdup(strempty(fs->fdname));
rfd_names[n_fds] = strdup(fs->fdname);
if (!rfd_names[n_fds])
return -ENOMEM;
rn_storage_fds++;
n_fds++;
}
rfd_names[n_fds] = NULL;
}
LIST_FOREACH(extra_fd, extra_fd, s->extra_fds)
rn_extra_fds++;
if (rn_extra_fds > 0) {
size_t n_fds;
char **nl;
int *t;
t = reallocarray(rfds, rn_socket_fds + rn_storage_fds + rn_extra_fds, sizeof(int));
if (!t)
return -ENOMEM;
rfds = t;
nl = reallocarray(rfd_names, rn_socket_fds + rn_storage_fds + rn_extra_fds + 1, sizeof(char *));
if (!nl)
return -ENOMEM;
rfd_names = nl;
n_fds = rn_socket_fds + rn_storage_fds;
LIST_FOREACH(extra_fd, extra_fd, s->extra_fds) {
rfds[n_fds] = extra_fd->fd;
rfd_names[n_fds] = strdup(extra_fd->fdname);
FOREACH_ARRAY(i, s->extra_fds, s->n_extra_fds) {
rfds[n_fds] = i->fd;
rfd_names[n_fds] = strdup(i->fdname);
if (!rfd_names[n_fds])
return -ENOMEM;
@@ -1587,8 +1550,8 @@ static int service_collect_fds(
*fds = TAKE_PTR(rfds);
*fd_names = TAKE_PTR(rfd_names);
*n_socket_fds = rn_socket_fds;
*n_storage_fds = rn_storage_fds;
*n_extra_fds = rn_extra_fds;
*n_storage_fds = s->n_fd_store;
*n_extra_fds = s->n_extra_fds;
return 0;
}

View File

@@ -115,8 +115,6 @@ struct ServiceFDStore {
struct ServiceExtraFD {
int fd;
char *fdname;
LIST_FIELDS(ServiceExtraFD, extra_fd);
};
struct Service {
@@ -228,23 +226,24 @@ struct Service {
unsigned n_fd_store_max;
ExecPreserveMode fd_store_preserve_mode;
char *usb_function_descriptors;
char *usb_function_strings;
int stdin_fd;
int stdout_fd;
int stderr_fd;
OOMPolicy oom_policy;
/* If service spawned from transient unit, extra file descriptors can be passed via dbus API */
ServiceExtraFD *extra_fds;
size_t n_extra_fds;
LIST_HEAD(OpenFile, open_files);
/* If service spawned from transient unit, extra file descriptors can be passed via dbus API */
LIST_HEAD(ServiceExtraFD, extra_fds);
int reload_signal;
usec_t reload_begin_usec;
OOMPolicy oom_policy;
char *usb_function_descriptors;
char *usb_function_strings;
/* The D-Bus request, we will reply once the operation is finished, so that callers can block */
sd_bus_message *mount_request;
};
@@ -306,6 +305,3 @@ DEFINE_CAST(SERVICE, Service);
/* Only exported for unit tests */
int service_deserialize_exec_command(Unit *u, const char *key, const char *value);
ServiceExtraFD* service_extra_fd_free(ServiceExtraFD *efd);
DEFINE_TRIVIAL_CLEANUP_FUNC(ServiceExtraFD*, service_extra_fd_free);