[RFC PATCH v2 bpf-next 0/3] bpf: cgroup: support writing and freezing cgroups from BPF

[RFC PATCH v2 bpf-next 0/3] bpf: cgroup: support writing and freezing cgroups from BPF

[Djalal Harouni]

This patch series add support to write cgroup interfaces from BPF.

It is useful to freeze a cgroup hierarchy on suspicious activity for
a more thorough analysis before killing it. Planned users of this
feature are: systemd and BPF tools where the cgroup hierarchy could
be a system service, user session, k8s pod or a container.

The writing happens via kernfs nodes and the cgroup must be on the
default hierarchy. It implements the requests and feedback from v1 [1]
where now we use a unified path for cgroup user space and BPF writing.

So I want to validate that this is the right approach first.

Todo:
* Limit size of data to be written.
* Further tests.
* Add cgroup kill support.


# RFC v1 -> v2

* Implemented Alexei and Tejun requests [1].
* Unified path where user space or BPF writing end up taking directly
  a kernfs_node with an example on the "cgroup.freeze" interface.

[1] https://lore.kernel.org/bpf/20240327225334.58474-1-tixxdz@gmail.com/


Djalal Harouni (3):
      kernfs: cgroup: support writing cgroup interfaces from a kernfs node
      bpf: cgroup: Add BPF Kfunc to write cgroup interfaces
      selftests/bpf: add selftest for bpf_cgroup_write_interface

 include/linux/cgroup.h                                      |   3 ++
 kernel/bpf/helpers.c                                        |  45 +++++
 kernel/cgroup/cgroup.c                                      | 102 +++++++
 tools/testing/selftests/bpf/prog_tests/task_freeze_cgroup.c | 172 ++++++++++++
 tools/testing/selftests/bpf/progs/test_task_freeze_cgroup.c | 155 ++++++++++
 5 files changed, 471 insertions(+), 6 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/task_freeze_cgroup.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_task_freeze_cgroup.c

-- 
2.34.1

[RFC PATCH v2 bpf-next 1/3] kernfs: cgroup: support writing cgroup interfaces from a kernfs node

[Djalal Harouni]

Freezing a cgroup of a task from BPF is better than user space which
could be too late and is subject to races. To achieve this allow writing to
cgroup core interfaces from BPF by adding a new kfunc helper that take a
kernfs node directly.

Currently only writing to "cgroup.freeze" on the default hierarchy is
allowed. The writing goes directly via a kernfs_node which allows to
share the same path as if a kernfs_node was opened from userspace.

Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
---
 include/linux/cgroup.h |   3 ++
 kernel/cgroup/cgroup.c | 102 ++++++++++++++++++++++++++++++++++++++---
 2 files changed, 99 insertions(+), 6 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index b18fb5fcb38e..03a0782c94bf 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -125,6 +125,9 @@ int cgroup_rm_cftypes(struct cftype *cfts);
 void cgroup_file_notify(struct cgroup_file *cfile);
 void cgroup_file_show(struct cgroup_file *cfile, bool show);
 
+ssize_t cgroup_kn_interface_write(struct kernfs_node *kn, const char *name__str,
+				  const char *buf, size_t nbytes, loff_t off);
+
 int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry);
 int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 		     struct pid *pid, struct task_struct *tsk);
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 312c6a8b55bb..cddd7c1d354d 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -229,6 +229,24 @@ static struct file_system_type cgroup2_fs_type;
 static struct cftype cgroup_base_files[];
 static struct cftype cgroup_psi_files[];
 
+struct cgroup_kn_cftype {
+	char name[MAX_CFTYPE_NAME];
+	unsigned int namelen;
+
+	/*
+	 * write() is the write operation on a kernfs node.
+	 */
+	ssize_t (*write)(struct kernfs_node *kn, const char *buf, size_t nbytes,
+			 loff_t off, bool revalidate);
+};
+
+#define CGROUP_PREFIX "cgroup."
+#define CGROUP_CORE_INTERFACE_FREEZE_SUFFIX "freeze"
+#define CGROUP_CORE_INTERFACE_FREEZE (CGROUP_PREFIX CGROUP_CORE_INTERFACE_FREEZE_SUFFIX)
+#define CGROUP_CORE_INTERFACE_FREEZE_LEN (sizeof(CGROUP_CORE_INTERFACE_FREEZE) - 1)
+
+static struct cgroup_kn_cftype kn_cfts[];
+
 /* cgroup optional features */
 enum cgroup_opt_features {
 #ifdef CONFIG_PSI
@@ -4030,29 +4048,58 @@ static int cgroup_freeze_show(struct seq_file *seq, void *v)
 	return 0;
 }
 
-static ssize_t cgroup_freeze_write(struct kernfs_open_file *of,
-				   char *buf, size_t nbytes, loff_t off)
+static bool cgroup_kn_revalidate(struct cgroup *cgrp)
+{
+	if (!cgroup_on_dfl(cgrp) || !cgroup_parent(cgrp))
+		return false;
+
+	return true;
+}
+
+static ssize_t cgroup_kn_freeze(struct kernfs_node *kn,
+				const char *buf, size_t nbytes, loff_t off,
+				bool revalidate)
 {
 	struct cgroup *cgrp;
 	ssize_t ret;
 	int freeze;
+	char b[4] = {0};
+
+	/* Handle userspace writes +(0|1)\n and fail otherwise */
+	ret = strscpy(b, buf, sizeof(b));
+	if (ret < 0)
+		return ret;
 
-	ret = kstrtoint(strstrip(buf), 0, &freeze);
+	nbytes = ret;
+	ret = kstrtoint(strstrip(b), 0, &freeze);
 	if (ret)
 		return ret;
 
 	if (freeze < 0 || freeze > 1)
 		return -ERANGE;
 
-	cgrp = cgroup_kn_lock_live(of->kn, false);
+	cgrp = cgroup_kn_lock_live(kn, false);
 	if (!cgrp)
 		return -ENOENT;
 
+	if (revalidate && !cgroup_kn_revalidate(cgrp)) {
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
 	cgroup_freeze(cgrp, freeze);
 
-	cgroup_kn_unlock(of->kn);
+	ret = nbytes;
 
-	return nbytes;
+out:
+	cgroup_kn_unlock(kn);
+	return ret;
+}
+
+static ssize_t cgroup_freeze_write(struct kernfs_open_file *of,
+				   char *buf, size_t nbytes, loff_t off)
+{
+	return cgroup_kn_freeze(of->kn, buf, nbytes, off, false);
 }
 
 static void __cgroup_kill(struct cgroup *cgrp)
@@ -4601,6 +4648,49 @@ void cgroup_file_show(struct cgroup_file *cfile, bool show)
 	kernfs_put(kn);
 }
 
