Merge pull request #33791 from YHNdnzj/sd-notify-pidref

core: pin notify sender through pidfd (potentially SCM_PIDFD) plus various cleanups
This commit is contained in:
Luca Boccassi
2024-07-23 10:36:57 +01:00
committed by GitHub
13 changed files with 147 additions and 105 deletions
+14 -20
View File
@@ -49,24 +49,24 @@
static void destroy_bus(Manager *m, sd_bus **bus);
int bus_send_pending_reload_message(Manager *m) {
void bus_send_pending_reload_message(Manager *m) {
int r;
assert(m);
if (!m->pending_reload_message)
return 0;
return;
/* If we cannot get rid of this message we won't dispatch any D-Bus messages, so that we won't end up wanting
* to queue another message. */
r = sd_bus_send(NULL, m->pending_reload_message, NULL);
if (r < 0)
log_warning_errno(r, "Failed to send queued message, ignoring: %m");
log_warning_errno(r, "Failed to send queued reload message, ignoring: %m");
m->pending_reload_message = sd_bus_message_unref(m->pending_reload_message);
return 0;
return;
}
int bus_forward_agent_released(Manager *m, const char *path) {
@@ -825,6 +825,8 @@ int bus_init_api(Manager *m) {
_cleanup_(sd_bus_close_unrefp) sd_bus *bus = NULL;
int r;
assert(m);
if (m->api_bus)
return 0;
@@ -857,7 +859,7 @@ int bus_init_api(Manager *m) {
return 0;
}
static int bus_setup_system(Manager *m, sd_bus *bus) {
static void bus_setup_system(Manager *m, sd_bus *bus) {
int r;
assert(m);
@@ -878,7 +880,7 @@ static int bus_setup_system(Manager *m, sd_bus *bus) {
}
log_debug("Successfully connected to system bus.");
return 0;
return;
}
int bus_init_system(Manager *m) {
@@ -905,9 +907,7 @@ int bus_init_system(Manager *m) {
return r;
}
r = bus_setup_system(m, bus);
if (r < 0)
return log_error_errno(r, "Failed to set up system bus: %m");
bus_setup_system(m, bus);
m->system_bus = TAKE_PTR(bus);
@@ -934,22 +934,16 @@ int bus_init_private(Manager *m) {
r = sockaddr_un_set_path(&sa.un, "/run/systemd/private");
} else {
_cleanup_free_ char *joined = NULL;
const char *e;
_cleanup_free_ char *p = NULL;
e = secure_getenv("XDG_RUNTIME_DIR");
if (!e)
return log_error_errno(SYNTHETIC_ERRNO(EHOSTDOWN),
"XDG_RUNTIME_DIR is not set, refusing.");
joined = path_join(e, "/systemd/private");
if (!joined)
p = path_join(m->prefix[EXEC_DIRECTORY_RUNTIME], "systemd/private");
if (!p)
return log_oom();
r = sockaddr_un_set_path(&sa.un, joined);
r = sockaddr_un_set_path(&sa.un, p);
}
if (r < 0)
return log_error_errno(r, "Can't set path for AF_UNIX socket to bind to: %m");
return log_error_errno(r, "Failed set socket path for private bus: %m");
sa_len = r;
(void) mkdir_parents_label(sa.un.sun_path, 0755);
+1 -1
View File
@@ -5,7 +5,7 @@
#include "manager.h"
int bus_send_pending_reload_message(Manager *m);
void bus_send_pending_reload_message(Manager *m);
int bus_init_private(Manager *m);
int bus_init_api(Manager *m);
+16 -18
View File
@@ -54,8 +54,6 @@ static int device_by_path(Manager *m, const char *path, Unit **ret) {
}
static void device_unset_sysfs(Device *d) {
Hashmap *devices;
assert(d);
if (!d->sysfs)
@@ -63,7 +61,7 @@ static void device_unset_sysfs(Device *d) {
/* Remove this unit from the chain of devices which share the same sysfs path. */
devices = UNIT(d)->manager->devices_by_sysfs;
Hashmap *devices = ASSERT_PTR(UNIT(d)->manager->devices_by_sysfs);
if (d->same_sysfs_prev)
/* If this is not the first unit, then simply remove this unit. */
@@ -84,36 +82,35 @@ static void device_unset_sysfs(Device *d) {
}
static int device_set_sysfs(Device *d, const char *sysfs) {
_cleanup_free_ char *copy = NULL;
Device *first;
Unit *u = UNIT(ASSERT_PTR(d));
int r;
assert(d);
if (streq_ptr(d->sysfs, sysfs))
return 0;
r = hashmap_ensure_allocated(&UNIT(d)->manager->devices_by_sysfs, &path_hash_ops);
Hashmap **devices = &u->manager->devices_by_sysfs;
r = hashmap_ensure_allocated(devices, &path_hash_ops);
if (r < 0)
return r;
copy = strdup(sysfs);
_cleanup_free_ char *copy = strdup(sysfs);
if (!copy)
return -ENOMEM;
device_unset_sysfs(d);
first = hashmap_get(UNIT(d)->manager->devices_by_sysfs, sysfs);
Device *first = hashmap_get(*devices, sysfs);
LIST_PREPEND(same_sysfs, first, d);
r = hashmap_replace(UNIT(d)->manager->devices_by_sysfs, copy, first);
r = hashmap_replace(*devices, copy, first);
if (r < 0) {
LIST_REMOVE(same_sysfs, first, d);
return r;
}
d->sysfs = TAKE_PTR(copy);
unit_add_to_dbus_queue(UNIT(d));
unit_add_to_dbus_queue(u);
return 0;
}
@@ -335,7 +332,7 @@ static void device_catchup(Unit *u) {
Device *d = ASSERT_PTR(DEVICE(u));
/* Second, let's update the state with the enumerated state */
device_update_found_one(d, d->enumerated_found, DEVICE_FOUND_MASK);
device_update_found_one(d, d->enumerated_found, _DEVICE_FOUND_MASK);
}
static const struct {
@@ -350,13 +347,14 @@ static const struct {
static int device_found_to_string_many(DeviceFound flags, char **ret) {
_cleanup_free_ char *s = NULL;
assert(flags >= 0);
assert(ret);
for (size_t i = 0; i < ELEMENTSOF(device_found_map); i++) {
if (!FLAGS_SET(flags, device_found_map[i].flag))
FOREACH_ELEMENT(i, device_found_map) {
if (!FLAGS_SET(flags, i->flag))
continue;
if (!strextend_with_separator(&s, ",", device_found_map[i].name))
if (!strextend_with_separator(&s, ",", i->name))
return -ENOMEM;
}
@@ -1099,7 +1097,7 @@ static void device_remove_old_on_move(Manager *m, sd_device *dev) {
if (!syspath_old)
return (void) log_oom();
device_update_found_by_sysfs(m, syspath_old, DEVICE_NOT_FOUND, DEVICE_FOUND_MASK);
device_update_found_by_sysfs(m, syspath_old, DEVICE_NOT_FOUND, _DEVICE_FOUND_MASK);
}
static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *userdata) {
@@ -1179,7 +1177,7 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *
/* If we get notified that a device was removed by udev, then it's completely gone, hence
* unset all found bits. Note this affects all .device units still point to the removed
* device. */
device_update_found_by_sysfs(m, sysfs, DEVICE_NOT_FOUND, DEVICE_FOUND_MASK);
device_update_found_by_sysfs(m, sysfs, DEVICE_NOT_FOUND, _DEVICE_FOUND_MASK);
/* These devices are found and ready now, set the udev found bit. Note, this is also necessary to do
* on remove uevent, as some devlinks may be updated and now point to other device nodes. */
+1 -1
View File
@@ -14,7 +14,7 @@ typedef enum DeviceFound {
DEVICE_FOUND_UDEV = 1 << 0, /* The device has shown up in the udev database */
DEVICE_FOUND_MOUNT = 1 << 1, /* The device has shown up in /proc/self/mountinfo */
DEVICE_FOUND_SWAP = 1 << 2, /* The device has shown up in /proc/swaps */
DEVICE_FOUND_MASK = DEVICE_FOUND_UDEV|DEVICE_FOUND_MOUNT|DEVICE_FOUND_SWAP,
_DEVICE_FOUND_MASK = DEVICE_FOUND_UDEV|DEVICE_FOUND_MOUNT|DEVICE_FOUND_SWAP,
} DeviceFound;
struct Device {
+60 -26
View File
@@ -1081,7 +1081,7 @@ static int manager_setup_notify(Manager *m) {
if (fd < 0)
return log_error_errno(errno, "Failed to allocate notification socket: %m");
fd_increase_rxbuf(fd, NOTIFY_RCVBUF_SIZE);
(void) fd_increase_rxbuf(fd, NOTIFY_RCVBUF_SIZE);
m->notify_socket = path_join(m->prefix[EXEC_DIRECTORY_RUNTIME], "systemd/notify");
if (!m->notify_socket)
@@ -1098,11 +1098,16 @@ static int manager_setup_notify(Manager *m) {
r = mac_selinux_bind(fd, &sa.sa, sa_len);
if (r < 0)
return log_error_errno(r, "bind(%s) failed: %m", m->notify_socket);
return log_error_errno(r, "Failed to bind notify fd to '%s': %m", m->notify_socket);
r = setsockopt_int(fd, SOL_SOCKET, SO_PASSCRED, true);
if (r < 0)
return log_error_errno(r, "SO_PASSCRED failed: %m");
return log_error_errno(r, "Failed to enable SO_PASSCRED for notify socket: %m");
r = setsockopt_int(fd, SOL_SOCKET, SO_PASSPIDFD, true);
if (r < 0 && r != -ENOPROTOOPT)
log_warning_errno(r, "Failed to enable SO_PASSPIDFD for notify socket, ignoring: %m");
// TODO: maybe enforce SO_PASSPIDFD?
m->notify_fd = TAKE_FD(fd);
@@ -2042,7 +2047,7 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds, const char *roo
* containers. */
manager_distribute_fds(m, fds);
/* We might have deserialized the notify fd, but if we didn't then let's create the bus now */
/* We might have deserialized the notify fd, but if we didn't then let's create it now */
r = manager_setup_notify(m);
if (r < 0)
/* No sense to continue without notifications, our children would fail anyway. */
@@ -2638,13 +2643,16 @@ static bool manager_process_barrier_fd(char * const *tags, FDSet *fds) {
static void manager_invoke_notify_message(
Manager *m,
Unit *u,
PidRef *pidref,
const struct ucred *ucred,
char * const *tags,
FDSet *fds) {
assert(m);
assert(u);
assert(pidref_is_set(pidref));
assert(ucred);
assert(pidref->pid == ucred->pid);
assert(tags);
if (u->notifygen == m->notifygen) /* Already invoked on this same unit in this same iteration? */
@@ -2652,7 +2660,7 @@ static void manager_invoke_notify_message(
u->notifygen = m->notifygen;
if (UNIT_VTABLE(u)->notify_message)
UNIT_VTABLE(u)->notify_message(u, ucred, tags, fds);
UNIT_VTABLE(u)->notify_message(u, pidref, ucred, tags, fds);
else if (DEBUG_LOGGING) {
_cleanup_free_ char *joined = strv_join(tags, ", ");
@@ -2718,13 +2726,12 @@ static int manager_get_units_for_pidref(Manager *m, const PidRef *pidref, Unit *
static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata) {
Manager *m = ASSERT_PTR(userdata);
_cleanup_fdset_free_ FDSet *fds = NULL;
char buf[NOTIFY_BUFFER_MAX+1];
struct iovec iovec = {
.iov_base = buf,
.iov_len = sizeof(buf)-1,
};
CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred)) +
CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred)) + CMSG_SPACE(sizeof(int)) /* SCM_PIDFD */ +
CMSG_SPACE(sizeof(int) * NOTIFY_FD_MAX)) control;
struct msghdr msghdr = {
.msg_iov = &iovec,
@@ -2732,13 +2739,8 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
.msg_control = &control,
.msg_controllen = sizeof(control),
};
struct cmsghdr *cmsg;
struct ucred *ucred = NULL;
_cleanup_strv_free_ char **tags = NULL;
int r, *fd_array = NULL;
size_t n_fds = 0;
ssize_t n;
int r;
assert(m->notify_fd == fd);
@@ -2751,7 +2753,7 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
if (ERRNO_IS_NEG_TRANSIENT(n))
return 0; /* Spurious wakeup, try again */
if (n == -EXFULL) {
log_warning("Got message with truncated control data (too many fds sent?), ignoring.");
log_warning_errno(n, "Got message with truncated control data (too many fds sent?), ignoring.");
return 0;
}
if (n < 0)
@@ -2761,20 +2763,32 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
* socket. */
return log_error_errno(n, "Failed to receive notification message: %m");
CMSG_FOREACH(cmsg, &msghdr)
if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS) {
_cleanup_close_ int pidfd = -EBADF;
struct ucred *ucred = NULL;
int *fd_array = NULL;
size_t n_fds = 0;
struct cmsghdr *cmsg;
CMSG_FOREACH(cmsg, &msghdr) {
if (cmsg->cmsg_level != SOL_SOCKET)
continue;
if (cmsg->cmsg_type == SCM_RIGHTS) {
assert(!fd_array);
fd_array = CMSG_TYPED_DATA(cmsg, int);
n_fds = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int);
} else if (cmsg->cmsg_level == SOL_SOCKET &&
cmsg->cmsg_type == SCM_CREDENTIALS &&
} else if (cmsg->cmsg_type == SCM_CREDENTIALS &&
cmsg->cmsg_len == CMSG_LEN(sizeof(struct ucred))) {
assert(!ucred);
ucred = CMSG_TYPED_DATA(cmsg, struct ucred);
} else if (cmsg->cmsg_type == SCM_PIDFD) {
assert(pidfd < 0);
pidfd = *CMSG_TYPED_DATA(cmsg, int);
}
}
_cleanup_fdset_free_ FDSet *fds = NULL;
if (n_fds > 0) {
assert(fd_array);
@@ -2782,7 +2796,7 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
r = fdset_new_array(&fds, fd_array, n_fds);
if (r < 0) {
close_many(fd_array, n_fds);
log_oom();
log_oom_warning();
return 0;
}
}
@@ -2792,6 +2806,28 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
return 0;
}
_cleanup_(pidref_done) PidRef pidref = PIDREF_NULL;
if (pidfd >= 0)
r = pidref_set_pidfd_consume(&pidref, TAKE_FD(pidfd));
else
r = pidref_set_pid(&pidref, ucred->pid);
if (r < 0) {
if (r == -ESRCH)
log_debug_errno(r, "Notify sender died before message is processed. Ignoring.");
else
log_warning_errno(r, "Failed to pin notify sender, ignoring message: %m");
return 0;
}
if (pidref.pid != ucred->pid) {
assert(pidref.fd >= 0);
log_warning("Got SCM_PIDFD for process " PID_FMT " but SCM_CREDENTIALS for process " PID_FMT ". Ignoring.",
pidref.pid, ucred->pid);
return 0;
}
if ((size_t) n >= sizeof(buf) || (msghdr.msg_flags & MSG_TRUNC)) {
log_warning("Received notify message exceeded maximum size. Ignoring.");
return 0;
@@ -2806,9 +2842,10 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
/* Make sure it's NUL-terminated, then parse it to obtain the tags list. */
buf[n] = 0;
tags = strv_split_newlines(buf);
_cleanup_strv_free_ char **tags = strv_split_newlines(buf);
if (!tags) {
log_oom();
log_oom_warning();
return 0;
}
@@ -2821,9 +2858,6 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
/* Increase the generation counter used for filtering out duplicate unit invocations. */
m->notifygen++;
/* Generate lookup key from the PID (we have no pidfd here, after all) */
PidRef pidref = PIDREF_MAKE_FROM_PID(ucred->pid);
/* Notify every unit that might be interested, which might be multiple. */
_cleanup_free_ Unit **array = NULL;
@@ -2838,7 +2872,7 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
/* And now invoke the per-unit callbacks. Note that manager_invoke_notify_message() will handle
* duplicate units – making sure we only invoke each unit's handler once. */
FOREACH_ARRAY(u, array, n_array)
manager_invoke_notify_message(m, *u, ucred, tags, fds);
manager_invoke_notify_message(m, *u, &pidref, ucred, tags, fds);
if (!fdset_isempty(fds))
log_warning("Got extra auxiliary fds with notification message, closing them.");
+29 -19
View File
@@ -4341,46 +4341,55 @@ static void service_force_watchdog(Service *s) {
service_enter_signal(s, SERVICE_STOP_WATCHDOG, SERVICE_FAILURE_WATCHDOG);
}
static bool service_notify_message_authorized(Service *s, pid_t pid) {
static bool service_notify_message_authorized(Service *s, PidRef *pid) {
assert(s);
assert(pid_is_valid(pid));
assert(pidref_is_set(pid));
NotifyAccess notify_access = service_get_notify_access(s);
switch (service_get_notify_access(s)) {
if (notify_access == NOTIFY_NONE) {
case NOTIFY_NONE:
/* Warn level only if no notifications are expected */
log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception is disabled", pid);
log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception is disabled", pid->pid);
return false;
}
if (notify_access == NOTIFY_MAIN && pid != s->main_pid.pid) {
case NOTIFY_ALL:
return true;
case NOTIFY_MAIN:
if (pidref_equal(pid, &s->main_pid))
return true;
if (pidref_is_set(&s->main_pid))
log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid, s->main_pid.pid);
log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid->pid, s->main_pid.pid);
else
log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID which is currently not known", pid);
log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID which is currently not known", pid->pid);
return false;
}
if (notify_access == NOTIFY_EXEC && pid != s->main_pid.pid && pid != s->control_pid.pid) {
case NOTIFY_EXEC:
if (pidref_equal(pid, &s->main_pid) || pidref_equal(pid, &s->control_pid))
return true;
if (pidref_is_set(&s->main_pid) && pidref_is_set(&s->control_pid))
log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT" and control PID "PID_FMT,
pid, s->main_pid.pid, s->control_pid.pid);
pid->pid, s->main_pid.pid, s->control_pid.pid);
else if (pidref_is_set(&s->main_pid))
log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid, s->main_pid.pid);
log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid->pid, s->main_pid.pid);
else if (pidref_is_set(&s->control_pid))
log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for control PID "PID_FMT, pid, s->control_pid.pid);
log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for control PID "PID_FMT, pid->pid, s->control_pid.pid);
else
log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID and control PID which are currently not known", pid);
log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID and control PID which are currently not known", pid->pid);
return false;
}
return true;
default:
assert_not_reached();
}
}
static void service_notify_message(
Unit *u,
PidRef *pidref,
const struct ucred *ucred,
char * const *tags,
FDSet *fds) {
@@ -4388,14 +4397,15 @@ static void service_notify_message(
Service *s = ASSERT_PTR(SERVICE(u));
int r;
assert(pidref_is_set(pidref));
assert(ucred);
if (!service_notify_message_authorized(s, ucred->pid))
if (!service_notify_message_authorized(s, pidref))
return;
if (DEBUG_LOGGING) {
_cleanup_free_ char *cc = strv_join(tags, ", ");
log_unit_debug(u, "Got notification message from PID "PID_FMT": %s", ucred->pid, empty_to_na(cc));
log_unit_debug(u, "Got notification message from PID "PID_FMT": %s", pidref->pid, empty_to_na(cc));
}
usec_t monotonic_usec = USEC_INFINITY;
+1 -1
View File
@@ -630,7 +630,7 @@ typedef struct UnitVTable {
void (*notify_cgroup_oom)(Unit *u, bool managed_oom);
/* Called whenever a process of this unit sends us a message */
void (*notify_message)(Unit *u, const struct ucred *ucred, char * const *tags, FDSet *fds);
void (*notify_message)(Unit *u, PidRef *pidref, const struct ucred *ucred, char * const *tags, FDSet *fds);
/* Called whenever we learn a handoff timestamp */
void (*notify_handoff_timestamp)(Unit *u, const struct ucred *ucred, const dual_timestamp *ts);