From d52b8493c28b72540779896ae5bfed75f9b6f90b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 30 Jun 2023 15:31:41 +0200 Subject: [PATCH 1/4] unit: don't encode literally which unit types to generate audit events for Let's abstract this a bit, and keep this info purely in UnitVTable. --- src/core/manager.c | 15 ++++++--------- src/core/service.c | 4 ++++ src/core/unit.c | 13 ++++++------- src/core/unit.h | 4 ++++ 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 8a081d0056..21d2e092fa 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3350,23 +3350,20 @@ void manager_send_unit_audit(Manager *m, Unit *u, int type, bool success) { if (MANAGER_IS_RELOADING(m)) return; - if (u->type != UNIT_SERVICE) - return; - r = unit_name_to_prefix_and_instance(u->id, &p); if (r < 0) { - log_error_errno(r, "Failed to extract prefix and instance of unit name: %m"); + log_warning_errno(r, "Failed to extract prefix and instance of unit name, ignoring: %m"); return; } msg = strjoina("unit=", p); if (audit_log_user_comm_message(audit_fd, type, msg, "systemd", NULL, NULL, NULL, success) < 0) { - if (errno == EPERM) - /* We aren't allowed to send audit messages? - * Then let's not retry again. */ + if (ERRNO_IS_PRIVILEGE(errno)) { + /* We aren't allowed to send audit messages? Then let's not retry again. */ + log_debug_errno(errno, "Failed to send audit message, closing audit socket: %m"); close_audit_fd(); - else - log_warning_errno(errno, "Failed to send audit message: %m"); + } else + log_warning_errno(errno, "Failed to send audit message, ignoring: %m"); } #endif diff --git a/src/core/service.c b/src/core/service.c index 146b892e46..3f27e28a7d 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -28,6 +28,7 @@ #include "load-fragment.h" #include "log.h" #include "manager.h" +#include "missing_audit.h" #include "open-file.h" #include "parse-util.h" #include "path-util.h" @@ -5166,4 +5167,7 @@ const UnitVTable service_vtable = { }, .can_start = service_can_start, + + .audit_start_message_type = AUDIT_SERVICE_START, + .audit_stop_message_type = AUDIT_SERVICE_STOP, }; diff --git a/src/core/unit.c b/src/core/unit.c index 81467093e7..6e0702a874 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -39,7 +39,6 @@ #include "log.h" #include "logarithm.h" #include "macro.h" -#include "missing_audit.h" #include "mkdir-label.h" #include "path-util.h" #include "process-util.h" @@ -2585,30 +2584,30 @@ static void unit_update_on_console(Unit *u) { static void unit_emit_audit_start(Unit *u) { assert(u); - if (u->type != UNIT_SERVICE) + if (UNIT_VTABLE(u)->audit_start_message_type <= 0) return; /* Write audit record if we have just finished starting up */ - manager_send_unit_audit(u->manager, u, AUDIT_SERVICE_START, true); + manager_send_unit_audit(u->manager, u, UNIT_VTABLE(u)->audit_start_message_type, /* success= */ true); u->in_audit = true; } static void unit_emit_audit_stop(Unit *u, UnitActiveState state) { assert(u); - if (u->type != UNIT_SERVICE) + if (UNIT_VTABLE(u)->audit_start_message_type <= 0) return; if (u->in_audit) { /* Write audit record if we have just finished shutting down */ - manager_send_unit_audit(u->manager, u, AUDIT_SERVICE_STOP, state == UNIT_INACTIVE); + manager_send_unit_audit(u->manager, u, UNIT_VTABLE(u)->audit_stop_message_type, /* success= */ state == UNIT_INACTIVE); u->in_audit = false; } else { /* Hmm, if there was no start record written write it now, so that we always have a nice pair */ - manager_send_unit_audit(u->manager, u, AUDIT_SERVICE_START, state == UNIT_INACTIVE); + manager_send_unit_audit(u->manager, u, UNIT_VTABLE(u)->audit_start_message_type, /* success= */ state == UNIT_INACTIVE); if (state == UNIT_INACTIVE) - manager_send_unit_audit(u->manager, u, AUDIT_SERVICE_STOP, true); + manager_send_unit_audit(u->manager, u, UNIT_VTABLE(u)->audit_stop_message_type, /* success= */ true); } } diff --git a/src/core/unit.h b/src/core/unit.h index 3f1f58d600..20b00799b7 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -782,6 +782,10 @@ typedef struct UnitVTable { /* True if systemd-oomd can monitor and act on this unit's recursive children's cgroups */ bool can_set_managed_oom; + + /* The audit events to generate on start + stop (or 0 if none shall be generated) */ + int audit_start_message_type; + int audit_stop_message_type; } UnitVTable; extern const UnitVTable * const unit_vtable[_UNIT_TYPE_MAX]; From b2bfd121b740d9f47c30118bcc216a5f091fb540 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 30 Jun 2023 15:35:49 +0200 Subject: [PATCH 2/4] pid1: also encode whether to send plymouth notifications in UnitVTable --- src/core/manager.c | 2 +- src/core/mount.c | 2 ++ src/core/service.c | 2 ++ src/core/swap.c | 2 ++ src/core/unit.h | 3 +++ 5 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/core/manager.c b/src/core/manager.c index 21d2e092fa..35b90e2159 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3386,7 +3386,7 @@ void manager_send_unit_plymouth(Manager *m, Unit *u) { if (detect_container() > 0) return; - if (!IN_SET(u->type, UNIT_SERVICE, UNIT_MOUNT, UNIT_SWAP)) + if (!UNIT_VTABLE(u)->notify_plymouth) return; /* We set SOCK_NONBLOCK here so that we rather drop the diff --git a/src/core/mount.c b/src/core/mount.c index 542c39e186..1424ad2bf4 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -2398,4 +2398,6 @@ const UnitVTable mount_vtable = { }, .can_start = mount_can_start, + + .notify_plymouth = true, }; diff --git a/src/core/service.c b/src/core/service.c index 3f27e28a7d..93545a252a 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -5168,6 +5168,8 @@ const UnitVTable service_vtable = { .can_start = service_can_start, + .notify_plymouth = true, + .audit_start_message_type = AUDIT_SERVICE_START, .audit_stop_message_type = AUDIT_SERVICE_STOP, }; diff --git a/src/core/swap.c b/src/core/swap.c index a57176b1a3..73cf320bfd 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -1687,4 +1687,6 @@ const UnitVTable swap_vtable = { }, .can_start = swap_can_start, + + .notify_plymouth = true, }; diff --git a/src/core/unit.h b/src/core/unit.h index 20b00799b7..3a7330c24f 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -783,6 +783,9 @@ typedef struct UnitVTable { /* True if systemd-oomd can monitor and act on this unit's recursive children's cgroups */ bool can_set_managed_oom; + /* If true, we'll notify plymouth about this unit */ + bool notify_plymouth; + /* The audit events to generate on start + stop (or 0 if none shall be generated) */ int audit_start_message_type; int audit_stop_message_type; From 56d83b74d45e0d6936ba44a524fe8b2c2237c833 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 30 Jun 2023 15:50:33 +0200 Subject: [PATCH 3/4] oom: don't encode whether unit types can do oomd hookup a second time We already encode this in UnitVTable, hence use it. Even if it means we'll do some minor extra iterations. --- src/core/core-varlink.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/core/core-varlink.c b/src/core/core-varlink.c index 843271593d..aa4c047418 100644 --- a/src/core/core-varlink.c +++ b/src/core/core-varlink.c @@ -146,7 +146,6 @@ int manager_varlink_send_managed_oom_update(Unit *u) { } static int build_managed_oom_cgroups_json(Manager *m, JsonVariant **ret) { - static const UnitType supported_unit_types[] = { UNIT_SLICE, UNIT_SERVICE, UNIT_SCOPE }; _cleanup_(json_variant_unrefp) JsonVariant *v = NULL, *arr = NULL; int r; @@ -157,8 +156,12 @@ static int build_managed_oom_cgroups_json(Manager *m, JsonVariant **ret) { if (r < 0) return r; - for (size_t i = 0; i < ELEMENTSOF(supported_unit_types); i++) - LIST_FOREACH(units_by_type, u, m->units_by_type[supported_unit_types[i]]) { + for (UnitType t = 0; t < _UNIT_TYPE_MAX; t++) { + + if (!unit_vtable[t]->can_set_managed_oom) + continue; + + LIST_FOREACH(units_by_type, u, m->units_by_type[t]) { CGroupContext *c; if (UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(u))) @@ -186,6 +189,7 @@ static int build_managed_oom_cgroups_json(Manager *m, JsonVariant **ret) { return r; } } + } r = json_build(&v, JSON_BUILD_OBJECT(JSON_BUILD_PAIR("cgroups", JSON_BUILD_VARIANT(arr)))); if (r < 0) From 472619672a731ac7daf28ad8002a03cb080432ad Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 30 Jun 2023 15:56:40 +0200 Subject: [PATCH 4/4] mount: make unit_start() mount ratelimiting check generic Let's move this into a vtable callout, so that unit.c doesn't check for explicit unit types anymore. (This is preparation for a future where we do a similar check for the automount logic, or the swap logic.) --- src/core/mount.c | 10 ++++++++++ src/core/unit.c | 11 ++++++++--- src/core/unit.h | 4 ++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 1424ad2bf4..765c9899ef 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -2299,6 +2299,15 @@ static int mount_can_start(Unit *u) { return 1; } +static int mount_subsystem_ratelimited(Manager *m) { + assert(m); + + if (!m->mount_event_source) + return false; + + return sd_event_source_is_ratelimited(m->mount_event_source); +} + static const char* const mount_exec_command_table[_MOUNT_EXEC_COMMAND_MAX] = { [MOUNT_EXEC_MOUNT] = "ExecMount", [MOUNT_EXEC_UNMOUNT] = "ExecUnmount", @@ -2379,6 +2388,7 @@ const UnitVTable mount_vtable = { .enumerate_perpetual = mount_enumerate_perpetual, .enumerate = mount_enumerate, .shutdown = mount_shutdown, + .subsystem_ratelimited = mount_subsystem_ratelimited, .status_message_formats = { .starting_stopping = { diff --git a/src/core/unit.c b/src/core/unit.c index 6e0702a874..5904c9f4b5 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1920,9 +1920,14 @@ int unit_start(Unit *u, ActivationDetails *details) { assert(u); - /* Let's hold off running start jobs for mount units when /proc/self/mountinfo monitor is rate limited. */ - if (u->type == UNIT_MOUNT && sd_event_source_is_ratelimited(u->manager->mount_event_source)) - return -EAGAIN; + /* Let's hold off running start jobs for mount units when /proc/self/mountinfo monitor is ratelimited. */ + if (UNIT_VTABLE(u)->subsystem_ratelimited) { + r = UNIT_VTABLE(u)->subsystem_ratelimited(u->manager); + if (r < 0) + return r; + if (r > 0) + return -EAGAIN; + } /* If this is already started, then this will succeed. Note that this will even succeed if this unit * is not startable by the user. This is relied on to detect when we need to wait for units and when diff --git a/src/core/unit.h b/src/core/unit.h index 3a7330c24f..c0710299a5 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -756,6 +756,10 @@ typedef struct UnitVTable { * limiting checks to occur before we do anything else. */ int (*can_start)(Unit *u); + /* Returns > 0 if the whole subsystem is ratelimited, and new start operations should not be started + * for this unit type right now. */ + int (*subsystem_ratelimited)(Manager *m); + /* The strings to print in status messages */ UnitStatusMessageFormats status_message_formats;