!83 systemd:fix Failed to migrate controller cgroups from *: Permission denied

From: @openeuler-basic
Reviewed-by: @overweight
Signed-off-by: @overweight
This commit is contained in:
openeuler-ci-bot 2021-03-19 19:37:26 +08:00 committed by Gitee
commit 1fc391add0
3 changed files with 271 additions and 1 deletions

View File

@ -0,0 +1,99 @@
From 81504017f462db1ef4ce2c1f617535f261fe01cc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20Koutn=C3=BD?= <mkoutny@suse.com>
Date: Tue, 26 Jan 2021 17:07:00 +0100
Subject: [PATCH] cgroup: Simplify cg_get_path_and_check
The function controller_is_accessible() doesn't do really much in case
of the unified hierarchy. Move common parts into cg_get_path_and_check
and make controller check v1 specific. This is refactoring only.
---
src/basic/cgroup-util.c | 56 ++++++++++++++++-------------------------
1 file changed, 22 insertions(+), 34 deletions(-)
diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c
index bb20a12294d..fcab1775bd5 100644
--- a/src/basic/cgroup-util.c
+++ b/src/basic/cgroup-util.c
@@ -527,41 +527,16 @@ int cg_get_path(const char *controller, const char *path, const char *suffix, ch
return 0;
}
-static int controller_is_accessible(const char *controller) {
- int r;
+static int controller_is_v1_accessible(const char *controller) {
+ const char *cc, *dn;
assert(controller);
- /* Checks whether a specific controller is accessible,
- * i.e. its hierarchy mounted. In the unified hierarchy all
- * controllers are considered accessible, except for the named
- * hierarchies */
-
- if (!cg_controller_is_valid(controller))
- return -EINVAL;
-
- r = cg_all_unified();
- if (r < 0)
- return r;
- if (r > 0) {
- /* We don't support named hierarchies if we are using
- * the unified hierarchy. */
-
- if (streq(controller, SYSTEMD_CGROUP_CONTROLLER))
- return 0;
-
- if (startswith(controller, "name="))
- return -EOPNOTSUPP;
-
- } else {
- const char *cc, *dn;
-
- dn = controller_to_dirname(controller);
- cc = strjoina("/sys/fs/cgroup/", dn);
+ dn = controller_to_dirname(controller);
+ cc = strjoina("/sys/fs/cgroup/", dn);
- if (laccess(cc, F_OK) < 0)
- return -errno;
- }
+ if (laccess(cc, F_OK) < 0)
+ return -errno;
return 0;
}
@@ -572,10 +547,23 @@ int cg_get_path_and_check(const char *controller, const char *path, const char *
assert(controller);
assert(fs);
- /* Check if the specified controller is actually accessible */
- r = controller_is_accessible(controller);
+ if (!cg_controller_is_valid(controller))
+ return -EINVAL;
+
+ r = cg_all_unified();
if (r < 0)
return r;
+ if (r > 0) {
+ /* In the unified hierarchy all controllers are considered accessible,
+ * except for the named hierarchies */
+ if (startswith(controller, "name="))
+ return -EOPNOTSUPP;
+ } else {
+ /* Check if the specified controller is actually accessible */
+ r = controller_is_v1_accessible(controller);
+ if (r < 0)
+ return r;
+ }
return cg_get_path(controller, path, suffix, fs);
}
@@ -1909,7 +1897,7 @@ int cg_mask_supported(CGroupMask *ret) {
continue;
n = cgroup_controller_to_string(c);
- if (controller_is_accessible(n) >= 0)
+ if (controller_is_v1_accessible(n) >= 0)
mask |= bit;
}
}

View File