+static struct cgroup_kn_cftype kn_cfts[] = {
+	{
+		.name = CGROUP_CORE_INTERFACE_FREEZE,
+		.namelen = CGROUP_CORE_INTERFACE_FREEZE_LEN,
+		.write = cgroup_kn_freeze,
+	},
+	{ },
+};
+
+static const struct cgroup_kn_cftype *cgroup_kn_cft(const char *name__str)
+{
+	struct cgroup_kn_cftype *kn_cft;
+
+	for (kn_cft = kn_cfts; kn_cft && kn_cft->name[0] != '\0'; kn_cft++) {
+		if (!strncmp(name__str, kn_cft->name, kn_cft->namelen))
+			return kn_cft;
+	}
+
+	return ERR_PTR(-EOPNOTSUPP);
+}
+
+ssize_t cgroup_kn_interface_write(struct kernfs_node *kn, const char *name__str,
+				  const char *buf, size_t nbytes, loff_t off)
+{
+	const struct cgroup_kn_cftype *kn_cft;
+
+	/* empty, do not remove */
+	if (!nbytes)
+		return 0;
+
+	if (kernfs_type(kn) != KERNFS_DIR)
+		return -ENOTDIR;
+
+	kn_cft = cgroup_kn_cft(name__str);
+	if (IS_ERR(kn_cft))
+		return PTR_ERR(kn_cft);
+
+	if (unlikely(!kn_cft->write))
+		return -EOPNOTSUPP;
+
+	return kn_cft->write(kn, buf, nbytes, off, true);
+}
+
 /**
  * css_next_child - find the next child of a given css
  * @pos: the current position (%NULL to initiate traversal)
-- 
2.43.0

[RFC PATCH v2 bpf-next 2/3] bpf: cgroup: Add BPF Kfunc to write and freeze a cgroup

[Djalal Harouni]

Add bpf_cgroup_write_interface() kfunc that writes to a cgroup
interface. Takes a cgroup on the default hierarchy as argument, and
writes to the specified interface file of that cgroup.

Freezing a cgroup of a task from BPF is better than user space
which could be too late and is subject to races. Hence, add support
for writing to "cgroup.freeze" interface using the mentioned bpf kfunc.

Planned users of this feature are: systemd and BPF tools.
Taking the freezing example, we could freeze a cgroup hierarchy on
suspicious activity for a more thorough analysis. The cgroup hierarchies
could be system services, user sessions, K8s pods or containers.

Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
---
 kernel/bpf/helpers.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 6b4877e85a68..5efc1bc57db9 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2605,6 +2605,50 @@ bpf_task_get_cgroup1(struct task_struct *task, int hierarchy_id)
 		return NULL;
 	return cgrp;
 }
+
+#define BPF_CGROUP_MAX_WRITE	((1UL << 24) - 1)
+
+/**
+ * bpf_cgroup_write_interface - Writes to a cgroup interface file.
+ * @cgrp: The target cgroup
+ * @name__str: name of the cgroup core interface file
+ * @value_p: value to write
+ * @off: offset
+ *
+ * Return: number of bytes written on success, a negative value on error.
+ */
+__bpf_kfunc int
+bpf_cgroup_write_interface(struct cgroup *cgrp, const char *name__str,
+			   const struct bpf_dynptr *value_p, loff_t off)
+{
+	struct bpf_dynptr_kern *value_ptr = (struct bpf_dynptr_kern *)value_p;
+	struct kernfs_node *kn;
+	const void *value;
+	u32 value_len;
+	int ret;
+
+	value_len = __bpf_dynptr_size(value_ptr);
+	if (!value_len)
+		return 0;
+
+	if (value_len > BPF_CGROUP_MAX_WRITE)
+		return -E2BIG;
+
+	value = __bpf_dynptr_data(value_ptr, value_len);
+	if (!value)
+		return -EINVAL;
+
+	rcu_read_lock();
+	kn = cgrp->kn;
+	rcu_read_unlock();
+
+	kernfs_get(kn);
+	ret = cgroup_kn_interface_write(kn, name__str, value, value_len, off);
+	kernfs_put(kn);
+
+	return ret;
+}
+
 #endif /* CONFIG_CGROUPS */
 
 /**
@@ -3736,6 +3780,7 @@ BTF_ID_FLAGS(func, bpf_cgroup_ancestor, KF_ACQUIRE | KF_RCU | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_cgroup_from_id, KF_ACQUIRE | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_task_under_cgroup, KF_RCU)
 BTF_ID_FLAGS(func, bpf_task_get_cgroup1, KF_ACQUIRE | KF_RCU | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_cgroup_write_interface, KF_TRUSTED_ARGS | KF_SLEEPABLE)
 #endif
 BTF_ID_FLAGS(func, bpf_task_from_pid, KF_ACQUIRE | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_task_from_vpid, KF_ACQUIRE | KF_RET_NULL)
-- 
2.43.0

[RFC PATCH v2 bpf-next 3/3] selftests/bpf: add selftest for bpf_cgroup_write_interface

[Djalal Harouni]

This adds a selftest for `bpf_cgroup_write_interface` kfunc. The test works
by forking a child then:

1. Child:
 - Migrate to a new cgroup
 - Loads bpf programs
 - Trigger the 'lsm_freeze_cgroup' bpf program so it freeze itself.

   <- wait for parent to unthaw

 - On unthaw it continues, forks another process and triggers the
   'tp_newchild' bpf program to set some monitored pids of the new
   process, that assert that the user space resumed correctly.

2. Parent:
 - Keeps reading the 'cgroup.freeze' file of the child cgroup until
   it prints 1 which means the child cgroup is frozen.
 - Attaches the sample 'lsm_task_free' so it triggers the bpf program
   to unthaw the child task cgroup.
 - Then waits for a clean exit of the child process.

The scenario allows to test multiple sides of: freeze and unthaw a cgroup.

Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
---
 .../bpf/prog_tests/task_freeze_cgroup.c       | 172 ++++++++++++++++++
 .../bpf/progs/test_task_freeze_cgroup.c       | 155 ++++++++++++++++
 2 files changed, 327 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/task_freeze_cgroup.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_task_freeze_cgroup.c

diff --git a/tools/testing/selftests/bpf/prog_tests/task_freeze_cgroup.c b/tools/testing/selftests/bpf/prog_tests/task_freeze_cgroup.c
new file mode 100644
index 000000000000..d4e9c0f32196
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/task_freeze_cgroup.c
@@ -0,0 +1,172 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <sys/syscall.h>
+#include <test_progs.h>
+#include <cgroup_helpers.h>
+#include <unistd.h>
+#include "test_task_freeze_cgroup.skel.h"
+
+#define CGROUP_PATH	"/test-task-freeze-cgroup"
+
+static int bpf_sleepable(struct test_task_freeze_cgroup *skel)
+{
+	int err, cgroup_fd;
+	pid_t new_pid2;
+
+	cgroup_fd = cgroup_setup_and_join(CGROUP_PATH);
+	if (!ASSERT_OK(cgroup_fd < 0, "cgroup_setup_and_join"))
+		return -errno;
+
+	skel = test_task_freeze_cgroup__open();
+	if (!ASSERT_OK_PTR(skel, "test_task_freeze_cgroup__open")) {
+		err = -errno;
+		goto cleanup_cgroup;
+	}
+
+	skel->rodata->parent_pid = getppid();
+	skel->rodata->monitor_pid = getpid();
+	skel->rodata->cgid = get_cgroup_id(CGROUP_PATH);
+	skel->bss->new_pid = getpid();
+	skel->bss->freeze = 1;
+
+	err = test_task_freeze_cgroup__load(skel);
+	if (!ASSERT_OK(err, "test_task_freeze_cgroup__load")) {
+		err = -errno;
+		goto cleanup_skel;
+	}
+
+	/* First attach the LSM Program that is triggered on bpf() calls
+	 * especially on TP_BTF programs when attached.
+	 */
+	skel->links.lsm_freeze_cgroup =
+		bpf_program__attach_lsm(skel->progs.lsm_freeze_cgroup);
+	if (!ASSERT_OK_PTR(skel->links.lsm_freeze_cgroup, "attach_lsm")) {
+		err = -errno;
+		goto cleanup_detach;
+	}
+
+	/* Attaching this must fail with -EPERM and freeze current task */
+	skel->links.tp_newchild =
+		bpf_program__attach_trace(skel->progs.tp_newchild);
+	if (!ASSERT_EQ(errno, EPERM, "attach_trace() must fail here")) {
+		err = -EINVAL;
+		goto cleanup_detach;
+	}
+
+	/* Continue */
+
+	/* Attach again now with success */
+	skel->links.tp_newchild =
+		bpf_program__attach_trace(skel->progs.tp_newchild);
+	if (!ASSERT_OK_PTR(skel->links.tp_newchild, "attach_trace")) {
+		err = -EINVAL;
+		goto cleanup_detach;
+	}
+
+	/* Fork, update vars from BPF and assert the unfrozen state */
+	new_pid2 = fork();
+	if (new_pid2 == 0)
+		exit(0);
+
+	err = (new_pid2 == -1);
+	if (ASSERT_OK(err, "fork process"))
+		wait(NULL);
+
+	/* Now assert that new_pid2 reflects this new child */
+	ASSERT_NEQ(0, skel->bss->new_pid,
+		   "test task_freeze_cgroup failed  at new_pid != 0");
+	ASSERT_NEQ(skel->rodata->monitor_pid, skel->bss->new_pid,
+		   "test task_freeze_cgroup failed  at old monitor_pid != new_pid");
+	/* Assert that bpf sets new_pid to new forked child new_pid2 */
+	ASSERT_EQ(skel->bss->new_pid, new_pid2,
+		  "test task_freeze_cgroup failed first child new_pid == new_pid2");
+
+cleanup_detach:
+	test_task_freeze_cgroup__detach(skel);
+cleanup_skel:
+	test_task_freeze_cgroup__destroy(skel);
+cleanup_cgroup:
+	close(cgroup_fd);
+	cleanup_cgroup_environment();
+	return err;
+}
+
+void test_task_freeze_cgroup(void)
+{
+	pid_t pid, result;
+	char buf[512] = {0};
+	char path[PATH_MAX] = {0};
+	int ret, status, attempts, frozen = 0, fd;
+	struct test_task_freeze_cgroup *skel = NULL;
+
+	pid = fork();
+	ret = (pid == -1);
+	if (!ASSERT_OK(ret, "fork process"))
+		return;
+
+	if (pid == 0) {
+		ret = bpf_sleepable(skel);
+		ASSERT_EQ(0, ret, "child bpf_sleepable failed");
+		exit(ret);
+	}
+
+	skel = test_task_freeze_cgroup__open();
+	if (!ASSERT_OK_PTR(skel, "test_task_freeze_cgroup__open"))
+		goto out;
+
+	snprintf(path, sizeof(path),
+		 "/sys/fs/cgroup/cgroup-test-work-dir%d%s/cgroup.freeze",
+		 pid, CGROUP_PATH);
+
+	for (attempts = 10; attempts >= 0; attempts--) {
+		ret = 0;
+
+		fd = open(path, O_RDONLY);
+		if (fd > 0)
+			ret = read(fd, buf, sizeof(buf) - 1);
+		if (ret > 0) {
+			errno = 0;
+			frozen = strtol(buf, NULL, 10);
+			if (errno)
+				frozen = 0;
+		}
+
+		close(fd);
+		if (frozen)
+			break;
+		sleep(1);
+	}
+
+	/* Assert that child cgroup is frozen */
+	if (!ASSERT_EQ(1, frozen, "child cgroup not frozen"))
+		goto out;
+
+	ret = test_task_freeze_cgroup__load(skel);
+	if (!ASSERT_OK(ret, "test_task_freeze_cgroup__load"))
+		goto out;
+
+	/* Trigger the unthaw child cgroup from parent */
+	skel->links.lsm_task_free =
+		bpf_program__attach_lsm(skel->progs.lsm_task_free);
+	if (!ASSERT_OK_PTR(skel->links.lsm_task_free, "attach_lsm"))
+		goto out;
+
+	result = waitpid(pid, &status, WUNTRACED);
+	if (!ASSERT_NEQ(result, -1, "waitpid"))
+		goto detach;
+
+	result = WIFEXITED(status);
+	if (!ASSERT_EQ(result, 1, "forked process did not terminate normally"))
+		goto detach;
+
+	result = WEXITSTATUS(status);
+	if (!ASSERT_EQ(result, 0, "forked process did not exit successfully"))
+		goto detach;
+
+detach:
+	test_task_freeze_cgroup__detach(skel);
+
+out:
+	if (skel)
+		test_task_freeze_cgroup__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_task_freeze_cgroup.c b/tools/testing/selftests/bpf/progs/test_task_freeze_cgroup.c
new file mode 100644
index 000000000000..07b4b65abc36
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_task_freeze_cgroup.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <linux/types.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_helpers.h>
+#include <errno.h>
+#include "bpf_kfuncs.h"
+#include "bpf_misc.h"
+
+struct cgroup *bpf_cgroup_from_id(u64 cgid) __ksym;
+long bpf_task_under_cgroup(struct task_struct *task, struct cgroup *ancestor) __ksym;
+void bpf_cgroup_release(struct cgroup *p) __ksym;
+struct task_struct *bpf_task_from_pid(s32 pid) __ksym;
+struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym;
+void bpf_task_release(struct task_struct *p) __ksym;
+
+extern int bpf_cgroup_write_interface(struct cgroup *cgrp,
+				      const char *name__str,
+				      const struct bpf_dynptr *value_p,
+				      loff_t off) __ksym __weak;
+
+char freeze_val[] = "1";
+char unthaw_val[] = "0";
+
+const volatile int parent_pid;
+const volatile int monitor_pid;
+const volatile __u64 cgid;
+int new_pid;
+int freeze;
+
+SEC("tp_btf/task_newtask")
+int BPF_PROG(tp_newchild, struct task_struct *task, u64 clone_flags)
+{
+	struct cgroup *cgrp = NULL;
+	struct task_struct *acquired;
+
+	if (monitor_pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
+
+	acquired = bpf_task_acquire(task);
+	if (!acquired)
+		return 0;
+
+	cgrp = bpf_cgroup_from_id(cgid);
+	if (!cgrp)
+		goto out;
+
+	/* Update new_pid with current pid */
+	if (bpf_task_under_cgroup(acquired, cgrp))
+		new_pid = acquired->tgid;
+
+out:
+	if (cgrp)
+		bpf_cgroup_release(cgrp);
+	bpf_task_release(acquired);
+
+	return 0;
+}
+
+/* Try to attach from parent to trigger the bpf lsm hook, so from
+ * parent context we unthaw child cgroup.
+ */
+SEC("lsm/task_free")
+int BPF_PROG(lsm_task_free, struct task_struct *task)
+{
+	return 0;
+}
+
+static int process_freeze_cgroup(int pid, int freeze)
+{
+	int ret = 0;
+	struct task_struct *task;
+	struct bpf_dynptr dyn_ptr;
+	struct cgroup *cgrp = NULL;
+
+	task = bpf_task_from_pid(pid);
+	if (!task)
+		return -EINVAL;
+
+	cgrp = bpf_cgroup_from_id(cgid);
+	if (!cgrp) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (!bpf_task_under_cgroup(task, cgrp))
+		goto out;
+
+	if (freeze)
+		bpf_dynptr_from_mem(freeze_val, sizeof(freeze_val), 0, &dyn_ptr);
+	else
+		bpf_dynptr_from_mem(unthaw_val, sizeof(unthaw_val), 0, &dyn_ptr);
+
+	ret = bpf_cgroup_write_interface(cgrp, "cgroup.freeze", &dyn_ptr, 0);
+
+out:
+	if (cgrp)
+		bpf_cgroup_release(cgrp);
+	bpf_task_release(task);
+	return ret;
+}
+
+SEC("lsm.s/bpf")
+int BPF_PROG(lsm_freeze_cgroup, int cmd, union bpf_attr *attr, unsigned int size)
+{
+	int ret = 0;
+	struct task_struct *task;
+	struct cgroup *cgrp = NULL;
+
+	if (cmd != BPF_LINK_CREATE)
+		return 0;
+
+	task = bpf_get_current_task_btf();
+	if (parent_pid == task->pid) {
+		/* Parent context: unthaw child */
+		process_freeze_cgroup(monitor_pid, 0);
+		return 0;
+	}
+
+	/* Nothing todo */
+	if (!freeze)
+		return 0;
+
+	/* Child context */
+	if (monitor_pid != task->pid)
+		return 0;
+
+	/* Ensure we are under the corresponding cgroup so we freeze
+	 * current child from its context
+	 */
+	cgrp = bpf_cgroup_from_id(cgid);
+	if (!cgrp)
+		return 0;
+
+	if (!bpf_task_under_cgroup(task, cgrp))
+		goto out;
+
+	/* Schedule freeze task and return -EPERM */
+	ret = process_freeze_cgroup(monitor_pid, freeze);
+
+	/* On error or 0 we return zero and we catch at
+	 * user space if the cgroup was not frozen.
+	 */
+	ret = (ret > 0) ? -EPERM : 0;
+
+	/* Reset for next calls */
+	freeze = 0;
+out:
+	if (cgrp)
+		bpf_cgroup_release(cgrp);
+	return ret;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.43.0

Re: [RFC PATCH v2 bpf-next 0/3] bpf: cgroup: support writing and freezing cgroups from BPF

[Tejun Heo]

On Mon, Aug 18, 2025 at 10:04:21AM +0100, Djalal Harouni wrote:
> This patch series add support to write cgroup interfaces from BPF.
> 
> It is useful to freeze a cgroup hierarchy on suspicious activity for
> a more thorough analysis before killing it. Planned users of this
> feature are: systemd and BPF tools where the cgroup hierarchy could
> be a system service, user session, k8s pod or a container.
> 
> The writing happens via kernfs nodes and the cgroup must be on the
> default hierarchy. It implements the requests and feedback from v1 [1]
> where now we use a unified path for cgroup user space and BPF writing.
> 
> So I want to validate that this is the right approach first.

I don't see any reason to object to the feature but the way it's constructed
seems rather odd to me. If it's going to need per-feature code, might as
well bypass the write part and implement a simpler interface - ie.
bpf_cgroup_freeze(). Otherwise, can't it actually write to kernfs files so
that we don't need to add code per enabled feature?

Thanks.

-- 
tejun

Re: [RFC PATCH v2 bpf-next 0/3] bpf: cgroup: support writing and freezing cgroups from BPF

[Djalal Harouni]

On 8/18/25 18:32, Tejun Heo wrote:
> On Mon, Aug 18, 2025 at 10:04:21AM +0100, Djalal Harouni wrote:
>> This patch series add support to write cgroup interfaces from BPF.
>>
>> It is useful to freeze a cgroup hierarchy on suspicious activity for
>> a more thorough analysis before killing it. Planned users of this
>> feature are: systemd and BPF tools where the cgroup hierarchy could
>> be a system service, user session, k8s pod or a container.
>>
>> The writing happens via kernfs nodes and the cgroup must be on the
>> default hierarchy. It implements the requests and feedback from v1 [1]
>> where now we use a unified path for cgroup user space and BPF writing.
>>
>> So I want to validate that this is the right approach first.
> 
> I don't see any reason to object to the feature but the way it's constructed
> seems rather odd to me. If it's going to need per-feature code, might as
> well bypass the write part and implement a simpler interface - ie.
> bpf_cgroup_freeze().

Approach 1:
First RFC months ago was something like that "bpf_task_freeze_cgroup" 
[1], can make it bpf_cgroup_freeze() as a proper kfunc, so resurrect 
approach 1?

Internally it used an ugly path to workaround kernfs active reference 
since we don't hold a kernfs_open_file coming from userspace
kernfs->write path.

I can improve it, but let's discuss please approach (2) since you
suggested it ;-)

Approach 2:
Per the old suggestions from you and Alexei [2] [3] you wanted something
like:

   s32 bpf_kernfs_knob_write(struct kernfs_node *dir,
                             const char *knob, char *buf);

I didn't make it generic for kernfs, since don't know yet about sysfs 
use cases and named it "bpf_cgroup_write_interface" to focus on cgroup 
base interfaces.
Doing something that generic now including sysfs without a proper valid 
use cases seems a bit too much. Also we have some cgroup kfunc to 
acquire and release that integrate well, so I kept it focused.

Alexei suggested to refactor the cgroup_base_file[] [4][5] to take 
"kernfs_node" as argument instead of "kernfs_open_file", which will open 
other possibilities for BPF.

However, instead of going full change on cgroup_base_files[], I added a 
minimalist: cgroup_kn_cftype kn_cfts[] that for now hold only 
"cgroup.freeze".

I see three possibilities here:

A. Minimal change with approach presented here:
    add dedicated array cgroup_kn_cftype kn_cfts[] with only
    "cgroup.freeze" and later try to unify it inside cgroup_base_file[].
B. Add "->bpf_write()" handler to cgroup_base_file[] and start only with
    "cgroup.freeze".
C. Refactor all cgroup_base_file[] to take a kernfs_node directly
    instead of kernfs_open_file as suggested.

I took (C) the simple one since I wanted to do cgroup freeze first. You
also suggested maybe in future "cgroup.kill" well if we have it, we 
definitely will start using it. Not sure if we are allowed to BPF sleep 
that path, however we can also start doing "cgroup.freeze" from BPF and 
kill from user space as a first step. But we definitly want more BPF 
operations on cgroup interfaces, I can think of a companion cgroup where 
we migrate tasks there on specific signals...

So more or less current proposed approach (2) followed the suggestions, 
but focused only on writing cgroup kernfs knobs.

Thoughts, did I miss something?


> Otherwise, can't it actually write to kernfs files so
> that we don't need to add code per enabled feature?

I'm not sure how we would write to kernfs files? As pointed by Alexei 
[6] it is more involved if we want to open files...

About "that we don't need to add code per enabled feature?" well if we
go the path of "bpf_cgroup_write_interface" or "bpf_cgroup_write_knob" 
adding a new write interface will involve theoretically:

1. Check if program can sleep or/and the calling context.
2. Add the new "cgroup.x" either in cgroup_base_file[] or in the new
    array.
3. New handlers or refactor old ones to take a "kernfs_node" instead of
    "kernfs_open_file".

Compared to having multiple bpf_cgroup_(freeze|kill|...) kfunc seems 
fair too, and not that much code from BPF part.


BTW current patches contain a bug, after testing. In normal writes from 
user context we break kernfs active protection to avoid nesting cgroup 
locking under.
Forget this part. In next series since we don't grab the kernfs_node 
active protection like userspace kernfs_write, then no need to break 
it... will add a parameter to check like the revalidate one that checks 
things is cgroup on dfl and not root, things that are automatically 
handled from normal userspace ->write.


Thank you!


[1] https://lore.kernel.org/bpf/20240327225334.58474-3-tixxdz@gmail.com/
[2] https://lore.kernel.org/bpf/ZgXMww9kJiKi4Vmd@slm.duckdns.org/
[3] https://lore.kernel.org/bpf/Zgc1BZnYCS9OSSTw@slm.duckdns.org/
[4] 
https://lore.kernel.org/bpf/CAADnVQK970_Nx3918V41ue031RkGs+WsteOAm6EJOY7oSwzS1A@mail.gmail.com/
[5] 
https://lore.kernel.org/bpf/CAADnVQ+WmaPG1WOaSDbjxNPVzVape_JfG_CNSRy188ni076Mog@mail.gmail.com/
[6] 
https://lore.kernel.org/bpf/CAADnVQLhWDcX-7XCdo-W=jthU=9iPqODwrE6c9fvU8sfAJ5ARg@mail.gmail.com/


> Thanks.
> 

Re: [RFC PATCH v2 bpf-next 0/3] bpf: cgroup: support writing and freezing cgroups from BPF

[Djalal Harouni]

On 8/20/25 00:31, Djalal Harouni wrote:
> On 8/18/25 18:32, Tejun Heo wrote:
>> On Mon, Aug 18, 2025 at 10:04:21AM +0100, Djalal Harouni wrote:
>>> This patch series add support to write cgroup interfaces from BPF.
>>>
>>> It is useful to freeze a cgroup hierarchy on suspicious activity for
>>> a more thorough analysis before killing it. Planned users of this
>>> feature are: systemd and BPF tools where the cgroup hierarchy could
>>> be a system service, user session, k8s pod or a container.
>>>
>>> The writing happens via kernfs nodes and the cgroup must be on the
>>> default hierarchy. It implements the requests and feedback from v1 [1]
>>> where now we use a unified path for cgroup user space and BPF writing.
>>>
>>> So I want to validate that this is the right approach first.
>>
>> I don't see any reason to object to the feature but the way it's 
>> constructed
>> seems rather odd to me. If it's going to need per-feature code, might as
>> well bypass the write part and implement a simpler interface - ie.
>> bpf_cgroup_freeze().
> 
> Approach 1:
> First RFC months ago was something like that 
> "bpf_task_freeze_cgroup" [1], can make it bpf_cgroup_freeze() as a 
> proper kfunc, so resurrect approach 1?
> 
> Internally it used an ugly path to workaround kernfs active reference 
> since we don't hold a kernfs_open_file coming from userspace
> kernfs->write path.
> 
> I can improve it, but let's discuss please approach (2) since you
> suggested it ;-)
> 
> Approach 2:
> Per the old suggestions from you and Alexei [2] [3] you wanted something
> like:
> 
>    s32 bpf_kernfs_knob_write(struct kernfs_node *dir,
>                              const char *knob, char *buf);
> 
> I didn't make it generic for kernfs, since don't know yet about sysfs 
> use cases and named it "bpf_cgroup_write_interface" to focus on cgroup 
> base interfaces.
> Doing something that generic now including sysfs without a proper valid 
> use cases seems a bit too much. Also we have some cgroup kfunc to 
> acquire and release that integrate well, so I kept it focused.
> 
> Alexei suggested to refactor the cgroup_base_file[] [4][5] to take 
> "kernfs_node" as argument instead of "kernfs_open_file", which will open 
> other possibilities for BPF.
> 
> However, instead of going full change on cgroup_base_files[], I added a 
> minimalist: cgroup_kn_cftype kn_cfts[] that for now hold only 
> "cgroup.freeze".
> 
> I see three possibilities here:
> 
> A. Minimal change with approach presented here:
>     add dedicated array cgroup_kn_cftype kn_cfts[] with only
>     "cgroup.freeze" and later try to unify it inside cgroup_base_file[].
> B. Add "->bpf_write()" handler to cgroup_base_file[] and start only with
>     "cgroup.freeze".
> C. Refactor all cgroup_base_file[] to take a kernfs_node directly
>     instead of kernfs_open_file as suggested.
> 
> I took (C) the simple one since I wanted to do cgroup freeze first. You

Sorry here I meant (A).

Thanks.


> also suggested maybe in future "cgroup.kill" well if we have it, we 
> definitely will start using it. Not sure if we are allowed to BPF sleep 
> that path, however we can also start doing "cgroup.freeze" from BPF and 
> kill from user space as a first step. But we definitly want more BPF 
> operations on cgroup interfaces, I can think of a companion cgroup where 
> we migrate tasks there on specific signals...
> 
> So more or less current proposed approach (2) followed the suggestions, 
> but focused only on writing cgroup kernfs knobs.
> 
> Thoughts, did I miss something?
> 
> 
>> Otherwise, can't it actually write to kernfs files so
>> that we don't need to add code per enabled feature?
> 
> I'm not sure how we would write to kernfs files? As pointed by Alexei 
> [6] it is more involved if we want to open files...
> 
> About "that we don't need to add code per enabled feature?" well if we
> go the path of "bpf_cgroup_write_interface" or "bpf_cgroup_write_knob" 
> adding a new write interface will involve theoretically:
> 
> 1. Check if program can sleep or/and the calling context.
> 2. Add the new "cgroup.x" either in cgroup_base_file[] or in the new
>     array.
> 3. New handlers or refactor old ones to take a "kernfs_node" instead of
>     "kernfs_open_file".
> 
> Compared to having multiple bpf_cgroup_(freeze|kill|...) kfunc seems 
> fair too, and not that much code from BPF part.
> 
> 
> BTW current patches contain a bug, after testing. In normal writes from 
> user context we break kernfs active protection to avoid nesting cgroup 
> locking under.
> Forget this part. In next series since we don't grab the kernfs_node 
> active protection like userspace kernfs_write, then no need to break 
> it... will add a parameter to check like the revalidate one that checks 
> things is cgroup on dfl and not root, things that are automatically 
> handled from normal userspace ->write.
> 
> 
> Thank you!
> 
> 
> [1] https://lore.kernel.org/bpf/20240327225334.58474-3-tixxdz@gmail.com/
> [2] https://lore.kernel.org/bpf/ZgXMww9kJiKi4Vmd@slm.duckdns.org/
> [3] https://lore.kernel.org/bpf/Zgc1BZnYCS9OSSTw@slm.duckdns.org/
> [4] https://lore.kernel.org/bpf/ 
> CAADnVQK970_Nx3918V41ue031RkGs+WsteOAm6EJOY7oSwzS1A@mail.gmail.com/
> [5] https://lore.kernel.org/bpf/ 
> CAADnVQ+WmaPG1WOaSDbjxNPVzVape_JfG_CNSRy188ni076Mog@mail.gmail.com/
> [6] https://lore.kernel.org/bpf/CAADnVQLhWDcX-7XCdo- 
> W=jthU=9iPqODwrE6c9fvU8sfAJ5ARg@mail.gmail.com/
> 
> 
>> Thanks.
>>
> 

Re: [RFC PATCH v2 bpf-next 0/3] bpf: cgroup: support writing and freezing cgroups from BPF

[Tejun Heo]

Hello,

On Wed, Aug 20, 2025 at 12:31:01AM +0100, Djalal Harouni wrote:
...
> Approach 1:
> First RFC months ago was something like that "bpf_task_freeze_cgroup" [1],
> can make it bpf_cgroup_freeze() as a proper kfunc, so resurrect approach 1?

Thanks for reminding me. I often feel like my memory is a ring buffer which
lasts a few weeks at most.

> Internally it used an ugly path to workaround kernfs active reference since
> we don't hold a kernfs_open_file coming from userspace
> kernfs->write path.
> 
> I can improve it, but let's discuss please approach (2) since you
> suggested it ;-)
> 
> Approach 2:
> Per the old suggestions from you and Alexei [2] [3] you wanted something
> like:
> 
>   s32 bpf_kernfs_knob_write(struct kernfs_node *dir,
>                             const char *knob, char *buf);
> 
> I didn't make it generic for kernfs, since don't know yet about sysfs use
> cases and named it "bpf_cgroup_write_interface" to focus on cgroup base
> interfaces.
> Doing something that generic now including sysfs without a proper valid use
> cases seems a bit too much. Also we have some cgroup kfunc to acquire and
> release that integrate well, so I kept it focused.
> 
> Alexei suggested to refactor the cgroup_base_file[] [4][5] to take
> "kernfs_node" as argument instead of "kernfs_open_file", which will open
> other possibilities for BPF.
> 
> However, instead of going full change on cgroup_base_files[], I added a
> minimalist: cgroup_kn_cftype kn_cfts[] that for now hold only
> "cgroup.freeze".

