cgroup: introduce cgroup.kill

From: Christian Brauner <christian.brauner-GeWIH/[email protected]>

Introduce the cgroup.kill file. It does what it says on the tin and allows a caller to kill a cgroup by writing "1" into cgroup.kill. The file is available in non-root cgroups.

Killing cgroups is a process directed operation, i.e. the whole thread-group is affected. Consequently trying to write to cgroup.kill in threaded cgroups will be rejected and EOPNOTSUPP returned. This behavior aligns with cgroup.procs where reads in threaded-cgroups are rejected with EOPNOTSUPP.

The cgroup.kill file is write-only since killing a cgroup is an event not which makes it different from e.g. freezer where a cgroup transitions between the two states.

As with all new cgroup features cgroup.kill is recursive by default.

Killing a cgroup is protected against concurrent migrations through the cgroup mutex. To protect against forkbombs and to mitigate the effect of racing forks a new CGRP_KILL css set lock protected flag is introduced that is set prior to killing a cgroup and unset after the cgroup has been killed. We can then check in cgroup_post_fork() where we hold the css set lock already whether the cgroup is currently being killed. If so we send the child a SIGKILL signal immediately taking it down as soon as it returns to userspace. To make the killing of the child semantically clean it is killed after all cgroup attachment operations have been finalized.

There are various use-cases of this interface:
- Containers usually have a conservative layout where each container usually has a delegated cgroup. For such layouts there is a 1:1 mapping between container and cgroup. If the container in addition uses a separate pid namespace then killing a container usually becomes a simple kill -9 <container-init-pid> from an ancestor pid namespace. However, there are quite a few scenarios where that isn't true. For example, there are containers that share the cgroup with other processes on purpose that are supposed to be bound to the lifetime of the container but are not in the same pidns of the container. Containers that are in a delegated cgroup but share the pid namespace with the host or other containers.
- Service managers such as systemd use cgroups to group and organize processes belonging to a service. They usually rely on a recursive algorithm now to kill a service. With cgroup.kill this becomes a simple write to cgroup.kill.
- Userspace OOM implementations can make good use of this feature to efficiently take down whole cgroups quickly.
- The kill program can gain a new
kill --cgroup /sys/fs/cgroup/delegated
flag to take down cgroups.

A few observations about the semantics:
- If parent and child are in the same cgroup and CLONE_INTO_CGROUP is not specified we are not taking cgroup mutex meaning the cgroup can be killed while a process in that cgroup is forking.
If the kill request happens right before cgroup_can_fork() and before the parent grabs its siglock the parent is guaranteed to see the pending SIGKILL. In addition we perform another check in cgroup_post_fork() whether the cgroup is being killed and is so take down the child (see above). This is robust enough and protects gainst forkbombs. If userspace really really wants to have stricter protection the simple solution would be to grab the write side of the cgroup threadgroup rwsem which will force all ongoing forks to complete before killing starts. We concluded that this is not necessary as the semantics for concurrent forking should simply align with freezer where a similar check as cgroup_post_fork() is performed.

For all other cases CLONE_INTO_CGROUP is required. In this case we will grab the cgroup mutex so the cgroup can't be killed while we fork. Once we're done with the fork and have dropped cgroup mutex we are visible and will be found by any subsequent kill request.
- We obviously don't kill kthreads. This means a cgroup that has a kthread will not become empty after killing and consequently no unpopulated event will be generated. The assumption is that kthreads should be in the root cgroup only anyway so this is not an issue.
- We skip killing tasks that already have pending fatal signals.
- Freezer doesn't care about tasks in different pid namespaces, i.e. if you have two tasks in different pid namespaces the cgroup would still be frozen. The cgroup.kill mechanism consequently behaves the same way, i.e. we kill all processes and ignore in which pid namespace they exist.
- If the caller is located in a cgroup that is killed the caller will obviously be killed as well.

---

The series can be pulled from

git-OoYKEaZ2EDaWaY/[email protected]:pub/scm/linux/kernel/git/brauner/linux tags/cgroup.kill.v5.14

/* v2 */
- Roman Gushchin <[email protected]>:
- Retrieve cgrp->flags only once and check CGRP_* bits on it.

/* v3 */
- Roman Gushchin <[email protected]>:
- Optimize a bit by adding unlikely() to freeze and kill branches in

    cgroup_post_fork().

