| From 758999246965eeb8b253d47e72f7bfe508804b16 Mon Sep 17 00:00:00 2001 |
| From: Xiaochen Shen <xiaochen.shen@intel.com> |
| Date: Sat, 31 Oct 2020 03:11:28 +0800 |
| Subject: x86/resctrl: Add necessary kernfs_put() calls to prevent refcount leak |
| |
| From: Xiaochen Shen <xiaochen.shen@intel.com> |
| |
| commit 758999246965eeb8b253d47e72f7bfe508804b16 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: f3cbeacaa06e ("x86/intel_rdt/cqm: Add rmdir support") |
| Fixes: e02737d5b826 ("x86/intel_rdt: Add tasks files") |
| Fixes: 60cf5e101fd4 ("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> |
| |
| --- |
| arch/x86/kernel/cpu/resctrl/rdtgroup.c | 32 +++++++++++++++++++++++++------- |
| 1 file changed, 25 insertions(+), 7 deletions(-) |
| |
| --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c |
| +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c |
| @@ -507,6 +507,24 @@ unlock: |
| return ret ?: nbytes; |
| } |
| |
| +/** |
| + * rdtgroup_remove - the helper to remove resource group safely |
| + * @rdtgrp: resource group to remove |
| + * |
| + * On resource group creation via a mkdir, an extra kernfs_node reference is |
| + * taken to ensure that the rdtgroup structure remains accessible for the |
| + * rdtgroup_kn_unlock() calls where it is removed. |
| + * |
| + * Drop the extra reference here, then free the rdtgroup structure. |
| + * |
| + * Return: void |
| + */ |
| +static void rdtgroup_remove(struct rdtgroup *rdtgrp) |
| +{ |
| + kernfs_put(rdtgrp->kn); |
| + kfree(rdtgrp); |
| +} |
| + |
| struct task_move_callback { |
| struct callback_head work; |
| struct rdtgroup *rdtgrp; |
| @@ -529,7 +547,7 @@ static void move_myself(struct callback_ |
| (rdtgrp->flags & RDT_DELETED)) { |
| current->closid = 0; |
| current->rmid = 0; |
| - kfree(rdtgrp); |
| + rdtgroup_remove(rdtgrp); |
| } |
| |
| preempt_disable(); |
| @@ -1914,8 +1932,7 @@ void rdtgroup_kn_unlock(struct kernfs_no |
| rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED) |
| rdtgroup_pseudo_lock_remove(rdtgrp); |
| kernfs_unbreak_active_protection(kn); |
| - kernfs_put(rdtgrp->kn); |
| - kfree(rdtgrp); |
| + rdtgroup_remove(rdtgrp); |
| } else { |
| kernfs_unbreak_active_protection(kn); |
| } |
| @@ -2207,7 +2224,7 @@ static void free_all_child_rdtgrp(struct |
| if (atomic_read(&sentry->waitcount) != 0) |
| sentry->flags = RDT_DELETED; |
| else |
| - kfree(sentry); |
| + rdtgroup_remove(sentry); |
| } |
| } |
| |
| @@ -2249,7 +2266,7 @@ static void rmdir_all_sub(void) |
| if (atomic_read(&rdtgrp->waitcount) != 0) |
| rdtgrp->flags = RDT_DELETED; |
| else |
| - kfree(rdtgrp); |
| + rdtgroup_remove(rdtgrp); |
| } |
| /* Notify online CPUs to update per cpu storage and PQR_ASSOC MSR */ |
| update_closid_rmid(cpu_online_mask, &rdtgroup_default); |
| @@ -2685,7 +2702,7 @@ static int mkdir_rdt_prepare(struct kern |
| * kernfs_remove() will drop the reference count on "kn" which |
| * will free it. But we still need it to stick around for the |
| * rdtgroup_kn_unlock(kn) call. Take one extra reference here, |
| - * which will be dropped inside rdtgroup_kn_unlock(). |
| + * which will be dropped by kernfs_put() in rdtgroup_remove(). |
| */ |
| kernfs_get(kn); |
| |
| @@ -2726,6 +2743,7 @@ static int mkdir_rdt_prepare(struct kern |
| out_idfree: |
| free_rmid(rdtgrp->mon.rmid); |
| out_destroy: |
| + kernfs_put(rdtgrp->kn); |
| kernfs_remove(rdtgrp->kn); |
| out_free_rgrp: |
| kfree(rdtgrp); |
| @@ -2738,7 +2756,7 @@ static void mkdir_rdt_prepare_clean(stru |
| { |
| kernfs_remove(rgrp->kn); |
| free_rmid(rgrp->mon.rmid); |
| - kfree(rgrp); |
| + rdtgroup_remove(rgrp); |
| } |
| |
| /* |