I think there's some misunderstanding here. IIUC, Alexei didn't want to
expose direct file interface because none of the necessary abstractions are
available and the effort becomes too big and wide. However, I don't think
the suggestion to use @kn as the path designator (or @cgroup for that
matter) means that we want to pipe that all the way down to each op that's
to be written to. That'd be rather pointless - why add generic interface if
each needs a dedicated backend anyway? Can't you make kernfs to create
open_file from kn and follow the usual write path?

Thanks.

-- 
tejun

Re: [RFC PATCH v2 bpf-next 0/3] bpf: cgroup: support writing and freezing cgroups from BPF

[Djalal Harouni]

On 8/20/25 02:14, Tejun Heo wrote:
> Hello,
> 
> On Wed, Aug 20, 2025 at 12:31:01AM +0100, Djalal Harouni wrote:
> ...
>> Approach 1:
>> First RFC months ago was something like that "bpf_task_freeze_cgroup" [1],
>> can make it bpf_cgroup_freeze() as a proper kfunc, so resurrect approach 1?
> 
> Thanks for reminding me. I often feel like my memory is a ring buffer which
> lasts a few weeks at most.

yeh been a while...

>> Internally it used an ugly path to workaround kernfs active reference since
>> we don't hold a kernfs_open_file coming from userspace
>> kernfs->write path.
>>
>> I can improve it, but let's discuss please approach (2) since you
>> suggested it ;-)
>>
>> Approach 2:
>> Per the old suggestions from you and Alexei [2] [3] you wanted something
>> like:
>>
>>    s32 bpf_kernfs_knob_write(struct kernfs_node *dir,
>>                              const char *knob, char *buf);
>>
>> I didn't make it generic for kernfs, since don't know yet about sysfs use
>> cases and named it "bpf_cgroup_write_interface" to focus on cgroup base
>> interfaces.
>> Doing something that generic now including sysfs without a proper valid use
>> cases seems a bit too much. Also we have some cgroup kfunc to acquire and
>> release that integrate well, so I kept it focused.
>>
>> Alexei suggested to refactor the cgroup_base_file[] [4][5] to take
>> "kernfs_node" as argument instead of "kernfs_open_file", which will open
>> other possibilities for BPF.
>>
>> However, instead of going full change on cgroup_base_files[], I added a
>> minimalist: cgroup_kn_cftype kn_cfts[] that for now hold only
>> "cgroup.freeze".
> 
> I think there's some misunderstanding here. IIUC, Alexei didn't want to
> expose direct file interface because none of the necessary abstractions are
> available and the effort becomes too big and wide. However, I don't think
> the suggestion to use @kn as the path designator (or @cgroup for that
> matter) means that we want to pipe that all the way down to each op that's
> to be written to. That'd be rather pointless - why add generic interface if
> each needs a dedicated backend anyway?

