From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id E25C3D1AD3B for ; Wed, 16 Oct 2024 10:16:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 726EB6B007B; Wed, 16 Oct 2024 06:16:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6D7D96B0082; Wed, 16 Oct 2024 06:16:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 577406B0083; Wed, 16 Oct 2024 06:16:59 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 390B26B007B for ; Wed, 16 Oct 2024 06:16:59 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 8000480818 for ; Wed, 16 Oct 2024 10:16:50 +0000 (UTC) X-FDA: 82679061864.26.0AFD679 Received: from fout-a7-smtp.messagingengine.com (fout-a7-smtp.messagingengine.com [103.168.172.150]) by imf06.hostedemail.com (Postfix) with ESMTP id A8B5D180016 for ; Wed, 16 Oct 2024 10:16:50 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=shutemov.name header.s=fm1 header.b="F 4DXBdQ"; dkim=pass header.d=messagingengine.com header.s=fm2 header.b="CSR/e5nZ"; spf=pass (imf06.hostedemail.com: domain of kirill@shutemov.name designates 103.168.172.150 as permitted sender) smtp.mailfrom=kirill@shutemov.name; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729073743; a=rsa-sha256; cv=none; b=WADZyIFkscEK/fMVN30IQdS8l1Z/iL91w5p4WHDqqyG6CJw1BOH55dW0hd72Mp1jzLhRzw ZtGh3pNF6jl8iiNBuzSSDwET9mRKehVOx8Rhb50QaFD3lANm2mM9VBbj671NQ7ixdMkgwv VTGiikYluHlAuuBgC7u5CUH5eCT63Cg= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=shutemov.name header.s=fm1 header.b="F 4DXBdQ"; dkim=pass header.d=messagingengine.com header.s=fm2 header.b="CSR/e5nZ"; spf=pass (imf06.hostedemail.com: domain of kirill@shutemov.name designates 103.168.172.150 as permitted sender) smtp.mailfrom=kirill@shutemov.name; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729073743; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=I0jjL1dUByTAPJLWxibZQUAh1Cxn8zTwcDHx7s3FQJQ=; b=CSEp1fo+t8/gGNdPFx4eGHqXiqJhiNbTPrgisiKTPvQ2By6Xzf/6FWqCo/3vpYv06RDvYq Tgb9QI6qLicDYvswI8Gw4n5ROQBZ+HOn02kn/VB5Buf4tp4PDJPyO5ZRa10UpaBKdRs1jB zf6Ypqwxftc1Y6NZoP+DEUlQ8lbb3vs= Received: from phl-compute-01.internal (phl-compute-01.phl.internal [10.202.2.41]) by mailfout.phl.internal (Postfix) with ESMTP id 185311380167; Wed, 16 Oct 2024 06:16:56 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-01.internal (MEProxy); Wed, 16 Oct 2024 06:16:56 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=shutemov.name; h=cc:cc:content-type:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to; s=fm1; t=1729073816; x= 1729160216; bh=I0jjL1dUByTAPJLWxibZQUAh1Cxn8zTwcDHx7s3FQJQ=; b=F 4DXBdQ4L63cEECudCAjaPmz4dKzKgbVwlL+fM9T6CTJ3pbmNRFaOpZV1AkJx4WLo qiUrWIRMxEAcmUJXlFmSiNt9e9dyGW9IFswsUbQ0v34FVIygWPM7oBjAIcR0HKjj Vxbvg3yq5bZkE1m6Bc1DDrQsdlpgMu4wudZHY3RnYMC3F0eKzywWol8p4kOtdIiV UkAGyn/TmAoDJXDRYnRAV2oKz1LU2WHC3PhnXKyFyOZwB+vPzQi11Kx83Od4IajX EITwde4amfOfkSR7OfAp1/jnTH5wCNBDC2FcYNaK8ltwVgzJbfsytFmD+viqZd8W aBGiy9Yr/DLuEA+UFIMjw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1729073816; x=1729160216; bh=I0jjL1dUByTAPJLWxibZQUAh1Cxn 8zTwcDHx7s3FQJQ=; b=CSR/e5nZNBZ+9RBMcM5OvTo5qh74/TnhyM4NPPt3JpDe J8IzYhz5NBzCD4h/gpuCREBygxqsgRszcC42iPQNfjRnQYeSPxoYOQGEHkSfH8rd 7rQtGzS0FryrS//FrAPMmaF1GBNBBGQXlT1LorJboiED4qxuPQoJOZiD25xAxpxi Jsu7X6xNaaiwjIQYQWbxEyO0WjafL9IMgLZCEKh0WyPgzVJ5rG9WX+Bb9myQEpQS FbbDckStNL4SpnyQiSYXhFZEfqQ4gwC2JCdZRWcZ/lw9z8EMvvy4BlSqlqHAW1x4 zS3g9TpOyFxmCLTd3SXHQlDDxNbR535YYbzXFVlyRA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrvdegledgvdejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtsfdttddtvden ucfhrhhomhepfdfmihhrihhllhcutedrucfuhhhuthgvmhhovhdfuceokhhirhhilhhlse hshhhuthgvmhhovhdrnhgrmhgvqeenucggtffrrghtthgvrhhnpeffvdevueetudfhhfff veelhfetfeevveekleevjeduudevvdduvdelteduvefhkeenucevlhhushhtvghrufhiii gvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehkihhrihhllhesshhhuhhtvghmohhv rdhnrghmvgdpnhgspghrtghpthhtohepuddtpdhmohguvgepshhmthhpohhuthdprhgtph htthhopehmuhgthhhunhdrshhonhhgsehlihhnuhigrdguvghvpdhrtghpthhtoheptghh vghnrhhiughonhhgsehhuhgrfigvihdrtghomhdprhgtphhtthhopegrnhhshhhumhgrnh drkhhhrghnughurghlsegrrhhmrdgtohhmpdhrtghpthhtoheprghkphhmsehlihhnuhig qdhfohhunhgurghtihhonhdrohhrghdprhgtphhtthhopegurghvihgusehfrhhomhhorh gsihhtrdgtohhmpdhrtghpthhtohepiihhvghnghhqihdrrghrtghhsegshihtvggurghn tggvrdgtohhmpdhrtghpthhtoheprhhomhgrnhdrghhushhhtghhihhnsehlihhnuhigrd guvghvpdhrtghpthhtoheplhhinhhugidqmhhmsehkvhgrtghkrdhorhhgpdhrtghpthht oheplhhinhhugidqkhgvrhhnvghlsehvghgvrhdrkhgvrhhnvghlrdhorhhg X-ME-Proxy: Feedback-ID: ie3994620:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 16 Oct 2024 06:16:51 -0400 (EDT) Date: Wed, 16 Oct 2024 13:16:46 +0300 From: "Kirill A. Shutemov" To: Muchun Song Cc: chenridong , Anshuman Khandual , akpm@linux-foundation.org, david@fromorbit.com, zhengqi.arch@bytedance.com, roman.gushchin@linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org, wangweiyang2@huawei.com Subject: Re: [PATCH v2] mm: shrinker: avoid memleak in alloc_shrinker_info Message-ID: References: <20241014032336.482088-1-chenridong@huaweicloud.com> <4a18e997-3a94-4248-8923-c3764d12b0d6@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Stat-Signature: zgdmmam35tpcbjxj8fuubdazaftb5wt4 X-Rspamd-Queue-Id: A8B5D180016 X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1729073810-706437 X-HE-Meta: U2FsdGVkX1/gONlV43An5/I7ljfvssDBX3AbDOeqaw2tLisbwjl9nGgZEQe/5qhoa4WA0iBs/fjvmRn0mEaeWYLJBYI1ohFXV84wKEF9b/X2yMpkOJAHhBLmO3TDQc1i7NfUH46mRXzaUzNa78cAOPMyKnqdGrTPdx64i1Ql1NbfCVC+KavwsI6QVj41iiQ0ZGE3E66Nf5+4gCN6OB0ZNzG0KBdfqDAmjJoTs87LTriuDr6prb0NgkPcVF38kzFb0cAFBu6gF/u5AGjlAGEQmVxkjreOw2iTaT5Y7f30r+g2iFtrPSTYNhAHCDeRFPchk4T7OnlJADIzWotyI+VdZj6rbypxBN4lrZALby34tOjeuX0tpQW5bL2NLOhjQbY/DQfZVJUJ8Y594uTGCCXjXxreBilDUG0NshC+soomeiPz1/xDMXeuQtKB7Aa4JUbNYVpmbdyiC/usC85C1JeiojwARJlyslWvLETExhH4sUysihPg0fe9hDV9d96+K0W3SWVS6fIQ67VUhkVBwshFZaqeEgYEOZIfW2yhUgPHwjwK3oUfpiQg9DWq94+xJCW8wunCcv3SWKtmikaQWImYvOMojyHF/BF1tmNgfeFKECpnv4MhSBQ3Qrm2gmSbzq86DveInaf0ppi2IwRE11HdkD/dft2/toOLVy+8OBUYJNVBSaOMLA2JYD47UlztmFofCQPGBzjDjwQvpAptFAqVUFW/EojVDOGmn7A41u1i93nQ+kcp+bjYCb1vtzaLeBIQbfMB3304Unx2F5o4yPbOEomR5VfXbvtybQAm5mcL6F4TRuy1RrNmT+jL0qjgR49Rv+lDXEtU2eRaCxMuqbPmoXiI7D5VQVHXdOXqvUhs+5VcO2xo5QsGnPngpcTXiHzKGE1AsaZRP5iA7vXo6DaXKDmO7PTlzecC7x2Acu07/YUUvl6bMG0L05mVFqqOEQv4ZqQ2UdQbKZiOUH8d/AA q3DVYOef VxXCBnDMhxGnjpTcBRAJE8B+2SN8VHKmD3fKRz9NLLrVe1tWGXno4Fq6Kg/NMT92N+f72mTf8dYRqlIzhn7TTa3ZSvNJQ/jeqhUJ9QxZ1nf3p3pgpYRUXRWdbAopKt/gKjp8Bw6OLL1B2JAEaDCcXN6NwNNo+AGX9+6qediTlrld7aeaPgtm1uvWbuwRk5N/k/PBecelqyWPqsLQZnrPgLuDUjFsT8SFeX6owq0jMm9oduZKzWscwo1Zlxr1o5Idvlk+VWLHAoQ5yd1hZM/MQoEq4ZTOADReMeqSh5DG0u5vR7J2+sv3lEQHBcEGQDDk03M4eR95Nzy724xkLWuR5jIzjr8f9Dlqzlh1Ag7PVSaryrVVsXtevI+zkbwE68qr2W3Kba+aYZOXgNY4OII7fxq3yMbliRPHhEDR3KmY/TCJKeeRVgN70WUj5mA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Wed, Oct 16, 2024 at 10:21:30AM +0800, Muchun Song wrote: > > > > On Oct 16, 2024, at 09:25, chenridong wrote: > > > > > > > > On 2024/10/15 14:55, Anshuman Khandual wrote: > >> On 10/14/24 16:59, Kirill A. Shutemov wrote: > >>> On Mon, Oct 14, 2024 at 03:23:36AM +0000, Chen Ridong wrote: > >>>> From: Chen Ridong > >>>> > >>>> A memleak was found as bellow: > >>>> > >>>> unreferenced object 0xffff8881010d2a80 (size 32): > >>>> comm "mkdir", pid 1559, jiffies 4294932666 > >>>> hex dump (first 32 bytes): > >>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > >>>> 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 @............... > >>>> backtrace (crc 2e7ef6fa): > >>>> [] __kmalloc_node_noprof+0x394/0x470 > >>>> [] alloc_shrinker_info+0x7b/0x1a0 > >>>> [] mem_cgroup_css_online+0x11a/0x3b0 > >>>> [] online_css+0x29/0xa0 > >>>> [] cgroup_apply_control_enable+0x20d/0x360 > >>>> [] cgroup_mkdir+0x168/0x5f0 > >>>> [] kernfs_iop_mkdir+0x5e/0x90 > >>>> [] vfs_mkdir+0x144/0x220 > >>>> [] do_mkdirat+0x87/0x130 > >>>> [] __x64_sys_mkdir+0x49/0x70 > >>>> [] do_syscall_64+0x68/0x140 > >>>> [] entry_SYSCALL_64_after_hwframe+0x76/0x7e > >>>> > >>>> In the alloc_shrinker_info function, when shrinker_unit_alloc return > >>>> err, the info won't be freed. Just fix it. > >>>> > >>>> Fixes: 307bececcd12 ("mm: shrinker: add a secondary array for shrinker_info::{map, nr_deferred}") > >>>> Signed-off-by: Chen Ridong > >>>> --- > >>>> mm/shrinker.c | 1 + > >>>> 1 file changed, 1 insertion(+) > >>>> > >>>> diff --git a/mm/shrinker.c b/mm/shrinker.c > >>>> index dc5d2a6fcfc4..92270413190d 100644 > >>>> --- a/mm/shrinker.c > >>>> +++ b/mm/shrinker.c > >>>> @@ -97,6 +97,7 @@ int alloc_shrinker_info(struct mem_cgroup *memcg) > >>>> err: > >>>> mutex_unlock(&shrinker_mutex); > >>>> + kvfree(info); > >>>> free_shrinker_info(memcg); > >>>> return -ENOMEM; > >>>> } > >>> > >>> NAK. If in the future there going to one more error case after > >>> rcu_assign_pointer() we will end up with double free. > >>> > >>> This should be safer: > >>> > >>> diff --git a/mm/shrinker.c b/mm/shrinker.c > >>> index dc5d2a6fcfc4..763fd556bc7d 100644 > >>> --- a/mm/shrinker.c > >>> +++ b/mm/shrinker.c > >>> @@ -87,8 +87,10 @@ int alloc_shrinker_info(struct mem_cgroup *memcg) > >>> if (!info) > >>> goto err; > >>> info->map_nr_max = shrinker_nr_max; > >>> - if (shrinker_unit_alloc(info, NULL, nid)) > >>> + if (shrinker_unit_alloc(info, NULL, nid)) { > >>> + kvfree(info); > >>> goto err; > >>> + } > >>> rcu_assign_pointer(memcg->nodeinfo[nid]->shrinker_info, info); > >>> } > >>> mutex_unlock(&shrinker_mutex); > >> Agreed, this is what I mentioned earlier as well. > >> ------------------------------------------------------------------ > >> I guess kvfree() should be called just after shrinker_unit_alloc() > >> fails but before calling into "goto err" > >> ------------------------------------------------------------------ > > > > After discussion, it seems that v1 is acceptable. > > Hi, Muchun, do you have any other opinions? > > I insist on my opinion, not mixing two different approaches > to do release resources. It makes no sense. This kvfree() is specifically to handle the case when 'info' is allocated, but not yet assigned to ->shrinker_info. And 'err:' block handles all other error cases. Putting kvfree() in 'err:' section is double-free timebomb. -- Kiryl Shutsemau / Kirill A. Shutemov