- "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>/ Roman Gushchin <[email protected]>:
- Use do_send_sig_info() instead of send_sig() to clearly indicate that this is a thread-group directed signal not a thread directed signal operation.

---
 include/linux/cgroup-defs.h |   3 +
 kernel/cgroup/cgroup.c      | 127 ++++++++++++++++++++++++++++++++----
 2 files changed, 116 insertions(+), 14 deletions(-)

index 559ee05f86b2..43fef771009a 100644
@@ -71,6 +71,9 @@ enum {
 
 	/* Cgroup is frozen. */
 	CGRP_FROZEN,
+
+	/* Control group has to be killed. */
+	CGRP_KILL,
 };
 
 /* cgroup_root->flags */

base-commit: 9f4ad9e425a1d3b6a34617b8ea226d56a119a717

2.27.0

From: Christian Brauner <christian.brauner-GeWIH/[email protected]>

Give a brief overview of the cgroup.kill functionality.

---
/* v2 */
- Roman Gushchin <[email protected]>:
- Drop sentence that mentions combined useage of cgroup.kill and cgroup.freezer.

/* v3 */
unchanged

---
 Documentation/admin-guide/cgroup-v2.rst | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

index 64c62b979f2f..6adc749d863e 100644
@@ -949,6 +949,21 @@ All cgroup core files are prefixed with "cgroup."
 	it's possible to delete a frozen (and empty) cgroup, as well as
 	create new sub-cgroups.
 
+  cgroup.kill
+	A write-only single value file which exists in non-root cgroups.
+	The only allowed value is "1".
+
+	Writing "1" to the file causes the cgroup and all descendant cgroups to
+	be killed. This means that all processes located in the affected cgroup
+	tree will be killed via SIGKILL.
+
+	Killing a cgroup tree will deal with concurrent forks appropriately and
+	is protected against migrations.
+
+	In a threaded cgroup, writing this file fails with EOPNOTSUPP as
+	killing cgroups is a process directed operation, i.e. it affects
+	the whole thread-group.
+
 Controllers
 ===========
 

2.27.0

From: Christian Brauner <christian.brauner-GeWIH/[email protected]>

If cgroup.kill file is supported make use of it.

---
/* v2 */
- Roman Gushchin <[email protected]>:
- Fix whitespace.

/* v3 */
unchanged

---
 tools/testing/selftests/cgroup/cgroup_util.c | 4 ++++
 1 file changed, 4 insertions(+)

index 027014662fb2..f60f7d764690 100644
@@ -252,6 +252,10 @@ int cg_killall(const char *cgroup)
 	char buf[PAGE_SIZE];
 	char *ptr = buf;
 
+	/* If cgroup.kill exists use it. */
+	if (!cg_write(cgroup, "cgroup.kill", "1"))
+		return 0;
+
 	if (cg_read(cgroup, "cgroup.procs", buf, sizeof(buf)))
 		return -1;
 

2.27.0

From: Christian Brauner <christian.brauner-GeWIH/[email protected]>

as they will be used by the tests for cgroup killing.

---
/* v2 */
unchanged

/* v3 */
unchanged

---
 tools/testing/selftests/cgroup/cgroup_util.c  | 47 +++++++++++++++
 tools/testing/selftests/cgroup/cgroup_util.h  |  2 +
 tools/testing/selftests/cgroup/test_freezer.c | 57 -------------------
 3 files changed, 49 insertions(+), 57 deletions(-)

index f60f7d764690..623cec04ad42 100644
@@ -5,10 +5,12 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/limits.h>
+#include <poll.h>
 #include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/inotify.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>

@@ -580,3 +582,48 @@ int clone_into_cgroup_run_wait(const char *cgroup)
 	(void)clone_reap(pid, WEXITED);
 	return 0;
 }
+
+int cg_prepare_for_wait(const char *cgroup)
+{
+	int fd, ret = -1;
+
+	fd = inotify_init1(0);
+	if (fd == -1)
+		return fd;
+
+	ret = inotify_add_watch(fd, cg_control(cgroup, "cgroup.events"),
+				IN_MODIFY);
+	if (ret == -1) {
+		close(fd);
+		fd = -1;
+	}
+
+	return fd;
+}
+
+int cg_wait_for(int fd)
+{
+	int ret = -1;
+	struct pollfd fds = {
+		.fd = fd,
+		.events = POLLIN,
+	};
+
+	while (true) {
+		ret = poll(&fds, 1, 10000);
+
+		if (ret == -1) {
+			if (errno == EINTR)
+				continue;
+
+			break;
+		}
+
+		if (ret > 0 && fds.revents & POLLIN) {
+			ret = 0;
+			break;
+		}
+	}
+
+	return ret;
+}