I took time to check...

I thought to avoid precisely creating a kernfs_open_file, since we don't
have the corresponding context. Beside the user information who opened
the file, kernfs_open_file assumes a corresponding opened file and a
corresponding kernfs_open_node for each kernfs_node with one or more
open files.

Also it seems each kernfs_open_file is added back into
kernfs_open_node->files...

> Can't you make kernfs to create
> open_file from kn and follow the usual write path?

Seems tricky, needs to handle also kernfs_global_locks->open_file_mutex
compared with writing directly to the dedicated cgroup backend. The
only protection there beside cgroup_lock() is break kernfs active
protection so kernfs nodes can be drained later...

The cgroup_kn_lock_live() grabs a ref on the cgroup, breaks the active
protection, takes the cgroup_lock() and ensures the cgroup is not dead,
from there we don't dereference kn->  until we restore the protection.
Plus we have the kernfs_get(cgrp->kn) on cgroup_mkdir which ensures it
is always accessible.

I do realize taking the same usual path with write is the obvious thing,
but we don't have the corresponding open context, and faking it seems
more trouble than calling directly cgroup backends...

Allow me please to do it again directly on cgroup_base_file[] assuming
it was Alexei suggestion and see how it looks.

Also Tejun, could you please point me to extra cgroup or kernfs tests
you run? much appreciated!