@ -0,0 +1,163 @@
From 0fa7b50053695a3012af71c719dd86c12ab10fc6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20Koutn=C3=BD?= <mkoutny@suse.com>
Date: Tue, 26 Jan 2021 19:15:45 +0100
Subject: [PATCH] core: Make (user) instance aware of delegated cgroup
controllers
systemd user instance assumed same controllers are available to it as to
PID 1. That is not true generally, in v1 (legacy, hybrid) we don't delegate any
controllers to anyone and in v2 (unified) we may delegate only subset of
controllers.
The user instance would fail silently when the controller cgroup cannot
be created or the controller cannot be enabled on the unified hierarchy.
The changes in 7b63961415 ("cgroup: Swap cgroup v1 deletion and
migration") caused some attempts of operating on non-delegated
controllers to be logged.
Make the user instance first check what controllers are availble to it
and narrow operations only to these controllers. The original checks are
kept in place.
Note that daemon-reexec needs to be invoked in order to update the set
of unabled controllers after a change.
Fixes: #18047
Fixes: #17862
---
src/basic/cgroup-util.c | 41 +++++++++++++++++++++++++++--------------
src/basic/cgroup-util.h | 1 +
src/core/cgroup.c | 2 +-
3 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c
index 19c0a9d..7ef30f2 100644
--- a/src/basic/cgroup-util.c
+++ b/src/basic/cgroup-util.c
@@ -527,15 +527,21 @@ int cg_get_path(const char *controller, const char *path, const char *suffix, ch
return 0;
}
-static int controller_is_v1_accessible(const char *controller) {
- const char *cc, *dn;
+static int controller_is_v1_accessible(const char *root, const char *controller) {
+ const char *cpath, *dn;
assert(controller);
dn = controller_to_dirname(controller);
- cc = strjoina("/sys/fs/cgroup/", dn);
-
- if (laccess(cc, F_OK) < 0)
+ cpath = strjoina("/sys/fs/cgroup/", dn);
+ if (root)
+ /* Also check that:
+ * - possible subcgroup is created at root,
+ * - we can modify the hierarchy.
+ * "Leak" cpath on stack */
+ cpath = strjoina(cpath, root, "/cgroup.procs");
+
+ if (laccess(cpath, root ? W_OK : F_OK) < 0)
return -errno;
return 0;
@@ -560,7 +566,7 @@ int cg_get_path_and_check(const char *controller, const char *path, const char *
return -EOPNOTSUPP;
} else {
/* Check if the specified controller is actually accessible */
- r = controller_is_v1_accessible(controller);
+ r = controller_is_v1_accessible(NULL, controller);
if (r < 0)
return r;
}
@@ -1829,7 +1835,7 @@ int cg_mask_from_string(const char *value, CGroupMask *ret) {
return 0;
}
-int cg_mask_supported(CGroupMask *ret) {
+int cg_mask_supported_subtree(const char *root, CGroupMask *ret) {
CGroupMask mask;
int r;
@@ -1841,16 +1847,12 @@ int cg_mask_supported(CGroupMask *ret) {
if (r < 0)
return r;
if (r > 0) {
- _cleanup_free_ char *root = NULL, *controllers = NULL, *path = NULL;
+ _cleanup_free_ char *controllers = NULL, *path = NULL;
/* In the unified hierarchy we can read the supported
* and accessible controllers from a the top-level
* cgroup attribute */
- r = cg_get_root_path(&root);
- if (r < 0)
- return r;
-
r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, root, "cgroup.controllers", &path);
if (r < 0)
return r;
@@ -1869,7 +1871,7 @@ int cg_mask_supported(CGroupMask *ret) {
} else {
CGroupController c;
- /* In the legacy hierarchy, we check which hierarchies are mounted. */
+ /* In the legacy hierarchy, we check which hierarchies are accessible. */
mask = 0;
for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) {
@@ -1880,7 +1882,7 @@ int cg_mask_supported(CGroupMask *ret) {
continue;
n = cgroup_controller_to_string(c);
- if (controller_is_v1_accessible(n) >= 0)
+ if (controller_is_v1_accessible(root, n) >= 0)
mask |= bit;
}
}
@@ -1889,6 +1891,17 @@ int cg_mask_supported(CGroupMask *ret) {
return 0;
}
+int cg_mask_supported(CGroupMask *ret) {
+ _cleanup_free_ char *root = NULL;
+ int r;
+
+ r = cg_get_root_path(&root);
+ if (r < 0)
+ return r;
+
+ return cg_mask_supported_subtree(root, ret);
+}
+
int cg_kernel_controllers(Set **ret) {
_cleanup_set_free_free_ Set *controllers = NULL;
_cleanup_fclose_ FILE *f = NULL;
diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h
index 2b88571..2b3b068 100644
--- a/src/basic/cgroup-util.h
+++ b/src/basic/cgroup-util.h
@@ -254,6 +254,7 @@ int cg_slice_to_path(const char *unit, char **ret);
typedef const char* (*cg_migrate_callback_t)(CGroupMask mask, void *userdata);
int cg_mask_supported(CGroupMask *ret);
+int cg_mask_supported_subtree(const char *root, CGroupMask *ret);
int cg_mask_from_string(const char *s, CGroupMask *ret);
int cg_mask_to_string(CGroupMask mask, char **ret);
diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 8b97d15..57b4974 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -3010,7 +3010,7 @@ int manager_setup_cgroup(Manager *m) {
(void) cg_set_attribute("memory", "/", "memory.use_hierarchy", "1");
/* 8. Figure out which controllers are supported */
- r = cg_mask_supported(&m->cgroup_supported);
+ r = cg_mask_supported_subtree(m->cgroup_root, &m->cgroup_supported);
if (r < 0)
return log_error_errno(r, "Failed to determine supported controllers: %m");
--
2.23.0

View File

@ -20,7 +20,7 @@
Name: systemd
Url: https://www.freedesktop.org/wiki/Software/systemd
Version: 246
Release: 13
Release: 14
License: MIT and LGPLv2+ and GPLv2+
Summary: System and Service Manager
@ -71,6 +71,8 @@ Patch0019: core-serialize-u-pids-until-the-processes-have-been-.patch
Patch0020: scope-on-unified-make-sure-to-unwatch-all-PIDs-once-.patch
Patch6000: backport-xdg-autostart-Lower-most-info-messages-to-debug-leve.patch
Patch6001: backport-RFC-Make-user-instance-aware-of-delegated-cgroup-controllers.patch
Patch6002: backport-core-Make-user-instance-aware-of-delegated-cgroup.patch
BuildRequires: gcc, gcc-c++
BuildRequires: libcap-devel, libmount-devel, pam-devel, libselinux-devel
@ -1486,6 +1488,12 @@ fi
%exclude /usr/share/man/man3/*
%changelog
* Wed Mar 3 2021 shenyangyang <shenyangyang4@huawei.com> - 246-14
- Type:bugfix
- ID:NA
- SUG:NA
- DESC:fix Failed to migrate controller cgroups from *: Permission denied
* Sat Feb 27 2021 shenyangyang <shenyangyang4@huawei.com> - 246-13
- Type:bugfix
- ID:NA