index 5a1305dd1f0b..82e59cdf16e7 100644
@@ -54,3 +54,5 @@ extern pid_t clone_into_cgroup(int cgroup_fd);
 extern int clone_reap(pid_t pid, int options);
 extern int clone_into_cgroup_run_wait(const char *cgroup);
 extern int dirfd_open_opath(const char *dir);
+extern int cg_prepare_for_wait(const char *cgroup);
+extern int cg_wait_for(int fd);

2.27.0

From: Christian Brauner <christian.brauner-GeWIH/[email protected]>

Test that the new cgroup.kill feature works as intended.

---
/* v2 */
- Christian Brauner <christian.brauner-GeWIH/[email protected]>:
- Remove debug logging (It wasn't very helpful in the first place.).

/* v3 */
- Shakeel Butt <[email protected]>:
- Remove misplaced second cgroup.kill since it is immediately followed by a cg_kill_wait() call.

- Christian Brauner <christian.brauner-GeWIH/[email protected]>:
- Move check for unpopulated == 0 after all killed processes have been reaped to avoid spurious failures when running test_kill in loops (e.g. during stress test).

---
 tools/testing/selftests/cgroup/.gitignore  |   3 +-
 tools/testing/selftests/cgroup/Makefile    |   2 +
 tools/testing/selftests/cgroup/test_kill.c | 297 +++++++++++++++++++++
 3 files changed, 301 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/cgroup/test_kill.c

index 84cfcabea838..be9643ef6285 100644
@@ -2,4 +2,5 @@
 test_memcontrol
 test_core
 test_freezer
-test_kmem
\ No newline at end of file
+test_kmem
+test_kill

index f027d933595b..59e222460581 100644
@@ -9,6 +9,7 @@ TEST_GEN_PROGS = test_memcontrol
 TEST_GEN_PROGS += test_kmem
 TEST_GEN_PROGS += test_core
 TEST_GEN_PROGS += test_freezer
+TEST_GEN_PROGS += test_kill
 
 include ../lib.mk
 

@@ -16,3 +17,4 @@ $(OUTPUT)/test_memcontrol: cgroup_util.c ../clone3/clone3_selftests.h
 $(OUTPUT)/test_kmem: cgroup_util.c ../clone3/clone3_selftests.h
 $(OUTPUT)/test_core: cgroup_util.c ../clone3/clone3_selftests.h
 $(OUTPUT)/test_freezer: cgroup_util.c ../clone3/clone3_selftests.h
+$(OUTPUT)/test_kill: cgroup_util.c ../clone3/clone3_selftests.h ../pidfd/pidfd.h

2.27.0

Applied to cgroup/for-5.14.

Thanks.

tejun

Thanks!
Christian

So I feel like I'm missing something fundamental here...perhaps somebody can supply a suitable cluebat. It seems inevitable to me that, sooner or later, somebody will surely wish that this mechanism could send a signal other than SIGKILL, but this interface won't allow that. Even if you won't want to implement an arbitrary signal now, it seems like defining the interface to require writing "9" rather than "1" would avoid closing that option off in the future.

I assume there's some reason why you don't want to do that, but I'm to slow to figure out what it is...?

Thanks,

jon

I think the consensus was that most signals are a process/thread-level thing and are rarely sent to more than one process, so sending them to all processes in a cgroup is never required. So far nobody came with any real-life use case behind SIGKILL. SIGSTOP/SIGCONT could be that case, but the cgroup v2 freezer is doing virtually the same and eliminates this need.

Even SIGTERM which is often used before SIGKILL as a grateful termination attempt has to be sent to a proper process in the cgroup (e.g. container-level init or the main process in the workload).

If a new case will arise, it will be possible to introduce a new interface file, but it looks very unlikely that there will be many such cases to justify a more generic interface. On the other side keeping it boolean makes it more consistent with the rest of the cgroup v2 api.

Thanks!