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 3201FC282DE for ; Thu, 13 Mar 2025 15:52:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8E637280003; Thu, 13 Mar 2025 11:52:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 86FA0280002; Thu, 13 Mar 2025 11:52:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7100A280003; Thu, 13 Mar 2025 11:52:22 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 50BAE280002 for ; Thu, 13 Mar 2025 11:52:22 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id DE5131401CC for ; Thu, 13 Mar 2025 15:52:23 +0000 (UTC) X-FDA: 83216969766.07.D362FFC Received: from mail-qt1-f177.google.com (mail-qt1-f177.google.com [209.85.160.177]) by imf03.hostedemail.com (Postfix) with ESMTP id E307F20013 for ; Thu, 13 Mar 2025 15:52:21 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=gourry.net header.s=google header.b=Jo712o8F; spf=pass (imf03.hostedemail.com: domain of gourry@gourry.net designates 209.85.160.177 as permitted sender) smtp.mailfrom=gourry@gourry.net; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741881142; 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=4w37FL3B+Ve9xBUgAXtVYhnw9ChVr8Pqzg/e4uFmP8Q=; b=b1ZreQC19HRosTFgFy5U0jAWPoxtaQwayh+yOOvffxKyDVRll9GDeCuLvaQSorPnTymGKl TWECu4hvB5Lf1O8Sg1JM4QA/1FxL5JTNzAjXQHzdI+7QdQcLTpO4kNJpAwb6kGebEcEG4W xuefmzfXNnbTdXWiTicyTiCmDiB58+I= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741881142; a=rsa-sha256; cv=none; b=ev6mc7HRIjfIIyK2eHmibkmis6Ah9SBjI+1m1cne268lZb+dk/RkzQvzXj5HKVfrbhZuw3 Lbv3y0v9rub+JBQpRsgMfNUJViGouudFXbQZrB3ck3DrfR+ly95mR6vAtuw/ED0ukmgTmq xBPMmS0Brr8zWCg8itEcOpLYRBqTH80= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=gourry.net header.s=google header.b=Jo712o8F; spf=pass (imf03.hostedemail.com: domain of gourry@gourry.net designates 209.85.160.177 as permitted sender) smtp.mailfrom=gourry@gourry.net; dmarc=none Received: by mail-qt1-f177.google.com with SMTP id d75a77b69052e-47690a4ec97so10636821cf.2 for ; Thu, 13 Mar 2025 08:52:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gourry.net; s=google; t=1741881141; x=1742485941; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=4w37FL3B+Ve9xBUgAXtVYhnw9ChVr8Pqzg/e4uFmP8Q=; b=Jo712o8Fpvj69ffIBPf2vUSXdDlByoMbH8dZN3dXm0tCTJ6In0L+IWxY/O3yvhfm5a 8LxcvKnaB6V1/RytzeU2VDEm/nTx0w4WAMqgCVeX3jF/a6OZnIco+khvqUMOhN3I6Bp6 9uCPe6c3n/nTTTD2D1GVFC4scrNP5szsXhQlytMAFFulMvrTyNuXJ+AjcEzvtt3w9EnQ frW4X0HB+r+YQBP+TQ7Q6+zajN+/ZvQSW0mE6dRGNOHgGFYRUyuLDLj8cNdnx77bwMbl sWLGxSv5Ap9qm8DWF1L5nXvBV/uboRUpdGjG+CPy68wc1XWcLMcexNnYQ6bY7Dx+WPnr 9ZEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741881141; x=1742485941; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=4w37FL3B+Ve9xBUgAXtVYhnw9ChVr8Pqzg/e4uFmP8Q=; b=H3kM7nU5Xn2+1idFnaiQ3O/BQxnWo9Mehy4qc0YpO/FERmDjF+1UybUc1qYjDm/lQ+ a0aZz3m13dAUwkiBuYw0AEx6r9wW9cGNSGGeKV+H2dOEb6q20CAObHEgo3CWxHf91v5C 7D8N1eHBybRyaofSZ8G9xY6caT0+TzLA89tmajxBicXaO82qzPr16xpVEPTaHnTqMVLI ZMLxJx7EVOkU8IxtrQHM5IZINVk0cqRUi7bBpYQ5jtgu6+wTGwrHMK+bLFhVH7dthlyY ha0T2FOtgLHhCyBGnyju7v0d6dFABaP5Gc4VD2DoQMjd8eWJNIOO4ZYm70oZ8GzCsd9C w8Rw== X-Forwarded-Encrypted: i=1; AJvYcCVZYxaV2cR1y1Yok7G8W0uTNMceXJLNcJ6XuvyDbyq47e6cYxafxy/vfizzY0FOwYae33idCJO+bw==@kvack.org X-Gm-Message-State: AOJu0Yx0j9A/vpAvGtq4mUT13U288ZBIyzsSBAmJ5c85/WpAIGlTLlaK T8544kgXk9BqICdUtDxBcdAr86idWpz5EYZVHycvF23IvBAInic94ngtuve5Uao= X-Gm-Gg: ASbGnct5QR0Eu16xerUxc/Thd7UOZkOnUs93aT5c/998uZ5FlzbxAy2boxcXOC3dX4a ilA+TvmffJC1xtiP6pezwlS8fTJOZcS1bJyq/gDFy04s0KdDfCFHXLNQBnpyEtE5NV/Av7ek5tL L2bUwMSnr03yZM4eB6ERq4q5oP+kEaz3ScMGexvScjQw0HINaJbB9mE+ZdEhQBFGVSy4kpB4xc2 T3yyp7fdvkuw7RyeSW6tXYnJRo0B0IyHM3Qjh7ZHvgsQTVjetwAFEX7AqnVZJIYQEhCch1q46+a Lr3RfDnO7747hkFaLSO9SkCIWyGJ/lpOMBoeNxh13bNjmqXR0LIpBn17sosgHH7nusDzBQspH1k ygHfdNp1PEUkWiiUqSfOh6EDR6O8= X-Google-Smtp-Source: AGHT+IEsoyWweiNgBmaWSKSfpN7pyNhVfLT8lOAAvmtKf+Ddah15RIIeIcih94D/NjT9dh4sWhGcyw== X-Received: by 2002:a05:620a:43aa:b0:7c5:5909:18f3 with SMTP id af79cd13be357-7c55e90fe47mr2004594185a.37.1741881140875; Thu, 13 Mar 2025 08:52:20 -0700 (PDT) Received: from gourry-fedora-PF4VCD3F (pool-173-79-56-208.washdc.fios.verizon.net. [173.79.56.208]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7c573d89815sm112187185a.91.2025.03.13.08.52.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Mar 2025 08:52:20 -0700 (PDT) Date: Thu, 13 Mar 2025 11:52:18 -0400 From: Gregory Price To: Rakie Kim 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 Subject: Re: [PATCH v2 1/4] mm/mempolicy: Fix memory leaks in mempolicy_sysfs_init() Message-ID: References: <20250313063247.681-1-rakie.kim@sk.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250313063247.681-1-rakie.kim@sk.com> X-Rspam-User: X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: E307F20013 X-Stat-Signature: sik4yn4pw8j6citi1s1ra95hshj8fcd4 X-HE-Tag: 1741881141-974223 X-HE-Meta: U2FsdGVkX1+S9+FHTJRqEuNabeHgfxxtaetogyfWxe8UD3CtRuhc9Gm22mRtFiuT69b9Wh8xSzYloRyytYum1TyFuqjHNq2TDAHXpukIS44I87Cqs6iwgTEdVQQx5etS7UcWG1azO4J15obRI5uqfCCITDQVYP1noltI6JwxOKtA+hNsSosaG5rLC84o8Vwpp/kKGa/SZGJWgwPS7E85wqurDDE9aYDXLNKtPJPxnoHOSVXJHLj+MXO4EBdo+hkiiOiXZgCkUTH6dRw3Y44K4EgVFHEIKD6qbEO96NJnn8iuc8OfC+O/KPelMjwBjMFym8P3j55JlQ4sx3Y+zeuAOULu8F39HyGsKYCX0+POiR1LEc0JoWUHN8imftAmmonK8wacIX89hIi8A6FujZcPrdCVOiHbs5F4+LzB2dMHYADCtjIazD+KbViis1LyOVb5Y8IzU9m5zzZ4H8j1thB+yeuEpAslBLnzNbOh7OYmV6wb1fCFJMJ6atyzN6Po3h0WNu56BIzJZlzIL5UTTk/KSFJwbNu5/B0bKpHVlVtYcmkCYDmfwyS6ZkdUIjRl4BNNpFai3HT9m35RSu0lYA20+aip/to8TQWhxgttz1CowroLtEqaRGKw/tIqB3wjNa5AomXHlBfhDrvF9T5bxFGCUQ0vN7OX7ZTDtXgPFHI4QKrjV3+RUGuKUAblag2uX8w+thOQZz88GIuBJt60UKdmBwq03T72M7A89eRT+HZRIY6wwz/MUzvOAMVNzKILbA/OLoPmv5B3LsgUqXkFvRWhfwnofWDq7bMkfql5GWxis9NYEWvedSsf//iIpZ6jq7q3Mi28xUubgCzTrt6FTMMQXh/s0qBdyAafWbJlZZLCYZFeFF5ffJzk/QOR4uWDFGZ7rFJZIjitKxREaBVzLxO1lwEmas+7z1lfXsiJ9weNbbGU/FsKG+S5up0Wl5nN7mieJbqIJ85WJBC5pbN0DD+ oh4yNPMG BPv/HZzmj/RsQ9i5WgPd/IjENVKpWyhmGSf3ijtsB/40cLiB1QpLWvBKYt8bC2T2KkpjfCwX/HBogqmgfgROD+l5a5+eDuf60iK5RJ/upgpBT6JgLCYB5UaYQo8kJG8YA0R2XhPF/grlyhvvNzTuZZYFLsnBqkWA/s6Cz473qULuPUkCiHGSi+i5mXwy9DjKrmBsq1yrAMYa8QcyU8cZNzsXPLOavl/eu5CgnBB/hFa/2UMG0mNVfHHsuu8J4mBYPX1iSYlo2S4Hmggxlu3ZLIv50/VavYtfwinCX64/KrcRmCoyIEMU6/PeEQEe15Wfl39Lh7i+DeufE3FAl4VvkeRun63T2RpCALn7E/hYt7n/CXfZ9avcuJXvurvIB616P1dsyP30hZKBD7G0EuccT9lDFxEsWY2Wc0H1fPa2DxsVxoBwsHkPygcnYW9JhM65OEq3y 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 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 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; }