Bug code flow:
cset X's initial refcount is 1.
1. cgroup_attach_task()
2. [For thread1]
cgroup_migrate_add_src()
[For thread2]
cgroup_migrate_add_src()
cset X is thread2's src_cset , ref->2,
and its mg_preload_node is added to
mgctx->preloaded_src_csets.
3. cgroup_migrate_prepare_dst()
[For thread1]
find_css_set()
cset X is thread1's dst_cset, ref->3
put_css_set()
ref->2 because cset X's mg_preload_node is not
empty(already in mgctx->preloaded_src_csets).
[For thread2]
find_css_cset()
cset X is also thread2's dst_cset, ref->3
then drop src_cset, ref->1
[cgroup_free] ref->0
4. cgroup_migrate_execute
[For thread1]
ref -> 0xc0000000(UAF)
The issue occurs because cset X's refcount should
not be substraced by one in this case. So mark a flag
'multiidentity' to this cset and call put_css_set()
one more time if 'multiidentity' marked in cleanup
progress.
Signed-off-by: shisiyuan <[email protected]>
---
include/linux/cgroup-defs.h | 5 +++++
kernel/cgroup/cgroup.c | 23 +++++++++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 8c19303..c9f2237 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -280,6 +280,11 @@ struct css_set {
/* For RCU-protected deletion */
struct rcu_head rcu_head;
+
+ /* Indicate if the css_set is the src_cset of one thread and
+ * the dst_cset of anonther thread during migration process.
+ */
+ bool multiidentity;
};
struct cgroup_base_stat {
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index e914646..cfdd12e 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -2593,6 +2593,10 @@ void cgroup_migrate_finish(struct cgroup_mgctx *mgctx)
cset->mg_dst_cgrp = NULL;
cset->mg_dst_cset = NULL;
list_del_init(&cset->mg_preload_node);
+ if (cset->multiidentity == true) {
+ cset->multiidentity = false;
+ put_css_set_locked(cset);
+ }
put_css_set_locked(cset);
}
@@ -2648,6 +2652,18 @@ void cgroup_migrate_add_src(struct css_set *src_cset,
list_add_tail(&src_cset->mg_preload_node, &mgctx->preloaded_src_csets);
}
+static bool list_contains(struct list_head *node, struct list_head *head)
+{
+ struct list_head *tmp;
+
+ list_for_each(tmp, head) {
+ if (tmp == node)
+ return true;
+ }
+
+ return false;
+}
+
/**
* cgroup_migrate_prepare_dst - prepare destination css_sets for migration
* @mgctx: migration context
@@ -2700,6 +2716,13 @@ int cgroup_migrate_prepare_dst(struct cgroup_mgctx *mgctx)
if (list_empty(&dst_cset->mg_preload_node))
list_add_tail(&dst_cset->mg_preload_node,
&mgctx->preloaded_dst_csets);
+ /* If dst_cset is already in mgctx->preloaded_src_csets,
+ * it means the dst_cset has multi identities. Keep it
+ * in mgctx->preloaded_src_csets, and mark it.
+ */
+ else if (list_contains(&dst_cset->mg_preload_node,
+ &mgctx->preloaded_src_csets))
+ dst_cset->multiidentity = true;
else
put_css_set(dst_cset);
--
2.7.4
Hello.
On Fri, Jun 03, 2022 at 12:34:48AM +0800, shisiyuan <[email protected]> wrote:
> Bug code flow:
> cset X's initial refcount is 1.
> 1. cgroup_attach_task()
> 2. [For thread1]
> cgroup_migrate_add_src()
> [For thread2]
> cgroup_migrate_add_src()
> cset X is thread2's src_cset , ref->2,
> and its mg_preload_node is added to
> mgctx->preloaded_src_csets.
> 3. cgroup_migrate_prepare_dst()
> [For thread1]
> find_css_set()
> cset X is thread1's dst_cset, ref->3
> put_css_set()
> ref->2 because cset X's mg_preload_node is not
> empty(already in mgctx->preloaded_src_csets).
> [For thread2]
> find_css_cset()
> cset X is also thread2's dst_cset, ref->3
> then drop src_cset, ref->1
> [cgroup_free] ref->0
> 4. cgroup_migrate_execute
> [For thread1]
> ref -> 0xc0000000(UAF)
I'm trying to understand when this happens.
You migrate a process with two threads while one of them exits?
This should be properly synchronized with cgroup_threadgroup_rwsem, so I
don't understand where does the [cgroup_free] between 3. and 4. come
from.
Do you have a reproducer for this?
Thanks,
Michal
Hello.
On Thu, Jun 09, 2022 at 11:49:38AM +0800, 史思远 <[email protected]> wrote:
> The process is like above photo, thread 2 exits
> between cgroup_migrate_prepare_dst() and cgroup_migrate_execute().
> Then the refcount of csetX turns to be 0 here, and UAF appears when thread1
> migrating.
> Thread2 exits asynchronously, can rwsem prevent it?
See the bailout in cgroup_migrate_add_task():
if (task->flags & PF_EXITING)
return;
And cgroup_threadgroup_change_begin(tsk) in exit_signals().
> The purpose of my patch is to keep csetX's refcount still 1 after thread2
> exits, and make sure thread1 migrating successfully.
Why is not src_cset==dst_cset in cgroup_migrate_prepare_dst() not
sufficient?
Still, can this be reproduced in real world or is your reasoning based
on theory only?
Thanks,
Michal