Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [V2][PATCH] cgroup: fix memory leak caused by missing
 cgroup_bpf_offline
To:     Ming Lei <ming.lei@redhat.com>
Cc:     Tejun Heo <tj@kernel.org>, Zefan Li <lizefan.x@bytedance.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Jens Axboe <axboe@kernel.dk>, Roman Gushchin <guro@fb.com>,
        mkoutny@suse.com, cgroups@vger.kernel.org,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        bpf@vger.kernel.org
References: <20211018075623.26884-1-quanyang.wang@windriver.com>
 <YW04Gqqm3lDisRTc@T590> <8fdcaded-474e-139b-a9bc-5ab6f91fbd4f@windriver.com>
 <YW1vuXh4C4tX9ZHP@T590>
From:   Quanyang Wang <quanyang.wang@windriver.com>
Message-ID: <bf88f85b-59a4-e2de-8b3a-70871e63adf6@windriver.com>
Date:   Tue, 19 Oct 2021 18:41:58 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
In-Reply-To: <YW1vuXh4C4tX9ZHP@T590>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NGcwZTNwZlRVZEQ1NVZzTTczVk1HRzM4NmxOL3diS3I2Tk9kcXVEM0dGR1Rl?=
 =?utf-8?B?bFI1dUZqWjZPQ3hlK240SW0xZWtVNHNtZkFmamJoSjB0RE5rSVpnaXhvZ0JB?=
 =?utf-8?B?MUFlbEZsN2Z2MlBBSlRnZzBHNG8wTlI3UmxIcjhrOU51ajVXMGpCdStwckI5?=
 =?utf-8?B?SkgyVm9yREhTczQ0SmlHTldtQy9nblNaR3NqT25QbHZkRjVTZjhYQlBwb1BO?=
 =?utf-8?B?azg2TEhPNHpucXJhMzJ5RTR5cUFLN3gvcTZSZVprV0laSHM0VEhCL1g1K2lR?=
 =?utf-8?B?TjA1eDBOY1d1QjJlVytTbW5SSWk2R2VkRHJKSWJrUWFBTThYQkgrMXh6R2ls?=
 =?utf-8?B?dEFncEpES3JoazgyTDhkUGlFTnpxODlBci9nditWc2lpQzBDRmtJZHR3Yzk2?=
 =?utf-8?B?M1RmaHB6b1pFbFJvcHlORzhzTXZjeTVNeHV2eHpMbFY3UjByeTBORXhVZlpm?=
 =?utf-8?B?UjZNcVd2SWdXeS9TWTJPTDZKandsMkF6bVppVXhhZTVGOGNOVFdDamJYbTlK?=
 =?utf-8?B?UDdlTjVFaHlDSVZYNXVZYWFOVStXMmg0Nk4zTEtDekVOazQ2MFJZNmNaS1Zm?=
 =?utf-8?B?Q0ovUFo5NDB5QU9yemYrSnhEMVZWZ2EvNXdyVFlHVEpvVmFSY1AzNEt0RjJ1?=
 =?utf-8?B?dHN6Q2NrcEoyYnhQNU9PN0hMUU1KTjBnQjY0d1Z1eFBlZ0M4OTNKZ0pHcWRM?=
 =?utf-8?B?akEvcDJGR2poc3MzQTVwVFBQOElJd0tac1lXVUg4dnJleGtMMlEwUm9zbnM1?=
 =?utf-8?B?Q1RkajVzYnRHdDJmbCtMdlJLejYvUnBRSHlpR2wzZStlSDhqallOTzRxNkZv?=
 =?utf-8?B?Z3hmWDFYOFUrTmxscGFVd2NGdHVHazR2dFI1M1ZoRTZCcHR0NzVKc0NaNStz?=
 =?utf-8?B?N1VsQmxLMW5JL2xYOEQvcHJGZHdIY1R1SnNLZ0l0NG9TNEEwTUtUN2U2VDcx?=
 =?utf-8?B?NktGeUZPcHAvV2lxeVpiZitndXl0S0VXVmtSK2dDVWVYeWVCUFFrY1JPMWxk?=
 =?utf-8?B?M3ZXc294SngyK3dMb1d4NFd5Z053TnY3K3NNa3R0RVJUd2RGeDFXYnBjaXlm?=
 =?utf-8?B?NDd6cUVHNCthcDNHdlVLY3ZCWXZWak1BNkRSRUxSRUNPbUQwZmRsYXcvZk5s?=
 =?utf-8?B?LzRwdlEvR2Fhd1FIZDl5N3NnZ245c3VpOGNnbnBlT0RDSFhuditSaytlRFY5?=
 =?utf-8?B?NzBXRGtIZ3ZWaUtiY1AzUVN3b2g2SVNUTUVHajl4QjJNcGdYenh0SUo4QmJG?=
 =?utf-8?B?eGFUVlV3bVR2MlV3aER2SHJQTnBENWxxSHd5RUdWNXJmTHZiMDRSUkgwWTZN?=
 =?utf-8?B?aGp0SnZhZDZvSFNJZi9hVlVXd08wT24wNEU0aVRhYmNIcWNzamZReVJkbW5H?=
 =?utf-8?B?R2VGSmowTG1pa1p0ZDhDbGx4M2FRM0hpSmNiRWVWdHlidWYvNGdYQXRKZXRW?=
 =?utf-8?B?U0JMeS8wTUJlWkNHQUtpc1NRc1c3WjBkV25TWmhkNHU2MDFSWGRCNXc0SWRV?=
 =?utf-8?B?OHVoZXc1M2J1blcrWVptWVNiSDY5SVk1Ky9wSlgxL1drRnRoQWhZbjFHeG0y?=
 =?utf-8?B?ZnoyWmdOSnMzZ2ZCV2I1MThkSUljVml1SDQ0UGh3ODl3WXBPeU8zQTBpZ21K?=
 =?utf-8?B?R0VDVkE1UG9raHdhejdNOXJSeGZHa1daaE5wQUhrL0dCS29TS1FUTnUyWTM4?=
 =?utf-8?B?NTAvZHV4cUlvbUljVWtpYm00U2EzcGJweklEbWE4ZHBaMWN6QWgvRVJVRS9U?=
 =?utf-8?Q?doYkzuIHLa5nNw8Ra280ScHExKgab4nAIUhEJfS?=
