[PATCH] cgroup: Improve cgroup_addrm_files remove files handling

[Wenyu Liu]

For now cgroup_apply_cftypes(cfts, is_add) with `is_add` false
will remove all the cftype files in `cfts` array, this can lead
to some unexpected behaviors in some abnormal situation.

Consider this situation: if we have two cftype arrays A and B
which contain the exact same files, and we add this two cftypes
with cgroup_add_cftypes().

We can correctly add files from A, but adding B will delete all
files previously added from A.

When adding cftype files of B:
cgroup_add_cftypes
  ->cgroup_apply_cftypes
      ->cgroup_addrm_files (failed with -EEXIST)
  ->cgroup_rm_cftypes_locked (this will delete all files added from A)

Even worse thing is that if we add B again at this point,
we can successfully create these files, but there will be two cftys
nodes (A and B) on the ss->cftys list at the same time.

    ss        A|0|1|2|3|...|   B|0|1|2|3|...|
 +------+       |                |
 |      |       +          +-----+
 +------+       |          |
 | cfts |<-->|node|<--->|node|<--->|...|
 +------+

This will lead to all hierarchies that apply this ss controller
to be unable to create any directory:
cgroup_mkdir
  ->cgroup_apply_control_enable
    ->css_populate_dir
      ->cgroup_addrm_files (will return -EEXIST when handling node of B)

Add a new flag __CFTYPE_ADDRM_END to track the end cft if something
wrong with cgroup_addrm_files() add files, make sure we only remove
the cftype files that were successfully added.

Signed-off-by: Wenyu Liu <liuwenyu.0311@xxxxxxxxxxxxx>
---
 include/linux/cgroup-defs.h | 1 +
 kernel/cgroup/cgroup.c      | 8 ++++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 93318fce31f3..7ad98048ca23 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -144,6 +144,7 @@ enum {
 	__CFTYPE_ONLY_ON_DFL	= (1 << 16),	/* only on default hierarchy */
 	__CFTYPE_NOT_ON_DFL	= (1 << 17),	/* not on default hierarchy */
 	__CFTYPE_ADDED		= (1 << 18),
+	__CFTYPE_ADDRM_END  = (1 << 19),
 };
 
 enum cgroup_attach_lock_mode {
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 6ae5f48cf64e..0d7d3079e635 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -4453,13 +4453,13 @@ static int cgroup_addrm_files(struct cgroup_subsys_state *css,
 			      struct cgroup *cgrp, struct cftype cfts[],
 			      bool is_add)
 {
-	struct cftype *cft, *cft_end = NULL;
+	struct cftype *cft;
 	int ret = 0;
 
 	lockdep_assert_held(&cgroup_mutex);
 
 restart:
-	for (cft = cfts; cft != cft_end && cft->name[0] != '\0'; cft++) {
+	for (cft = cfts; !(cft->flags & __CFTYPE_ADDRM_END) && cft->name[0] != '\0'; cft++) {
 		/* does cft->flags tell us to skip this file on @cgrp? */
 		if ((cft->flags & __CFTYPE_ONLY_ON_DFL) && !cgroup_on_dfl(cgrp))
 			continue;
@@ -4476,7 +4476,7 @@ static int cgroup_addrm_files(struct cgroup_subsys_state *css,
 			if (ret) {
 				pr_warn("%s: failed to add %s, err=%d\n",
 					__func__, cft->name, ret);
-				cft_end = cft;
+				cft->flags |= __CFTYPE_ADDRM_END;
 				is_add = false;
 				goto restart;
 			}
@@ -4526,7 +4526,7 @@ static void cgroup_exit_cftypes(struct cftype *cfts)
 
 		/* revert flags set by cgroup core while adding @cfts */
 		cft->flags &= ~(__CFTYPE_ONLY_ON_DFL | __CFTYPE_NOT_ON_DFL |
-				__CFTYPE_ADDED);
+				__CFTYPE_ADDED | __CFTYPE_ADDRM_END);
 	}
 }
 
-- 
2.39.3 (Apple Git-146)