| Message ID | 20250312075628.648-1-rakie.kim@sk.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/4] mm/mempolicy: Fix memory leaks in mempolicy_sysfs_init() | expand |
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. > Fixes: dce41f5ae253 ("mm/mempolicy: implement the sysfs-based weighted_interleave interface") > Signed-off-by: Rakie Kim <rakie.kim@sk.com> > --- > 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. > + 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. > 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 >
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; } err = kobject_init_and_add(mempolicy_kobj, &mempolicy_ktype, mm_kobj, "mempolicy"); - if (err) - goto node_out; + if (err) { + kobject_put(mempolicy_kobj); + 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; + 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);
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. Fixes: dce41f5ae253 ("mm/mempolicy: implement the sysfs-based weighted_interleave interface") Signed-off-by: Rakie Kim <rakie.kim@sk.com> --- mm/mempolicy.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) base-commit: 80e54e84911a923c40d7bee33a34c1b4be148d7a