Thanks!

> Thanks.
> 

Re: [RFC PATCH v2 bpf-next 0/3] bpf: cgroup: support writing and freezing cgroups from BPF

[Tejun Heo]

Hello,

On Fri, Aug 22, 2025 at 07:16:15PM +0100, Djalal Harouni wrote:
...
> I do realize taking the same usual path with write is the obvious thing,
> but we don't have the corresponding open context, and faking it seems
> more trouble than calling directly cgroup backends...
> 
> Allow me please to do it again directly on cgroup_base_file[] assuming
> it was Alexei suggestion and see how it looks.

I'm probably missing something but what prevents you from getting a dentry
from kernfs_node and then calling vfs_open() on it and then do vfs_write()
on the returned file?

If there are some fundamental reasons that we can't do something like that,
let's go back to the simple approach where we just have bpf helpers for
freezing and unfreezing cgroups outside of fs interface.

> Also Tejun, could you please point me to extra cgroup or kernfs tests
> you run? much appreciated!

I'm afraid there isn't much outside what's in the selftest directory.

Thanks.

-- 
tejun

Re: [RFC PATCH v2 bpf-next 0/3] bpf: cgroup: support writing and freezing cgroups from BPF

[Alexei Starovoitov]

On Mon, Aug 25, 2025 at 11:48 AM Tejun Heo <tj@kernel.org> wrote:
>
> Hello,
>
> On Fri, Aug 22, 2025 at 07:16:15PM +0100, Djalal Harouni wrote:
> ...
> > I do realize taking the same usual path with write is the obvious thing,
> > but we don't have the corresponding open context, and faking it seems
> > more trouble than calling directly cgroup backends...
> >
> > Allow me please to do it again directly on cgroup_base_file[] assuming
> > it was Alexei suggestion and see how it looks.

