Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7cc82da3-04f6-55d4-6a0a-a88079de0e14@redhat.com>
Date:   Mon, 5 Jun 2023 22:32:40 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH] cgroup: fixed the cset refcnt leak when fork() failed
Content-Language: en-US
To:     Tejun Heo <tj@kernel.org>, Zou Cao <zoucaox@gmail.com>
Cc:     linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
        lizefan.x@bytedance.com, hannes@cmpxchg.org, brauner@kernel.org,
        Zou Cao <zoucao@kuaishou.com>
References: <20230605130444.1421-1-zoucao@kuaishou.com>
 <ZH5V5S5InM6b1iK7@slm.duckdns.org>
From:   Waiman Long <longman@redhat.com>
In-Reply-To: <ZH5V5S5InM6b1iK7@slm.duckdns.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 6/5/23 17:38, Tejun Heo wrote:
> On Mon, Jun 05, 2023 at 09:04:44PM +0800, Zou Cao wrote:
>> TeamID: B1486294
>>
>> 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);
>>    ......
>>       |
>> 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@kuaishou.com>
> Is this the same bug fixed by the following commit?
>
>   https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git/commit/?h=for-6.4-fixes&id=2bd110339288c18823dcace602b63b0d8627e520

I believe it is the same bug that this patch is trying to fix. I missed 
the part kargs->cset is cleared in cgroup_post_fork() so that the put 
can be done solely in cgroup_css_set_put_fork(). That is definitely the 
cleaner approach.

Cheers,
Longman