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 5576FC282EC for ; Mon, 17 Mar 2025 08:24:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0402D280004; Mon, 17 Mar 2025 04:24:39 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F3110280001; Mon, 17 Mar 2025 04:24:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DFB43280004; Mon, 17 Mar 2025 04:24:38 -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 C2DA1280001 for ; Mon, 17 Mar 2025 04:24:38 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 2BDC01A19FB for ; Mon, 17 Mar 2025 08:24:39 +0000 (UTC) X-FDA: 83230356678.09.BBC6787 Received: from invmail4.hynix.com (exvmail4.hynix.com [166.125.252.92]) by imf16.hostedemail.com (Postfix) with ESMTP id 34BBD180007 for ; Mon, 17 Mar 2025 08:24:36 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=none; spf=pass (imf16.hostedemail.com: domain of rakie.kim@sk.com designates 166.125.252.92 as permitted sender) smtp.mailfrom=rakie.kim@sk.com; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1742199877; 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-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=St4bdyTrihQyPzGZI9NcXxw7rOrJH4S6ya0Oqq0DIlM=; b=6NPTPRy12VO0cKnQaHGdnEDOpqfkzYuPCqvbJrGuY1W2rPtBGr7se3avzQ9ch91LxXl/QE E5jRDtg0DRCl+ON97ujIbG5+5KJKmq8Tt8iN9+izGyYeuHDHVxuKur0fF09MgVw/CiWRwH mMiY60pqf/lv4tIf+edwPyJAjJVGJrA= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1742199877; a=rsa-sha256; cv=none; b=ofMM2WVt83XB9v2ZThsO38WdUXU071m+ectWsWkMZ4f0aWz/hZ4ahjFXRze73n9hq0hjTr fRUmZgMXMqGGCE9+IUxOGho1lJRr0yK7C63KR4SXhWGjxZPWfRCGfp1UMyXvA+FnMxEL+z ZnlKqkEjdG1pk7ZY/nQ8AsORJIov5PU= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=none; spf=pass (imf16.hostedemail.com: domain of rakie.kim@sk.com designates 166.125.252.92 as permitted sender) smtp.mailfrom=rakie.kim@sk.com; dmarc=none X-AuditID: a67dfc5b-681ff7000002311f-4f-67d7dc4337dc From: Rakie Kim To: Jonathan Cameron Cc: Rakie Kim , akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-cxl@vger.kernel.org, joshua.hahnjy@gmail.com, dan.j.williams@intel.com, ying.huang@linux.alibaba.com, kernel_team@skhynix.com, honggyu.kim@sk.com, yunjeong.mun@sk.com, Gregory Price Subject: Re: [PATCH v2 1/4] mm/mempolicy: Fix memory leaks in mempolicy_sysfs_init() Date: Mon, 17 Mar 2025 17:24:23 +0900 Message-ID: <20250317082430.829-1-rakie.kim@sk.com> X-Mailer: git-send-email 2.48.1.windows.1 In-Reply-To: <20250314105500.00000157@huawei.com> References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrGLMWRmVeSWpSXmKPExsXC9ZZnoa7znevpBjPWcFnMWb+GzWL61AuM Fj/vHme3WLXwGpvF8a3z2C3OzzrFYnF51xw2i3tr/rNarF6T4cDpsXPWXXaP7rbL7B4tR96y eize85LJY9OnSeweJ2b8ZvHY+dDS4/MmuQCOKC6blNSczLLUIn27BK6Mni9P2QpOKlas33iC tYHxgGQXIweHhICJxISdwl2MnGDmsSXLWUHCbAJKEsf2xoCERQSMJN7dmMTYxcjFwSxwhUni 9bEWNpCEsECIxLpbi9lBbBYBVYkfn26zgNi8AsYSpy43MUPM1JRouHSPCcTmFDCUePlgO1i9 kACPxKsN+xkh6gUlTs58AtbLLCAv0bx1NjPIMgmB+2wS63b0MUEMkpQ4uOIGywRG/llIemYh 6VnAyLSKUSgzryw3MTPHRC+jMi+zQi85P3cTIzC0l9X+id7B+OlC8CFGAQ5GJR5eg/XX0oVY E8uKK3MPMUpwMCuJ8LLsuJ4uxJuSWFmVWpQfX1Sak1p8iFGag0VJnNfoW3mKkEB6Yklqdmpq QWoRTJaJg1OqgdHkhelBRoWbyrL+//a+UpnTxF+l2bd9lshKdfO8HsMv8x++U5v50HBH5RfT 0MCOgjO+StctTujeuL3dWfeec00EazFL5qJ1L28ZVCeuTHrDyR3FpyXq1HhcO+/yn0792HMs gekS87e4mTRLvlot+GH1SS0XUYXjlbzeW+Qsay4Hb9ddw2nwXImlOCPRUIu5qDgRABN4wzNp AgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrILMWRmVeSWpSXmKPExsXCNUNNS9fpzvV0g3XTbSzmrF/DZjF96gVG i593j7NbfH72mtli1cJrbBbHt85jtzg89ySrxflZp1gsLu+aw2Zxb81/VotD156zWqxek2Hx e9sKNgdej52z7rJ7dLddZvdoOfKW1WPxnpdMHps+TWL3ODHjN4vHzoeWHt9ue3gsfvGByePz JrkArigum5TUnMyy1CJ9uwSujJ4vT9kKTipWrN94grWB8YBkFyMnh4SAicSxJctZuxg5ONgE lCSO7Y0BCYsIGEm8uzGJsYuRi4NZ4AqTxOtjLWwgCWGBEIl1txazg9gsAqoSPz7dZgGxeQWM JU5dbmKGmKkp0XDpHhOIzSlgKPHywXaweiEBHolXG/YzQtQLSpyc+QSsl1lAXqJ562zmCYw8 s5CkZiFJLWBkWsUokplXlpuYmWOqV5ydUZmXWaGXnJ+7iREY0Mtq/0zcwfjlsvshRgEORiUe XoP119KFWBPLiitzDzFKcDArifCy7LieLsSbklhZlVqUH19UmpNafIhRmoNFSZzXKzw1QUgg PbEkNTs1tSC1CCbLxMEp1cC4cqbulFb/peJNa2rVXJruOi8/b+TN/kLlaX3hasbV3ad6f/06 GVw9bYmMHsf6dpmG6mdLVhe+tnvwatuKrM9lbo9MbMrTIl2kON9f0VD6vqTNIstxnvU8/h63 7k0KXkFyywq3T3imUfvOUai+7orh0yvJAbqLV3bMefxNdIfTLdPN80Jy7M8qsRRnJBpqMRcV JwIAZeOdBGQCAAA= X-CFilter-Loop: Reflected X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 34BBD180007 X-Stat-Signature: xp6bhfs8mo6jwcezebcy7kxgffe8ozew X-HE-Tag: 1742199876-680378 X-HE-Meta: U2FsdGVkX19k6BB9tef6w8TvzL0iT7vbYbSZ0gOB3Iwpk+l7J1lTESmaLkNYFC/HjDa9+HSl+QXF41J6ENYw8UNZIjUj8VLw3CXSopkAbhUK0zKEkQNS7Ag0P05CoZNhd4yv0rcuKYmlNqRADiZMNz+RzGHgjww10ID5Cg25/pNGtUbDdErY/Ep/E308SN24mG861T47P84AeRWtCQzlj/x1Eg9FQkjk4kFu5EwdN52JyjMeeXmF22IcXkkPTQ6eYDD71e8uo205ov++SIAt7BFG5bSi51qPGhD8CdG5KSk7tMCZEh+PbB0XQb81B9a7iP8gpuVvBUapzjLcUpI6lbMir7U6CZC30MlqXMxZ+E4AAN0kVLTe+zZfes1BOBPML6LHcz8ZjTOSTq2AR4iTBB5ItglGCCibUxK55vbQMDEhZjikrDdQMMun/OkEail53dJixJqhd8xwXGCkAVNU0QaH20lymdN86TIupeh7amNukqega1Hi9l9CEAUEnoitKT2dZY0eza8EwR+617qlZ1jf8J5gxGMTSOo2v2CNHc0W0oF9nqraR9LhLU/6tkPu2LzVyQccGVYJ7CgZFLBExc+Go/SVP0Y8nQ2VWYFVZJnico5Z+FNi0hlEnc2S8xB96torRQmW1sOG68SE6kEZEcYMuZqZBZZUDylAwMLFoe+nV7S6vKmpq9yRQ+M77ElB9Yb2benf+rCh3JyuIr7V5f23koqCSFzJ0Zt8u97PoQyRBJaZLKmaSo8w9MCN6n9ijVEDz9f5q2akyd695k5Iaxf1RZcM90UylAmWCpc1v5HVfJV9/2kZyq+t97QfBQC+COcYv6DsVY3+MW5KNLvnRhUv6rIr0egd+gDVU6cm1OYTFnnP9U9flkcHnB2oguoimhVogz4bbX9RdqJ/5wa+GdVOH1h2Pa9qailK+CMGyN/mT81clSUmTk2xcvRQOjVusmcdd5Q/vMats5yReWo oUMjspQt 4w92n6bNM0oP+8Gmu+PVkdbTRwafhHZPjqmP9idlFuZoKcDAKZ6PvdX7GmHVK7RKQiqFmhNOdwZqSPicFo3ysJWgs4Sc+vhgS3sX2qMatsx4yOM3VAGkNDPItqqJUX2L4XUUPL0c9Dn1+G5QjjogjIVbaPEF+tLAYYRQpTwuTCuFoA8Q= 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 Fri, 14 Mar 2025 10:55:00 +0000 Jonathan Cameron wrote: > On Thu, 13 Mar 2025 11:52:18 -0400 > Gregory Price wrote: > > > On Thu, Mar 13, 2025 at 03:31:38PM +0900, Rakie Kim wrote: > > > > Is this correct? If kobject_init_and_add fails, from other examples we > > > > need only free the mempolicy_kobj - because it failed to initialize and > > > > therefore should not have any references. I think this causes an > > > > underflow. > > > > > > Regarding the reordering of mempolicy_kobj allocation: > > > 1) In kobject_init_and_add(), kobject_init() is always called, which > > > > Quite right, mea culpa. > > > > > > > > 2) The release function for mempolicy_kobj is responsible for freeing > > > associated memory: > > > > > > static void mempolicy_kobj_release(struct kobject *kobj) > > > { > > > ... > > > kfree(ngrp->nattrs); > > > kfree(ngrp); > > > kfree(kobj); > > > } > > > > > > > I see what you're trying to do now after looking at the free-ordering > > at little closer. > > > > Lets do the following: > > > > 1) allocate node_attrs and mempolicy_kobj up front and keep your > > reordering, this lets us clean up allocations on failure before > > kobject_init is called > > > > 2) after this remove all the other code and just let > > mempolicy_kobj_release clean up node_attrs > > > > 3) Add a (%d) to the error message to differentiate failures > > Given how unlikely (and noisy) a memory allocation failure is, > maybe just drop the printing at all in those paths - allowing > early returns. > > The lifetime rules around node_attrs in here are making readability > poor. It is implicitly owned by the mempolicy_kobj, but no direct association. > Maybe just encapsulating the kobject in a structure that contains > this as a [] array at the end. Then we end up with single allocation of > stuff that is effectively one thing. > Hi Jonathan Thank you for your response regarding this patch. Your suggestions seem very appropriate. As you recommended, I will proceed to encapsulate node_attrs and mempolicy_kobj into a single structure. Rakie > > > > > This is a little bit cleaner and is a bit less code. (Not built or > > tested, just a recommendation). > > > > I'd recommend submitting this patch by itself to mm-stable, since the > > remainder of the patch line changes functionality and this fixes a bug > > in LTS kernels. > > > > ~Gregory > > > > --- > > > > > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > > index 530e71fe9147..05a410db08b4 100644 > > --- a/mm/mempolicy.c > > +++ b/mm/mempolicy.c > > @@ -3541,38 +3541,34 @@ static int __init mempolicy_sysfs_init(void) > > int err; > > static struct kobject *mempolicy_kobj; > > > > - mempolicy_kobj = kzalloc(sizeof(*mempolicy_kobj), GFP_KERNEL); > > - if (!mempolicy_kobj) { > > + node_attrs = kcalloc(nr_node_ids, sizeof(struct iw_node_attr *), > > + GFP_KERNEL); > > + if (!node_attrs) { > > err = -ENOMEM; > > goto err_out; > > } > > > > - node_attrs = kcalloc(nr_node_ids, sizeof(struct iw_node_attr *), > > - GFP_KERNEL); > > - if (!node_attrs) { > > + mempolicy_kobj = kzalloc(sizeof(*mempolicy_kobj), GFP_KERNEL); > > + if (!mempolicy_kobj) { > > err = -ENOMEM; > > - goto mempol_out; > > + kfree(node_attrs); > > + goto err_out; > > } > > > > err = kobject_init_and_add(mempolicy_kobj, &mempolicy_ktype, mm_kobj, > > "mempolicy"); > > if (err) > > - goto node_out; > > + goto mempol_out; > > > > err = add_weighted_interleave_group(mempolicy_kobj); > > - if (err) { > > - pr_err("mempolicy sysfs structure failed to initialize\n"); > > - kobject_put(mempolicy_kobj); > > - return err; > > - } > > + if (err) > > + goto mempol_out; > > > > - return err; > > -node_out: > > - kfree(node_attrs); > > + return 0; > > mempol_out: > > - kfree(mempolicy_kobj); > > + kobject_put(mempolicy_kobj); > > err_out: > > - pr_err("failed to add mempolicy kobject to the system\n"); > > + pr_err("mempolicy sysfs structure failed to initialize (%d)\n", err); > > return err; > > } > > > > >