It's been 1.5 year since v1. It's safe to assume that all opinions
have changed, including mine.

> I'm probably missing something but what prevents you from getting a dentry
> from kernfs_node and then calling vfs_open() on it and then do vfs_write()
> on the returned file?

Generic vfs ops from kfunc feels like a can of worms.
It will require a lot more thinking and coordination with vfs folks.
I'd rather keep things simple especially, since this thread
might continue in 2026.

> If there are some fundamental reasons that we can't do something like that,
> let's go back to the simple approach where we just have bpf helpers for
> freezing and unfreezing cgroups outside of fs interface.

I'd just do whatever version of cgroup_lock*() is necessary
from kfunc followed by cgroup_freeze(),
and limit kfunc to sleepable progs, of course.

Re: [RFC PATCH v2 bpf-next 0/3] bpf: cgroup: support writing and freezing cgroups from BPF

[Djalal Harouni]

Hello,

On 8/25/25 19:48, Tejun Heo wrote:
> Hello,
> 
> On Fri, Aug 22, 2025 at 07:16:15PM +0100, Djalal Harouni wrote:
> ...
>> I do realize taking the same usual path with write is the obvious thing,
>> but we don't have the corresponding open context, and faking it seems
>> more trouble than calling directly cgroup backends...
>>
>> Allow me please to do it again directly on cgroup_base_file[] assuming
>> it was Alexei suggestion and see how it looks.
> 
> I'm probably missing something but what prevents you from getting a dentry
> from kernfs_node and then calling vfs_open() on it and then do vfs_write()
> on the returned file?

