From 02638280390d75f25efca683c5abd57a65c5a16f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 9 Nov 2017 15:29:34 +0100 Subject: [PATCH] core: rework the Delegate= unit file setting to take a list of controller names Previously it was not possible to select which controllers to enable for a unit where Delegate=yes was set, as all controllers were enabled. With this change, this is made configurable, and thus delegation units can pick specifically what they want to manage themselves, and what they don't care about. --- src/core/cgroup.c | 50 ++++++++++++++------- src/core/cgroup.h | 4 +- src/core/dbus-cgroup.c | 82 +++++++++++++++++++++++++++++++++++ src/core/load-fragment-gperf.gperf.m4 | 2 +- src/core/load-fragment.c | 61 ++++++++++++++++++++++++++ src/core/load-fragment.h | 1 + src/core/unit.c | 21 +++++++-- src/shared/bus-unit-util.c | 47 +++++++++++++++++++- 8 files changed, 246 insertions(+), 22 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index d81d10a..6872da1 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -209,6 +209,16 @@ void cgroup_context_dump(CGroupContext *c, FILE* f, const char *prefix) { prefix, cgroup_device_policy_to_string(c->device_policy), prefix, yes_no(c->delegate)); + if (c->delegate) { + _cleanup_free_ char *t = NULL; + + (void) cg_mask_to_string(c->delegate_controllers, &t); + + fprintf(f, "%sDelegateController=%s\n", + prefix, + strempty(t)); + } + LIST_FOREACH(device_allow, a, c->device_allow) fprintf(f, "%sDeviceAllow=%s %s%s%s\n", @@ -1062,37 +1072,47 @@ CGroupMask unit_get_own_mask(Unit *u) { if (!c) return 0; - /* If delegation is turned on, then turn on all cgroups, - * unless we are on the legacy hierarchy and the process we - * fork into it is known to drop privileges, and hence - * shouldn't get access to the controllers. + return cgroup_context_get_mask(c); +} + +CGroupMask unit_get_delegate_mask(Unit *u) { + CGroupContext *c; + + /* If delegation is turned on, then turn on selected controllers, unless we are on the legacy hierarchy and the + * process we fork into is known to drop privileges, and hence shouldn't get access to the controllers. * - * Note that on the unified hierarchy it is safe to delegate - * controllers to unprivileged services. */ + * Note that on the unified hierarchy it is safe to delegate controllers to unprivileged services. */ - if (c->delegate) { + if (u->type == UNIT_SLICE) + return 0; + + c = unit_get_cgroup_context(u); + if (!c) + return 0; + + if (!c->delegate) + return 0; + + if (cg_all_unified() <= 0) { ExecContext *e; e = unit_get_exec_context(u); - if (!e || - exec_context_maintains_privileges(e) || - cg_all_unified() > 0) - return _CGROUP_MASK_ALL; + if (e && !exec_context_maintains_privileges(e)) + return 0; } - return cgroup_context_get_mask(c); + return c->delegate_controllers; } CGroupMask unit_get_members_mask(Unit *u) { assert(u); - /* Returns the mask of controllers all of the unit's children - * require, merged */ + /* Returns the mask of controllers all of the unit's children require, merged */ if (u->cgroup_members_mask_valid) return u->cgroup_members_mask; - u->cgroup_members_mask = 0; + u->cgroup_members_mask = unit_get_delegate_mask(u); if (u->type == UNIT_SLICE) { void *v; diff --git a/src/core/cgroup.h b/src/core/cgroup.h index 65245fb..a75be38 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -126,6 +126,7 @@ struct CGroupContext { uint64_t tasks_max; bool delegate; + CGroupMask delegate_controllers; }; /* Used when querying IP accounting data */ @@ -153,8 +154,9 @@ void cgroup_context_free_blockio_device_weight(CGroupContext *c, CGroupBlockIODe void cgroup_context_free_blockio_device_bandwidth(CGroupContext *c, CGroupBlockIODeviceBandwidth *b); CGroupMask unit_get_own_mask(Unit *u); -CGroupMask unit_get_siblings_mask(Unit *u); +CGroupMask unit_get_delegate_mask(Unit *u); CGroupMask unit_get_members_mask(Unit *u); +CGroupMask unit_get_siblings_mask(Unit *u); CGroupMask unit_get_subtree_mask(Unit *u); CGroupMask unit_get_target_mask(Unit *u); diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index a99d727..bef70b6 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -32,6 +32,42 @@ static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_cgroup_device_policy, cgroup_device_policy, CGroupDevicePolicy); +static int property_get_delegate_controllers( + sd_bus *bus, + const char *path, + const char *interface, + const char *property, + sd_bus_message *reply, + void *userdata, + sd_bus_error *error) { + + CGroupContext *c = userdata; + CGroupController cc; + int r; + + assert(bus); + assert(reply); + assert(c); + + if (!c->delegate) + return sd_bus_message_append(reply, "as", 0); + + r = sd_bus_message_open_container(reply, 'a', "s"); + if (r < 0) + return r; + + for (cc = 0; cc < _CGROUP_CONTROLLER_MAX; cc++) { + if ((c->delegate_controllers & CGROUP_CONTROLLER_TO_MASK(cc)) == 0) + continue; + + r = sd_bus_message_append(reply, "s", cgroup_controller_to_string(cc)); + if (r < 0) + return r; + } + + return sd_bus_message_close_container(reply); +} + static int property_get_io_device_weight( sd_bus *bus, const char *path, @@ -255,6 +291,7 @@ static int property_get_ip_address_access( const sd_bus_vtable bus_cgroup_vtable[] = { SD_BUS_VTABLE_START(0), SD_BUS_PROPERTY("Delegate", "b", bus_property_get_bool, offsetof(CGroupContext, delegate), 0), + SD_BUS_PROPERTY("DelegateControllers", "as", property_get_delegate_controllers, 0, 0), SD_BUS_PROPERTY("CPUAccounting", "b", bus_property_get_bool, offsetof(CGroupContext, cpu_accounting), 0), SD_BUS_PROPERTY("CPUWeight", "t", NULL, offsetof(CGroupContext, cpu_weight), 0), SD_BUS_PROPERTY("StartupCPUWeight", "t", NULL, offsetof(CGroupContext, startup_cpu_weight), 0), @@ -315,10 +352,55 @@ static int bus_cgroup_set_transient_property( if (mode != UNIT_CHECK) { c->delegate = b; + c->delegate_controllers = b ? _CGROUP_MASK_ALL : 0; + unit_write_drop_in_private(u, mode, name, b ? "Delegate=yes" : "Delegate=no"); } return 1; + + } else if (streq(name, "DelegateControllers")) { + CGroupMask mask = 0; + + r = sd_bus_message_enter_container(message, 'a', "s"); + if (r < 0) + return r; + + for (;;) { + CGroupController cc; + const char *t; + + r = sd_bus_message_read(message, "s", &t); + if (r < 0) + return r; + if (r == 0) + break; + + cc = cgroup_controller_from_string(t); + if (cc < 0) + return sd_bus_error_set_errnof(error, EINVAL, "Unknown cgroup contoller '%s'", t); + + mask |= CGROUP_CONTROLLER_TO_MASK(cc); + } + + r = sd_bus_message_exit_container(message); + if (r < 0) + return r; + + if (mode != UNIT_CHECK) { + _cleanup_free_ char *t = NULL; + + r = cg_mask_to_string(mask, &t); + if (r < 0) + return r; + + c->delegate = true; + c->delegate_controllers |= mask; + + unit_write_drop_in_private_format(u, mode, name, "Delegate=%s", t); + } + + return 1; } return 0; diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index 0f6372e..42c2dbb 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -174,7 +174,7 @@ $1.BlockIOReadBandwidth, config_parse_blockio_bandwidth, 0, $1.BlockIOWriteBandwidth, config_parse_blockio_bandwidth, 0, offsetof($1, cgroup_context) $1.TasksAccounting, config_parse_bool, 0, offsetof($1, cgroup_context.tasks_accounting) $1.TasksMax, config_parse_tasks_max, 0, offsetof($1, cgroup_context.tasks_max) -$1.Delegate, config_parse_bool, 0, offsetof($1, cgroup_context.delegate) +$1.Delegate, config_parse_delegate, 0, offsetof($1, cgroup_context) $1.IPAccounting, config_parse_bool, 0, offsetof($1, cgroup_context.ip_accounting) $1.IPAddressAllow, config_parse_ip_address_access, 0, offsetof($1, cgroup_context.ip_address_allow) $1.IPAddressDeny, config_parse_ip_address_access, 0, offsetof($1, cgroup_context.ip_address_deny) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 53a95ca..02d507f 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3202,6 +3202,67 @@ int config_parse_tasks_max( return 0; } +int config_parse_delegate( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + + CGroupContext *c = data; + int r; + + /* We either accept a boolean value, which may be used to turn on delegation for all controllers, or turn it + * off for all. Or it takes a list of controller names, in which case we add the specified controllers to the + * mask to delegate. */ + + r = parse_boolean(rvalue); + if (r < 0) { + const char *p = rvalue; + CGroupMask mask = 0; + + for (;;) { + _cleanup_free_ char *word = NULL; + CGroupController cc; + + r = extract_first_word(&p, &word, NULL, EXTRACT_QUOTES); + if (r == 0) + break; + if (r == -ENOMEM) + return log_oom(); + if (r < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, "Invalid syntax, ignoring: %s", rvalue); + return r; + } + + cc = cgroup_controller_from_string(word); + if (cc < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, "Invalid controller name '%s', ignoring", rvalue); + continue; + } + + mask |= CGROUP_CONTROLLER_TO_MASK(cc); + } + + c->delegate = true; + c->delegate_controllers |= mask; + + } else if (r > 0) { + c->delegate = true; + c->delegate_controllers = _CGROUP_MASK_ALL; + } else { + c->delegate = false; + c->delegate_controllers = 0; + } + + return 0; +} + int config_parse_device_allow( const char *unit, const char *filename, diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h index 1353b0a..0bd6ec1 100644 --- a/src/core/load-fragment.h +++ b/src/core/load-fragment.h @@ -85,6 +85,7 @@ int config_parse_cpu_weight(const char *unit, const char *filename, unsigned lin int config_parse_cpu_shares(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_memory_limit(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_tasks_max(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); +int config_parse_delegate(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_device_policy(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_device_allow(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_io_weight(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); diff --git a/src/core/unit.c b/src/core/unit.c index 7d95f9d..e244680 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1057,8 +1057,9 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { timespan[FORMAT_TIMESPAN_MAX]; Unit *following; _cleanup_set_free_ Set *following_set = NULL; - int r; const char *n; + CGroupMask m; + int r; assert(u); assert(u->type >= 0); @@ -1105,11 +1106,23 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { if (u->cgroup_realized_mask != 0) { _cleanup_free_ char *s = NULL; (void) cg_mask_to_string(u->cgroup_realized_mask, &s); - fprintf(f, "%s\tCGroup mask: %s\n", prefix, strnull(s)); + fprintf(f, "%s\tCGroup realized mask: %s\n", prefix, strnull(s)); + } + if (u->cgroup_enabled_mask != 0) { + _cleanup_free_ char *s = NULL; + (void) cg_mask_to_string(u->cgroup_enabled_mask, &s); + fprintf(f, "%s\tCGroup enabled mask: %s\n", prefix, strnull(s)); + } + m = unit_get_own_mask(u); + if (m != 0) { + _cleanup_free_ char *s = NULL; + (void) cg_mask_to_string(m, &s); + fprintf(f, "%s\tCGroup own mask: %s\n", prefix, strnull(s)); } - if (u->cgroup_members_mask != 0) { + m = unit_get_members_mask(u); + if (m != 0) { _cleanup_free_ char *s = NULL; - (void) cg_mask_to_string(u->cgroup_members_mask, &s); + (void) cg_mask_to_string(m, &s); fprintf(f, "%s\tCGroup members mask: %s\n", prefix, strnull(s)); } diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 57de997..529bc62 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -183,6 +183,51 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen r = sd_bus_message_append(m, "sv", field, "t", bytes); goto finish; + + } else if (streq(field, "Delegate")) { + + r = parse_boolean(eq); + if (r < 0) { + const char *p = eq; + + r = sd_bus_message_append(m, "s", "DelegateControllers"); + if (r < 0) + goto finish; + + r = sd_bus_message_open_container(m, 'v', "as"); + if (r < 0) + goto finish; + + r = sd_bus_message_open_container(m, 'a', "s"); + if (r < 0) + goto finish; + + for (;;) { + _cleanup_free_ char *word = NULL; + + r = extract_first_word(&p, &word, NULL, EXTRACT_QUOTES); + if (r == 0) + break; + if (r == -ENOMEM) + return log_oom(); + if (r < 0) + return log_error_errno(r, "Invalid syntax: %s", eq); + + r = sd_bus_message_append(m, "s", word); + if (r < 0) + goto finish; + } + + r = sd_bus_message_close_container(m); + if (r < 0) + goto finish; + + r = sd_bus_message_close_container(m); + } else + r = sd_bus_message_append(m, "sv", "Delegate", "b", r); + + goto finish; + } else if (streq(field, "TasksMax")) { uint64_t t; @@ -238,7 +283,7 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen "TasksAccounting", "IPAccounting", "SendSIGHUP", "SendSIGKILL", "WakeSystem", "DefaultDependencies", "IgnoreSIGPIPE", "TTYVHangup", "TTYReset", "TTYVTDisallocate", "RemainAfterExit", "PrivateTmp", "PrivateDevices", "PrivateNetwork", "PrivateUsers", - "NoNewPrivileges", "SyslogLevelPrefix", "Delegate", "RemainAfterElapse", + "NoNewPrivileges", "SyslogLevelPrefix", "RemainAfterElapse", "MemoryDenyWriteExecute", "RestrictRealtime", "DynamicUser", "RemoveIPC", "ProtectKernelTunables", "ProtectKernelModules", "ProtectControlGroups", "MountAPIVFS", "CPUSchedulingResetOnFork", "LockPersonality")) { -- 2.7.4