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 09C44C282DE for ; Thu, 13 Mar 2025 06:33:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 50694280002; Thu, 13 Mar 2025 02:33:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4B63E280001; Thu, 13 Mar 2025 02:33:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 37E18280002; Thu, 13 Mar 2025 02:33:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 19CCB280001 for ; Thu, 13 Mar 2025 02:33:10 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id E811A57B73 for ; Thu, 13 Mar 2025 06:33:10 +0000 (UTC) X-FDA: 83215560540.04.A77B1CE Received: from invmail4.hynix.com (exvmail4.skhynix.com [166.125.252.92]) by imf21.hostedemail.com (Postfix) with ESMTP id 645BD1C000F for ; Thu, 13 Mar 2025 06:33:08 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=none; spf=pass (imf21.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=1741847589; 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=qGwT+dk0s7WoIm+zQaVNmvsSkFNftx/jxT1CR4whA/E=; b=dj9cz9VcR/YGACmmEOSGVLvRtq51OVP9arVTgXTYMlNJ1HqHGLfr49AB3JAlH9+ZohRSbl Da0dKIQzVXxai+hnEe0vk9fXZkpSHehewknpo9Ic5CjjKl+ZpoUHetDWfvXAEkfg2tD2OX eyMVr4Ii6Ft+TAxQOyTVc/Lz6mraMhY= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741847589; a=rsa-sha256; cv=none; b=Rv0gFBscnsQstM1qRynOJfrYcWa6mU0t8DfOHuW0j/QbyXLVOH5kb3YqENpOdHPFVTyhNp RBfRvN6QPf2rhApkzz/TwF2xIHfwqkd4NC+mbYmElzyoZrg3YJIh8mqJQ6jhCz2b9y7gIA g5OotVUpafI1CB8MkU5jhcRLZBU0Q0s= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=none; spf=pass (imf21.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-669ff7000002311f-fc-67d27c215fcf From: Rakie Kim To: Gregory Price Cc: 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, Rakie Kim Subject: Re: [PATCH v2 1/4] mm/mempolicy: Fix memory leaks in mempolicy_sysfs_init() Date: Thu, 13 Mar 2025 15:31:38 +0900 Message-ID: <20250313063247.681-1-rakie.kim@sk.com> X-Mailer: git-send-email 2.48.1.windows.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrNLMWRmVeSWpSXmKPExsXC9ZZnoa5izaV0g0XrLS3mrF/DZjF96gVG i593j7NbHN86j93i/KxTLBaXd81hs7i35j+rxeo1GQ4cHjtn3WX36G67zO6xeM9LJo9Nnyax e5yY8ZvFY+dDS4/Pm+QC2KO4bFJSczLLUov07RK4Mn7/Pcde8ESlYuG/dawNjHtkuhg5OSQE TCS+fJ/JBmMv2raZvYuRg4NNQEni2N4YEFNEQFWi7Yp7FyMXB7PAeiaJ15tmgZULC4RIrLu1 GKycBaim8SLYRF4BY4n+n2sZISZqSjRcuscEYnMKmElc+/WaBcQWEuCReLVhPyNEvaDEyZlP wOLMAvISzVtnM4PskhA4wSYxc+oTJohBkhIHV9xgmcDIPwtJzywkPQsYmVYxCmXmleUmZuaY 6GVU5mVW6CXn525iBAbxsto/0TsYP10IPsQowMGoxMObMPliuhBrYllxZe4hRgkOZiUR3tW2 F9KFeFMSK6tSi/Lji0pzUosPMUpzsCiJ8xp9K08REkhPLEnNTk0tSC2CyTJxcEo1MBqfUzul Ly1ouuPoxFM7/mwQXrdw1p5slTmFN7MbXA8+ueUewPnZ9uHUh4m8tdMz7Ot2/bU8e9q2fPc8 HafbOj+3OnxvbFvsNSnaVcIwpM+ZSTj7jdO5pm+uF1VDU64yPBGMkFiwQqg0wd+zY4575e/D Xz9sYTp/9e+bL7JfQ/LVU1PYQuakrVRiKc5INNRiLipOBADkTu32XgIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrKLMWRmVeSWpSXmKPExsXCNUNNS1ex5lK6wYHjhhZz1q9hs5g+9QKj xc+7x9ktPj97zWxxfOs8dovDc0+yWpyfdYrF4vKuOWwW99b8Z7U4dO05q8XqNRkWv7etYHPg 8dg56y67R3fbZXaPxXteMnls+jSJ3ePEjN8sHjsfWnp8u+3hsfjFByaPz5vkAjijuGxSUnMy y1KL9O0SuDJ+/z3HXvBEpWLhv3WsDYx7ZLoYOTkkBEwkFm3bzN7FyMHBJqAkcWxvDIgpIqAq 0XbFvYuRi4NZYD2TxOtNs9hAyoUFQiTW3VoMVs4CVNN4EWwKr4CxRP/PtYwQEzUlGi7dYwKx OQXMJK79es0CYgsJ8Ei82rCfEaJeUOLkzCdgcWYBeYnmrbOZJzDyzEKSmoUktYCRaRWjSGZe WW5iZo6pXnF2RmVeZoVecn7uJkZg4C6r/TNxB+OXy+6HGAU4GJV4eBMmX0wXYk0sK67MPcQo wcGsJMK72vZCuhBvSmJlVWpRfnxRaU5q8SFGaQ4WJXFer/DUBCGB9MSS1OzU1ILUIpgsEwen VAPj+gq5wCk+R+z0RT6mVTzMkP5VtWpfXsnmG49+Kljeu7D5d5iKrpcd4yZ7u58SB3vfdsju esR1IETi68UVildUVY2e+pZmfWj+uegZI4/9ko6ubS/LFhXv/VGyVtOvvz7m372NL1xU3+rl 3A6ZlauTb9xw3cJw2baHOz7tT/3+rHuJ7nXrFNNeJZbijERDLeai4kQA/UUOuFgCAAA= X-CFilter-Loop: Reflected X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 645BD1C000F X-Stat-Signature: 9jfe6oubmqhzcho49f36xd3pww99me1m X-HE-Tag: 1741847588-703119 X-HE-Meta: U2FsdGVkX1/VdJJuixRy+cX3lNdszm0AMwWgcBYUsbo9wjy5XBKeW1E/JwhiNMTrhYxloDP0vGv6ZTxPRC+duzNP60a+i27cQmQk9fKGbGdeUdgihEm8fHLoJ25kkTe6bR1gTxwJ08DZYm1EMEj6ZN8TaulbkQhshaz/AZ9y47RMvHY3zW+n4K3y7IduRqJzs90Xb9egXSgTYRZVXS51K0e6XRB0nF93sjiUrHa2SLBIlP2t8YHBiF1qmYXcjRDcqm5izN8r3wNEix33hMfrWr2bLRLDhLkZu2pV+iGUul5LZW16Jb6L4pVHPi0gQIBB4gOXASbM/uNp1icQdP5Nmll3i8o122v7HvpTDWXBGNCEK5yIfV8kMNjreaNxAARTuJdfDh64xSbJka+ygBfFoYgBTAiaUrNq5lGmDjCQgbqoDnPNmRxs5nQBQpU6EwKJU2qnRauT8FmeoEz3BjK71f3Ats0pNYLyDlJGrU3PIfonwXJDQakVhWt0rt0P/UNvZ2WMA1XF2rLfZoagU//hnXOGh2q38UFtvNCcNXffxVZ87vosvxpVEsAJ1GojNYataAxKCDK0dIWmeoI/ZHY2QYE24/pW3x4BgTUJlZSNdJ593Xs/JijZpeZbpgZM4qQ6B9J2ysHmim3AApK1BEHktyx8L7fFLlhVvGmdGsVgZLPjrL4l5HKyQoaQ3pD7f/3YDfoR4Pj7HEwHwJoyR8F901cdtZql7D+7yMS6Qibksc2z5GtNKOzfF46eHaz+wgvC4SyID7XZAld6kPWwiLrAyoqw4K2RyIj0m/57bGQwI8Fo4dHfcC0JqMzCkuBKJgpTx9Rno/0hztRVpvnOBGFzhnsBBppQ8UyZCcbUNAhNXXLC9JrHazApxoxg89ztHA867i9tI/yTKuPzW6yCiC+Db6JD1CzV5Iajm00MLRFCVOE0Lwiv6bFgyya8T1C0Ura2BmQSkFTX9AYu7ZPHuiB TdCPIAa7 /rhLNaZyF96pqmVpQ3+2MxXyckvIgeZy1reZcRENNuNOpxkIox3LsnrAvxC0s8EJFdCb9Q5DF+wC5kodj7eV56OVZEjwUIQU+j4BRDzIVzVx2QP3UukYoOTHdrBkep/LAUryuvs+nzAwMG9rlWCfv24iD+MjoidFUT5kgHXmaijgORnD5pwnEnsCITj86qZPNU9hL08Q6dD+TqCc= 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, 12 Mar 2025 11:49:06 -0400 Gregory Price wrote: Hi Gregory Thank you for your response regarding this patch. > On Wed, Mar 12, 2025 at 04:56:24PM +0900, Rakie Kim wrote: > > Improper cleanup of sysfs attributes caused kobject and memory leaks when > > initialization failed or nodes were removed. > > > > This patch ensures proper deallocation of kobjects and memory, preventing > > resource leaks and improving stability. > > > > This patch does multiple things, please split these changes into > multiple patches. This patch should remain as a single patch since all changes address kobject-related memory issues in mempolicy_sysfs_init(). If you still believe it should be split, I would appreciate your suggestion on which parts should be separated. > > > Fixes: dce41f5ae253 ("mm/mempolicy: implement the sysfs-based weighted_interleave interface") > > Signed-off-by: Rakie Kim > > --- > > mm/mempolicy.c | 29 +++++++++++++++-------------- > > 1 file changed, 15 insertions(+), 14 deletions(-) > > > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > > index bbaadbeeb291..1691748badb2 100644 > > --- a/mm/mempolicy.c > > +++ b/mm/mempolicy.c > > @@ -3541,39 +3541,40 @@ 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; > > + goto node_out; > > } > > It's not clear to me why you re-ordered these allocations, it seems > superfluous. > > > > > err = kobject_init_and_add(mempolicy_kobj, &mempolicy_ktype, mm_kobj, > > "mempolicy"); > > - if (err) > > - goto node_out; > > + if (err) { > > + kobject_put(mempolicy_kobj); > > 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 increments the kobject's refcount. Therefore, even if kobject_init_and_add() fails, kobject_put() must be called to ensure proper memory cleanup. int kobject_init_and_add(struct kobject *kobj, const struct kobj_type *ktype, struct kobject *parent, const char *fmt, ...) { ... kobject_init(kobj, ktype); retval = kobject_add_varg(kobj, parent, fmt, args); ... return retval; } 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); } Once mempolicy_kobj is passed to kobject_init_and_add(), the memory for ngrp->attrs and ngrp should be released via mempolicy_kobj_release(). The allocation order was changed to ensure that kobject_put() properly invokes mempolicy_kobj_release() when required. > > > + goto err_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; > > + goto err_out; > > } > > > > - return err; > > + return 0; > > + > > Please keep functional changes and refactors separate. There's more > churn in this patch than is needed to accomplish what the changelog > states is the intent. As mentioned earlier, I believe this patch does not need to be split. However, if you have further concerns or suggestions, I would appreciate your input. > > > node_out: > > kfree(node_attrs); > > -mempol_out: > > - kfree(mempolicy_kobj); > > err_out: > > - pr_err("failed to add mempolicy kobject to the system\n"); > > + pr_err("mempolicy sysfs structure failed to initialize\n"); > > return err; > > + > > } > > > > late_initcall(mempolicy_sysfs_init); > > > > base-commit: 80e54e84911a923c40d7bee33a34c1b4be148d7a > > -- > > 2.34.1 > >