Re: [PATCH] cgroup: fixed the cset refcnt leak when fork() failed

From: Waiman Long
Date: Mon Jun 05 2023 - 10:17:28 EST


On 6/5/23 09:04, Zou Cao wrote:
TeamID: B1486294
What is this?

when fork, cset will be increased by commit "ef2c41cf38a7", the refcnt will
be decrease by child exit, but when failed in fork(), this refcnt will
be lost decrease in cgroup_cancel_fork as follow:

copy_process
|
cgroup_can_fork // increase the css refcount
......
spin_lock_irq(&css_set_lock);
cset = task_css_setcurrent);
get_css_set(cset);
spin_unlock_irq&css_set_lock);
......

The code quoted above is actually in cgroup_css_set_fork(). You may want to list it as well.

I believe the problem is in

        if (!(kargs->flags & CLONE_INTO_CGROUP)) {
                kargs->cset = cset;
                return 0;
        }

When CLONE_INTO_CGROUP isn't set, a reference to the current cset is taken.

|
goto cgroup_cancel_fork // if failed in copy_process
|
cgroup_cancel_fork // lost the decrease refcount if flag not CLONE_INTO_CGROUP

Fixes: ef2c41cf38a7 ("clone3: allow spawning processes into cgroups")
Signed-off-by: Zou Cao <zoucao@xxxxxxxxxxxx>
---
kernel/cgroup/cgroup.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index d18c2ef..5ecd706 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -6284,6 +6284,11 @@ void cgroup_cancel_fork(struct task_struct *child,
if (ss->cancel_fork)
ss->cancel_fork(child, kargs->cset);
+ if (!(kargs->flags & CLONE_INTO_CGROUP) &&
+ kargs->cset) {
+ put_css_set(kargs->cset);
+ }
+
I believe the out_revert error path of cgroup_can_fork() has a similar issue. Perhaps you may want to put the put_css_set() call in cgroup_css_set_put_fork().
cgroup_css_set_put_fork(kargs);
}
Cheers,
Longman