If we include the open path then don't have the right context, first
example in vfs_open() will use the wrong current cred context to perform
permission checks, current could have dropped privileges while the
cgroup hierarchy is still root owned...

The thing here is that the bpf program will be called from arbitrary
paths, not a single pre-defined path/function were we could control
the context...

> If there are some fundamental reasons that we can't do something like that,
> let's go back to the simple approach where we just have bpf helpers for
> freezing and unfreezing cgroups outside of fs interface.

Alright, seems Alexei also agree on this. Thanks will prepare another
version.

>> Also Tejun, could you please point me to extra cgroup or kernfs tests
>> you run? much appreciated!
> 
> I'm afraid there isn't much outside what's in the selftest directory.

Ok, thank you!


> Thanks.
> 

Re: [RFC PATCH v2 bpf-next 0/3] bpf: cgroup: support writing and freezing cgroups from BPF

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 1247 bytes --]

Hi Djalal.

On Mon, Aug 18, 2025 at 10:04:21AM +0100, Djalal Harouni <tixxdz@gmail.com> wrote:
> This patch series add support to write cgroup interfaces from BPF.
> 
> It is useful to freeze a cgroup hierarchy on suspicious activity for
> a more thorough analysis before killing it. Planned users of this
> feature are: systemd and BPF tools where the cgroup hierarchy could
> be a system service, user session, k8s pod or a container.

Could you please give more specific example of the "suspicious
activity"? The last time (v1) it was referring to LSM hooks where such
asynchronous approach wasn't ideal.
Also why couldn't all these tools execute the cgroup actions themselves
through traditional userspace API?

One more point (for possible interference with lifecycles) -- what is
the relation between cgroup in which the BPF code "runs" and cgroup
that's target of the operation? (I hope this isn't supposed to run from
BPF without process context.)

 
> Todo:
> * Limit size of data to be written.
> * Further tests.
> * Add cgroup kill support.

I'm missing the retrieval of freeze result in this plan :) cgroup kill
would be simpler for PoC (and maybe even sufficient for your use case?).

Regards,
Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]

Re: [RFC PATCH v2 bpf-next 0/3] bpf: cgroup: support writing and freezing cgroups from BPF

[Djalal Harouni]

Hi Michal,

On 8/26/25 15:18, Michal Koutný wrote:
> Hi Djalal.
> 
> On Mon, Aug 18, 2025 at 10:04:21AM +0100, Djalal Harouni <tixxdz@gmail.com> wrote:
>> This patch series add support to write cgroup interfaces from BPF.
>>
>> It is useful to freeze a cgroup hierarchy on suspicious activity for
>> a more thorough analysis before killing it. Planned users of this
>> feature are: systemd and BPF tools where the cgroup hierarchy could
>> be a system service, user session, k8s pod or a container.
> 
> Could you please give more specific example of the "suspicious
> activity"? The last time (v1) it was referring to LSM hooks where such
> asynchronous approach wasn't ideal.

It solves the case perfectly, you detect something you fail the
security hook return -EPERM and optionally freeze the cgroup,
snapshot the runtime state.

Oh I thought the attached example is an obvious one, customers want to
restrict bpf() usage per cgroup specific container/pod, so when
we detect bpf() that's not per allowed cgroup we fail it and freeze
it.

Take this and build on top, detect bash/shell exec or any other new
dropped binaries, fail and freeze the exec early at linux_bprm object
checks.


> Also why couldn't all these tools execute the cgroup actions themselves
> through traditional userspace API?

- Freezing at BPF is obviously better, less race since you don't need
   access to the corresponding cgroup fs and namespace. Not all tools run
   as supervisor/container manager.
- The bpf_send_signal in some cases is not enough, what if you race with
   a task clone as an example? however freezing the cgroup hierarchy or
   the one above is a catch all...


> One more point (for possible interference with lifecycles) -- what is
> the relation between cgroup in which the BPF code "runs" and cgroup
> that's target of the operation? (I hope this isn't supposed to run from
> BPF without process context.)

The feature is supposed to be used by sleepable BPF programs, I don't
think we need extra checks here?

It could be that this BPF code runs in a process that is under
pod-x/container-y/cgroup-z/  and maybe you want to freeze "cgroup-z"
or "container-y" and so on... or in case of delegated hierarchies,
freezing the parent is a catch all.


