Skip to content

Commit cc54f8b

Browse files
xiaochenshengregkh
authored andcommitted
x86/resctrl: Add necessary kernfs_put() calls to prevent refcount leak
commit 7589992 upstream. On resource group creation via a mkdir an extra kernfs_node reference is obtained by kernfs_get() to ensure that the rdtgroup structure remains accessible for the rdtgroup_kn_unlock() calls where it is removed on deletion. Currently the extra kernfs_node reference count is only dropped by kernfs_put() in rdtgroup_kn_unlock() while the rdtgroup structure is removed in a few other locations that lack the matching reference drop. In call paths of rmdir and umount, when a control group is removed, kernfs_remove() is called to remove the whole kernfs nodes tree of the control group (including the kernfs nodes trees of all child monitoring groups), and then rdtgroup structure is freed by kfree(). The rdtgroup structures of all child monitoring groups under the control group are freed by kfree() in free_all_child_rdtgrp(). Before calling kfree() to free the rdtgroup structures, the kernfs node of the control group itself as well as the kernfs nodes of all child monitoring groups still take the extra references which will never be dropped to 0 and the kernfs nodes will never be freed. It leads to reference count leak and kernfs_node_cache memory leak. For example, reference count leak is observed in these two cases: (1) mount -t resctrl resctrl /sys/fs/resctrl mkdir /sys/fs/resctrl/c1 mkdir /sys/fs/resctrl/c1/mon_groups/m1 umount /sys/fs/resctrl (2) mkdir /sys/fs/resctrl/c1 mkdir /sys/fs/resctrl/c1/mon_groups/m1 rmdir /sys/fs/resctrl/c1 The same reference count leak issue also exists in the error exit paths of mkdir in mkdir_rdt_prepare() and rdtgroup_mkdir_ctrl_mon(). Fix this issue by following changes to make sure the extra kernfs_node reference on rdtgroup is dropped before freeing the rdtgroup structure. (1) Introduce rdtgroup removal helper rdtgroup_remove() to wrap up kernfs_put() and kfree(). (2) Call rdtgroup_remove() in rdtgroup removal path where the rdtgroup structure is about to be freed by kfree(). (3) Call rdtgroup_remove() or kernfs_put() as appropriate in the error exit paths of mkdir where an extra reference is taken by kernfs_get(). Fixes: f3cbeac ("x86/intel_rdt/cqm: Add rmdir support") Fixes: e02737d ("x86/intel_rdt: Add tasks files") Fixes: 60cf5e1 ("x86/intel_rdt: Add mkdir to resctrl file system") Reported-by: Willem de Bruijn <willemb@google.com> Signed-off-by: Xiaochen Shen <xiaochen.shen@intel.com> Signed-off-by: Borislav Petkov <bp@suse.de> Reviewed-by: Reinette Chatre <reinette.chatre@intel.com> Cc: stable@vger.kernel.org Link: https://lkml.kernel.org/r/1604085088-31707-1-git-send-email-xiaochen.shen@intel.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent d0c4c5a commit cc54f8b

1 file changed

Lines changed: 25 additions & 7 deletions

File tree

arch/x86/kernel/cpu/resctrl/rdtgroup.c

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,24 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
507507
return ret ?: nbytes;
508508
}
509509

510+
/**
511+
* rdtgroup_remove - the helper to remove resource group safely
512+
* @rdtgrp: resource group to remove
513+
*
514+
* On resource group creation via a mkdir, an extra kernfs_node reference is
515+
* taken to ensure that the rdtgroup structure remains accessible for the
516+
* rdtgroup_kn_unlock() calls where it is removed.
517+
*
518+
* Drop the extra reference here, then free the rdtgroup structure.
519+
*
520+
* Return: void
521+
*/
522+
static void rdtgroup_remove(struct rdtgroup *rdtgrp)
523+
{
524+
kernfs_put(rdtgrp->kn);
525+
kfree(rdtgrp);
526+
}
527+
510528
struct task_move_callback {
511529
struct callback_head work;
512530
struct rdtgroup *rdtgrp;
@@ -529,7 +547,7 @@ static void move_myself(struct callback_head *head)
529547
(rdtgrp->flags & RDT_DELETED)) {
530548
current->closid = 0;
531549
current->rmid = 0;
532-
kfree(rdtgrp);
550+
rdtgroup_remove(rdtgrp);
533551
}
534552

535553
preempt_disable();
@@ -1914,8 +1932,7 @@ void rdtgroup_kn_unlock(struct kernfs_node *kn)
19141932
rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED)
19151933
rdtgroup_pseudo_lock_remove(rdtgrp);
19161934
kernfs_unbreak_active_protection(kn);
1917-
kernfs_put(rdtgrp->kn);
1918-
kfree(rdtgrp);
1935+
rdtgroup_remove(rdtgrp);
19191936
} else {
19201937
kernfs_unbreak_active_protection(kn);
19211938
}
@@ -2207,7 +2224,7 @@ static void free_all_child_rdtgrp(struct rdtgroup *rdtgrp)
22072224
if (atomic_read(&sentry->waitcount) != 0)
22082225
sentry->flags = RDT_DELETED;
22092226
else
2210-
kfree(sentry);
2227+
rdtgroup_remove(sentry);
22112228
}
22122229
}
22132230

@@ -2249,7 +2266,7 @@ static void rmdir_all_sub(void)
22492266
if (atomic_read(&rdtgrp->waitcount) != 0)
22502267
rdtgrp->flags = RDT_DELETED;
22512268
else
2252-
kfree(rdtgrp);
2269+
rdtgroup_remove(rdtgrp);
22532270
}
22542271
/* Notify online CPUs to update per cpu storage and PQR_ASSOC MSR */
22552272
update_closid_rmid(cpu_online_mask, &rdtgroup_default);
@@ -2685,7 +2702,7 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,
26852702
* kernfs_remove() will drop the reference count on "kn" which
26862703
* will free it. But we still need it to stick around for the
26872704
* rdtgroup_kn_unlock(kn) call. Take one extra reference here,
2688-
* which will be dropped inside rdtgroup_kn_unlock().
2705+
* which will be dropped by kernfs_put() in rdtgroup_remove().
26892706
*/
26902707
kernfs_get(kn);
26912708

@@ -2726,6 +2743,7 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,
27262743
out_idfree:
27272744
free_rmid(rdtgrp->mon.rmid);
27282745
out_destroy:
2746+
kernfs_put(rdtgrp->kn);
27292747
kernfs_remove(rdtgrp->kn);
27302748
out_free_rgrp:
27312749
kfree(rdtgrp);
@@ -2738,7 +2756,7 @@ static void mkdir_rdt_prepare_clean(struct rdtgroup *rgrp)
27382756
{
27392757
kernfs_remove(rgrp->kn);
27402758
free_rmid(rgrp->mon.rmid);
2741-
kfree(rgrp);
2759+
rdtgroup_remove(rgrp);
27422760
}
27432761

27442762
/*

0 commit comments

Comments
 (0)