From: Ryan Wilson Date: Wed, 16 Oct 2024 23:51:17 +0000 (-0700) Subject: core: Refactor ProtectControlGroups= to use enum vs bool X-Git-Tag: v257-rc1~127^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5fe29238289104855fa00cbebd71742d504d6a9f;p=thirdparty%2Fsystemd.git core: Refactor ProtectControlGroups= to use enum vs bool This commit refactors ProtectControlGroups= from using a boolean in the dbus/execute backend to using an enum. There is no functional change but this will allow adding new non-boolean values (e.g. strict, private) a la PrivateHome. --- diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 9e067854280..4ef57137ea6 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1043,6 +1043,21 @@ static int property_get_private_users( return sd_bus_message_append_basic(reply, 'b', &b); } +static int property_get_protect_control_groups( + sd_bus *bus, + const char *path, + const char *interface, + const char *property, + sd_bus_message *reply, + void *userdata, + sd_bus_error *error) { + + ProtectControlGroups *p = ASSERT_PTR(userdata); + int b = *p != PROTECT_CONTROL_GROUPS_NO; + + return sd_bus_message_append_basic(reply, 'b', &b); +} + const sd_bus_vtable bus_exec_vtable[] = { SD_BUS_VTABLE_START(0), SD_BUS_PROPERTY("Environment", "as", NULL, offsetof(ExecContext, environment), SD_BUS_VTABLE_PROPERTY_CONST), @@ -1163,7 +1178,7 @@ const sd_bus_vtable bus_exec_vtable[] = { SD_BUS_PROPERTY("ProtectKernelTunables", "b", bus_property_get_bool, offsetof(ExecContext, protect_kernel_tunables), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("ProtectKernelModules", "b", bus_property_get_bool, offsetof(ExecContext, protect_kernel_modules), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("ProtectKernelLogs", "b", bus_property_get_bool, offsetof(ExecContext, protect_kernel_logs), SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("ProtectControlGroups", "b", bus_property_get_bool, offsetof(ExecContext, protect_control_groups), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("ProtectControlGroups", "b", property_get_protect_control_groups, offsetof(ExecContext, protect_control_groups), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("PrivateNetwork", "b", bus_property_get_bool, offsetof(ExecContext, private_network), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("PrivateUsers", "b", property_get_private_users, offsetof(ExecContext, private_users), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("PrivateUsersEx", "s", property_get_private_users_ex, offsetof(ExecContext, private_users), SD_BUS_VTABLE_PROPERTY_CONST), @@ -1909,6 +1924,21 @@ int bus_exec_context_set_transient_property( return 1; } + if (streq(name, "ProtectControlGroups")) { + int v; + + r = sd_bus_message_read(message, "b", &v); + if (r < 0) + return r; + + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { + c->protect_control_groups = v ? PROTECT_CONTROL_GROUPS_YES : PROTECT_CONTROL_GROUPS_NO; + (void) unit_write_settingf(u, flags, name, "%s=%s", name, yes_no(v)); + } + + return 1; + } + if (streq(name, "PrivateDevices")) return bus_set_transient_bool(u, name, &c->private_devices, message, flags, error); @@ -1960,9 +1990,6 @@ int bus_exec_context_set_transient_property( if (streq(name, "ProtectClock")) return bus_set_transient_bool(u, name, &c->protect_clock, message, flags, error); - if (streq(name, "ProtectControlGroups")) - return bus_set_transient_bool(u, name, &c->protect_control_groups, message, flags, error); - if (streq(name, "CPUSchedulingResetOnFork")) return bus_set_transient_bool(u, name, &c->cpu_sched_reset_on_fork, message, flags, error); diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index c71d6b4c2a4..a0caf9bf893 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -3098,7 +3098,7 @@ static int apply_mount_namespace( /* We need to make the pressure path writable even if /sys/fs/cgroups is made read-only, as the * service will need to write to it in order to start the notifications. */ - if (context->protect_control_groups && memory_pressure_path && !streq(memory_pressure_path, "/dev/null")) { + if (context->protect_control_groups != PROTECT_CONTROL_GROUPS_NO && memory_pressure_path && !streq(memory_pressure_path, "/dev/null")) { read_write_paths_cleanup = strv_copy(context->read_write_paths); if (!read_write_paths_cleanup) return -ENOMEM; @@ -3242,7 +3242,7 @@ static int apply_mount_namespace( * sandbox inside the mount namespace. */ .ignore_protect_paths = !needs_sandboxing && !context->dynamic_user && root_dir, - .protect_control_groups = needs_sandboxing && context->protect_control_groups, + .protect_control_groups = needs_sandboxing ? context->protect_control_groups : PROTECT_CONTROL_GROUPS_NO, .protect_kernel_tunables = needs_sandboxing && context->protect_kernel_tunables, .protect_kernel_modules = needs_sandboxing && context->protect_kernel_modules, .protect_kernel_logs = needs_sandboxing && context->protect_kernel_logs, @@ -3886,7 +3886,7 @@ static bool exec_context_need_unprivileged_private_users( context->protect_kernel_tunables || context->protect_kernel_modules || context->protect_kernel_logs || - context->protect_control_groups || + context->protect_control_groups != PROTECT_CONTROL_GROUPS_NO || context->protect_clock || context->protect_hostname || !strv_isempty(context->read_write_paths) || diff --git a/src/core/execute-serialize.c b/src/core/execute-serialize.c index 1b44c49238c..39369e22b83 100644 --- a/src/core/execute-serialize.c +++ b/src/core/execute-serialize.c @@ -1910,7 +1910,7 @@ static int exec_context_serialize(const ExecContext *c, FILE *f) { if (r < 0) return r; - r = serialize_bool_elide(f, "exec-context-protect-control-groups", c->protect_control_groups); + r = serialize_item(f, "exec-context-protect-control-groups", protect_control_groups_to_string(c->protect_control_groups)); if (r < 0) return r; @@ -2792,7 +2792,7 @@ static int exec_context_deserialize(ExecContext *c, FILE *f) { return r; c->protect_clock = r; } else if ((val = startswith(l, "exec-context-protect-control-groups="))) { - r = parse_boolean(val); + r = protect_control_groups_from_string(val); if (r < 0) return r; c->protect_control_groups = r; diff --git a/src/core/execute.c b/src/core/execute.c index 0c2c278d69c..c1acc992f52 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -259,7 +259,7 @@ bool exec_needs_mount_namespace( context->protect_kernel_tunables || context->protect_kernel_modules || context->protect_kernel_logs || - context->protect_control_groups || + context->protect_control_groups != PROTECT_CONTROL_GROUPS_NO || context->protect_proc != PROTECT_PROC_DEFAULT || context->proc_subset != PROC_SUBSET_ALL || exec_needs_ipc_namespace(context)) @@ -1000,7 +1000,7 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) { prefix, yes_no(c->protect_kernel_modules), prefix, yes_no(c->protect_kernel_logs), prefix, yes_no(c->protect_clock), - prefix, yes_no(c->protect_control_groups), + prefix, protect_control_groups_to_string(c->protect_control_groups), prefix, yes_no(c->private_network), prefix, private_users_to_string(c->private_users), prefix, protect_home_to_string(c->protect_home), diff --git a/src/core/execute.h b/src/core/execute.h index 29c08d48b9d..5fbc196cb70 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -324,7 +324,7 @@ struct ExecContext { bool protect_kernel_modules; bool protect_kernel_logs; bool protect_clock; - bool protect_control_groups; + ProtectControlGroups protect_control_groups; ProtectSystem protect_system; ProtectHome protect_home; bool protect_hostname; diff --git a/src/core/load-fragment-gperf.gperf.in b/src/core/load-fragment-gperf.gperf.in index ea82a578ba0..f5cbb319d78 100644 --- a/src/core/load-fragment-gperf.gperf.in +++ b/src/core/load-fragment-gperf.gperf.in @@ -125,7 +125,7 @@ {{type}}.ProtectKernelModules, config_parse_bool, 0, offsetof({{type}}, exec_context.protect_kernel_modules) {{type}}.ProtectKernelLogs, config_parse_bool, 0, offsetof({{type}}, exec_context.protect_kernel_logs) {{type}}.ProtectClock, config_parse_bool, 0, offsetof({{type}}, exec_context.protect_clock) -{{type}}.ProtectControlGroups, config_parse_bool, 0, offsetof({{type}}, exec_context.protect_control_groups) +{{type}}.ProtectControlGroups, config_parse_protect_control_groups, 0, offsetof({{type}}, exec_context.protect_control_groups) {{type}}.NetworkNamespacePath, config_parse_unit_path_printf, 0, offsetof({{type}}, exec_context.network_namespace_path) {{type}}.IPCNamespacePath, config_parse_unit_path_printf, 0, offsetof({{type}}, exec_context.ipc_namespace_path) {{type}}.LogNamespace, config_parse_log_namespace, 0, offsetof({{type}}, exec_context) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index fa8909e1d11..b059e515ef6 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -135,6 +135,7 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_protect_proc, protect_proc, ProtectProc); DEFINE_CONFIG_PARSE_ENUM(config_parse_proc_subset, proc_subset, ProcSubset); DEFINE_CONFIG_PARSE_ENUM(config_parse_private_tmp, private_tmp, PrivateTmp); DEFINE_CONFIG_PARSE_ENUM(config_parse_private_users, private_users, PrivateUsers); +DEFINE_CONFIG_PARSE_ENUM(config_parse_protect_control_groups, protect_control_groups, ProtectControlGroups); DEFINE_CONFIG_PARSE_ENUM(config_parse_exec_utmp_mode, exec_utmp_mode, ExecUtmpMode); DEFINE_CONFIG_PARSE_ENUM(config_parse_job_mode, job_mode, JobMode); DEFINE_CONFIG_PARSE_ENUM(config_parse_notify_access, notify_access, NotifyAccess); diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h index e8b2eaee52c..9b95f0c24ed 100644 --- a/src/core/load-fragment.h +++ b/src/core/load-fragment.h @@ -114,6 +114,7 @@ CONFIG_PARSER_PROTOTYPE(config_parse_namespace_path_strv); CONFIG_PARSER_PROTOTYPE(config_parse_temporary_filesystems); CONFIG_PARSER_PROTOTYPE(config_parse_private_tmp); CONFIG_PARSER_PROTOTYPE(config_parse_private_users); +CONFIG_PARSER_PROTOTYPE(config_parse_protect_control_groups); CONFIG_PARSER_PROTOTYPE(config_parse_cpu_quota); CONFIG_PARSER_PROTOTYPE(config_parse_allowed_cpuset); CONFIG_PARSER_PROTOTYPE(config_parse_protect_home); diff --git a/src/core/namespace.c b/src/core/namespace.c index c45be457a8e..371d4a237ad 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -199,6 +199,11 @@ static const MountEntry protect_home_yes_table[] = { { "/root", MOUNT_INACCESSIBLE, true }, }; +/* ProtectControlGroups=yes table */ +static const MountEntry protect_control_groups_yes_table[] = { + { "/sys/fs/cgroup", MOUNT_READ_ONLY, false }, +}; + /* ProtectSystem=yes table */ static const MountEntry protect_system_yes_table[] = { { "/usr", MOUNT_READ_ONLY, false }, @@ -727,6 +732,22 @@ static int append_static_mounts(MountList *ml, const MountEntry *mounts, size_t return 0; } +static int append_protect_control_groups(MountList *ml, ProtectControlGroups protect_control_groups, bool ignore_protect) { + assert(ml); + + switch (protect_control_groups) { + + case PROTECT_CONTROL_GROUPS_NO: + return 0; + + case PROTECT_CONTROL_GROUPS_YES: + return append_static_mounts(ml, protect_control_groups_yes_table, ELEMENTSOF(protect_control_groups_yes_table), ignore_protect); + + default: + assert_not_reached(); + } +} + static int append_protect_home(MountList *ml, ProtectHome protect_home, bool ignore_protect) { assert(ml); @@ -1933,7 +1954,7 @@ static bool namespace_parameters_mount_apivfs(const NamespaceParameters *p) { */ return p->mount_apivfs || - p->protect_control_groups || + p->protect_control_groups != PROTECT_CONTROL_GROUPS_NO || p->protect_kernel_tunables || p->protect_proc != PROTECT_PROC_DEFAULT || p->proc_subset != PROC_SUBSET_ALL; @@ -2490,16 +2511,9 @@ int setup_namespace(const NamespaceParameters *p, char **reterr_path) { return r; } - if (p->protect_control_groups) { - MountEntry *me = mount_list_extend(&ml); - if (!me) - return log_oom_debug(); - - *me = (MountEntry) { - .path_const = "/sys/fs/cgroup", - .mode = MOUNT_READ_ONLY, - }; - } + r = append_protect_control_groups(&ml, p->protect_control_groups, false); + if (r < 0) + return r; r = append_protect_home(&ml, p->protect_home, p->ignore_protect_paths); if (r < 0) @@ -3195,6 +3209,13 @@ static const char *const protect_system_table[_PROTECT_SYSTEM_MAX] = { DEFINE_STRING_TABLE_LOOKUP_WITH_BOOLEAN(protect_system, ProtectSystem, PROTECT_SYSTEM_YES); +static const char *const protect_control_groups_table[_PROTECT_CONTROL_GROUPS_MAX] = { + [PROTECT_CONTROL_GROUPS_NO] = "no", + [PROTECT_CONTROL_GROUPS_YES] = "yes", +}; + +DEFINE_STRING_TABLE_LOOKUP_WITH_BOOLEAN(protect_control_groups, ProtectControlGroups, PROTECT_CONTROL_GROUPS_YES); + static const char* const namespace_type_table[] = { [NAMESPACE_MOUNT] = "mnt", [NAMESPACE_CGROUP] = "cgroup", diff --git a/src/core/namespace.h b/src/core/namespace.h index 2c8f7987fe8..17c0dd9e187 100644 --- a/src/core/namespace.h +++ b/src/core/namespace.h @@ -69,6 +69,13 @@ typedef enum PrivateUsers { _PRIVATE_USERS_INVALID = -EINVAL, } PrivateUsers; +typedef enum ProtectControlGroups { + PROTECT_CONTROL_GROUPS_NO, + PROTECT_CONTROL_GROUPS_YES, + _PROTECT_CONTROL_GROUPS_MAX, + _PROTECT_CONTROL_GROUPS_INVALID = -EINVAL, +} ProtectControlGroups; + struct BindMount { char *source; char *destination; @@ -151,7 +158,6 @@ struct NamespaceParameters { bool ignore_protect_paths; - bool protect_control_groups; bool protect_kernel_tunables; bool protect_kernel_modules; bool protect_kernel_logs; @@ -165,6 +171,7 @@ struct NamespaceParameters { bool bind_log_sockets; bool mount_nosuid; + ProtectControlGroups protect_control_groups; ProtectHome protect_home; ProtectSystem protect_system; ProtectProc protect_proc; @@ -210,6 +217,9 @@ PrivateTmp private_tmp_from_string(const char *s) _pure_; const char* private_users_to_string(PrivateUsers i) _const_; PrivateUsers private_users_from_string(const char *s) _pure_; +const char* protect_control_groups_to_string(ProtectControlGroups i) _const_; +ProtectControlGroups protect_control_groups_from_string(const char *s) _pure_; + void bind_mount_free_many(BindMount *b, size_t n); int bind_mount_add(BindMount **b, size_t *n, const BindMount *item);