X-OriginatorOrg: windriver.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 980fec80-e956-4e27-c542-08d992ed1911
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB5819.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Oct 2021 10:42:07.6190
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ddb2873-a1ad-4a18-ae4e-4644631433be
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: jdsx7o/skDkgXx8r84prbJiNYZzoR4wuU1Bx2OWtpVmopPGO00NSBgoPzHKNe4jN4rlzAKk+ZnfnnQpLOM3dqNSmi1JWNfz7Kn79dobFbuE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5111
X-Proofpoint-ORIG-GUID: B2dOhPHdR6ZC-YaiKSt4KK1kQnBoj14V
X-Proofpoint-GUID: B2dOhPHdR6ZC-YaiKSt4KK1kQnBoj14V
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.182.1,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.0.607.475
 definitions=2021-10-18_10,2021-10-19_01,2020-04-07_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0 spamscore=0
 clxscore=1015 impostorscore=0 mlxlogscore=947 adultscore=0
 lowpriorityscore=0 phishscore=0 priorityscore=1501 bulkscore=0
 malwarescore=0 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2109230001 definitions=main-2110190065
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Ming,

On 10/18/21 8:59 PM, Ming Lei wrote:
> On Mon, Oct 18, 2021 at 06:06:28PM +0800, Quanyang Wang wrote:
>> Hi Ming,
>>
>> On 10/18/21 5:02 PM, Ming Lei wrote:
>>> On Mon, Oct 18, 2021 at 03:56:23PM +0800, quanyang.wang@windriver.com wrote:
>>>> From: Quanyang Wang <quanyang.wang@windriver.com>
>>>>
>>>> When enabling CONFIG_CGROUP_BPF, kmemleak can be observed by running
>>>> the command as below:
>>>>
>>>>       $mount -t cgroup -o none,name=foo cgroup cgroup/
>>>>       $umount cgroup/
>>>>
>>>> unreferenced object 0xc3585c40 (size 64):
>>>>     comm "mount", pid 425, jiffies 4294959825 (age 31.990s)
>>>>     hex dump (first 32 bytes):
>>>>       01 00 00 80 84 8c 28 c0 00 00 00 00 00 00 00 00  ......(.........
>>>>       00 00 00 00 00 00 00 00 6c 43 a0 c3 00 00 00 00  ........lC......
>>>>     backtrace:
>>>>       [<e95a2f9e>] cgroup_bpf_inherit+0x44/0x24c
>>>>       [<1f03679c>] cgroup_setup_root+0x174/0x37c
>>>>       [<ed4b0ac5>] cgroup1_get_tree+0x2c0/0x4a0
>>>>       [<f85b12fd>] vfs_get_tree+0x24/0x108
>>>>       [<f55aec5c>] path_mount+0x384/0x988
>>>>       [<e2d5e9cd>] do_mount+0x64/0x9c
>>>>       [<208c9cfe>] sys_mount+0xfc/0x1f4
>>>>       [<06dd06e0>] ret_fast_syscall+0x0/0x48
>>>>       [<a8308cb3>] 0xbeb4daa8
>>>>
>>>> This is because that since the commit 2b0d3d3e4fcf ("percpu_ref: reduce
>>>> memory footprint of percpu_ref in fast path") root_cgrp->bpf.refcnt.data
>>>> is allocated by the function percpu_ref_init in cgroup_bpf_inherit which
>>>> is called by cgroup_setup_root when mounting, but not freed along with
>>>> root_cgrp when umounting. Adding cgroup_bpf_offline which calls
>>>> percpu_ref_kill to cgroup_kill_sb can free root_cgrp->bpf.refcnt.data in
>>>> umount path.
>>>>
>>>> This patch also fixes the commit 4bfc0bb2c60e ("bpf: decouple the lifetime
>>>> of cgroup_bpf from cgroup itself"). A cgroup_bpf_offline is needed to do a
>>>> cleanup that frees the resources which are allocated by cgroup_bpf_inherit
>>>> in cgroup_setup_root.
>>>>
>>>> And inside cgroup_bpf_offline, cgroup_get() is at the beginning and
>>>> cgroup_put is at the end of cgroup_bpf_release which is called by
>>>> cgroup_bpf_offline. So cgroup_bpf_offline can keep the balance of
>>>> cgroup's refcount.
>>>>
>>>> Fixes: 2b0d3d3e4fcf ("percpu_ref: reduce memory footprint of percpu_ref in fast path")
>>>
>>> If I understand correctly, cgroup_bpf_release() won't be called without
>>> your patch. So anything allocated in cgroup_bpf_inherit() will be
>>> leaked?
>> No, for now cgroup_bpf_release is called to release bpf.refcnt.data of the
>> cgroup which is not root_cgroup. Only root_cgroup's bpf data is leaked.
> 
> You mean that cgroup_bpf_inherit() allocates nothing for root_cgroup?
> 
> If yes, I agree you can add 'Fixes: 2b0d3d3e4fcf', otherwise please
> remove it.
> 
> 
> Thanks,
> Ming
>