>   
>> Todo:
>> * Limit size of data to be written.
>> * Further tests.
>> * Add cgroup kill support.
> 
> I'm missing the retrieval of freeze result in this plan :) cgroup kill

Indeed you are right a small kfunc to read back, yes ;) !

> would be simpler for PoC (and maybe even sufficient for your use case?).
> 

I think both are useful cases.

Thank you!


> Regards,
> Michal

Re: [RFC PATCH v2 bpf-next 0/3] bpf: cgroup: support writing and freezing cgroups from BPF

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 3143 bytes --]

On Wed, Aug 27, 2025 at 12:27:08AM +0100, Djalal Harouni <tixxdz@gmail.com> wrote:
> It solves the case perfectly, you detect something you fail the
> security hook return -EPERM and optionally freeze the cgroup,
> snapshot the runtime state.

So -EPERM is the right way to cut off such tasks.

> Oh I thought the attached example is an obvious one, customers want to
> restrict bpf() usage per cgroup specific container/pod, so when
> we detect bpf() that's not per allowed cgroup we fail it and freeze
> it.
> 
> Take this and build on top, detect bash/shell exec or any other new
> dropped binaries, fail and freeze the exec early at linux_bprm object
> checks.

Or if you want to do some followup analysis, the process can be killed
and coredump'd (at least seccomp allows this, it'd be good to have such
a possibility with LSMs if there isn't (I'm not that familiar)).
Freezing the groups sounds like a way to DoS the system (not only
because of hanging the faulty process itself but possibly spreading via
IPC dependencies to unrelated processes).

> > Also why couldn't all these tools execute the cgroup actions themselves
> > through traditional userspace API?
> 
> - Freezing at BPF is obviously better, less race since you don't need
>   access to the corresponding cgroup fs and namespace. Not all tools run
>   as supervisor/container manager.

Less race or more race -- I know the race window size may vary but
strictly speaking , there is a race or isn't (depends on having proper
synchronization or not). (And when intentionally misbehaving processes are
considered even tiny window is potential risk.)

> - The bpf_send_signal in some cases is not enough, what if you race with
>   a task clone as an example? however freezing the cgroup hierarchy or
>   the one above is a catch all...

Yeah, this might be part that I don't internalize well. If you're
running the hook in particular task's process context, it cannot do
clone at the same time. If they are independent tasks, there's no
ordering, so there's always possibility of the race (so why not embrace
it and do whatever is possible with userspace monitoring audit log or
similar and respond based on that).

> The feature is supposed to be used by sleepable BPF programs, I don't
> think we need extra checks here?

Good.

> It could be that this BPF code runs in a process that is under
> pod-x/container-y/cgroup-z/  and maybe you want to freeze "cgroup-z"
> or "container-y" and so on... or in case of delegated hierarchies,
> freezing the parent is a catch all.

OK, this would be good. Could it also be pod-x/container-y/cgroup-z2?

---

I acknowledge that sooner or later some kind of access to cgroup through
BPF will be added, I'd prefer if it was done in a generic way (so that
it doesn't become cgroup's problem but someone else's e.g. VFS's or
kernfs's ;-)).
I can even imagine some usefulness of helpers for selected specific
cgroup (core) operations (which is the direction brought up in the other
discussion), I just don't think it solves the problem as you present it.

HTH,
Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]

Re: [RFC PATCH v2 bpf-next 0/3] bpf: cgroup: support writing and freezing cgroups from BPF

[Djalal Harouni]

Hi Michal,

On 8/28/25 15:38, Michal Koutný wrote:
> On Wed, Aug 27, 2025 at 12:27:08AM +0100, Djalal Harouni <tixxdz@gmail.com> wrote:
>> It solves the case perfectly, you detect something you fail the
>> security hook return -EPERM and optionally freeze the cgroup,
>> snapshot the runtime state.
> 
> So -EPERM is the right way to cut off such tasks.
> 

Indeed. A process can retry x y z paths, at some point we just
want to stop the process or container before snapshot.

>> Oh I thought the attached example is an obvious one, customers want to
>> restrict bpf() usage per cgroup specific container/pod, so when
>> we detect bpf() that's not per allowed cgroup we fail it and freeze
>> it.
>>
>> Take this and build on top, detect bash/shell exec or any other new
>> dropped binaries, fail and freeze the exec early at linux_bprm object
>> checks.
> 
> Or if you want to do some followup analysis, the process can be killed
> and coredump'd (at least seccomp allows this, it'd be good to have such
> a possibility with LSMs if there isn't (I'm not that familiar)).
> Freezing the groups sounds like a way to DoS the system (not only
> because of hanging the faulty process itself but possibly spreading via
> IPC dependencies to unrelated processes).

Well mis-using things is possible, but here nothing is new.

Pausing a container, freezing the cgroup, or criu have been there for
years, no new features here ;-)

> 
>>> Also why couldn't all these tools execute the cgroup actions themselves
>>> through traditional userspace API?
>>
>> - Freezing at BPF is obviously better, less race since you don't need
>>    access to the corresponding cgroup fs and namespace. Not all tools run
>>    as supervisor/container manager.
> 
> Less race or more race -- I know the race window size may vary but
> strictly speaking , there is a race or isn't (depends on having proper
> synchronization or not). (And when intentionally misbehaving processes are
> considered even tiny window is potential risk.)
> 
>> - The bpf_send_signal in some cases is not enough, what if you race with
>>    a task clone as an example? however freezing the cgroup hierarchy or
>>    the one above is a catch all...
> 
> Yeah, this might be part that I don't internalize well. If you're
> running the hook in particular task's process context, it cannot do
> clone at the same time. If they are independent tasks, there's no
> ordering, so there's always possibility of the race (so why not embrace
> it and do whatever is possible with userspace monitoring audit log or
> similar and respond based on that).

The complexity to do that from userspace for an eBPF security tool
that is not running as a supervisor in other namespaces is high.
Basically the race window is reduced... we trigger in the task context 
that we want to freeze and set the freeze jobctl bit of the other
tasks.

The extra offending syscalls by that cgroup tasks are reduced.

> 
>> The feature is supposed to be used by sleepable BPF programs, I don't
>> think we need extra checks here?
> 
> Good.
> 
>> It could be that this BPF code runs in a process that is under
>> pod-x/container-y/cgroup-z/  and maybe you want to freeze "cgroup-z"
>> or "container-y" and so on... or in case of delegated hierarchies,
>> freezing the parent is a catch all.
> 
> OK, this would be good. Could it also be pod-x/container-y/cgroup-z2?
> 

Yes, basically the cgroup of the task, or any parent by fetching its
cgroup.

BPF is already the core interface of cgroup device controller.


Thanks!

> ---
> 
> I acknowledge that sooner or later some kind of access to cgroup through
> BPF will be added, I'd prefer if it was done in a generic way (so that
> it doesn't become cgroup's problem but someone else's e.g. VFS's or
> kernfs's ;-)).
> I can even imagine some usefulness of helpers for selected specific
> cgroup (core) operations (which is the direction brought up in the other
> discussion), I just don't think it solves the problem as you present it.
> 
> HTH,
> Michal