[PATCH openEuler-1.0-LTS 0/2] arm64/mpam: Fix use-after-free when deleting resource groups
Wang ShaoBo (1): arm64/mpam: remove kernfs_get() calls() and add kernfs_put() calls to prevent refcount leak Zeng Heng (1): arm64/mpam: Fix use-after-free when deleting resource groups arch/arm64/include/asm/resctrl.h | 18 +++++++++++++ arch/arm64/kernel/mpam/mpam_ctrlmon.c | 8 ------ arch/arm64/kernel/mpam/mpam_resctrl.c | 2 +- fs/resctrlfs.c | 37 ++++++++++----------------- 4 files changed, 32 insertions(+), 33 deletions(-) -- 2.25.1
From: Wang ShaoBo <bobo.shaobowang@huawei.com> hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I975PZ -------------------------------- Refer to the two commits: commit fd8d9db3559a ("x86/resctrl: Remove superfluous kernfs_get() calls to prevent refcount leak") commit 758999246965 ("x86/resctrl: Add necessary kernfs_put() calls to prevent refcount leak") there are some places where a kernfs_node reference is obtained without a corresponding release. The excessive amount of reference count on kernfs nodes will never be dropped to 0 and the kernfs nodes will never be freed in the call paths of rmdir and umount. 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 none /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 Superfluous kernfs_get() calls are removed from two areas: (1) In call paths of mount and mkdir, when kernfs nodes for "info", "mon_groups" and "mon_data" directories and sub-directories are created, the reference count of newly created kernfs node is set to 1. But after kernfs_create_dir() returns, superfluous kernfs_get() are called to take an additional reference. (2) kernfs_get() calls in rmdir call paths. Necessary kernfs_put() calls are added by following changes: (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(). Signed-off-by: Wang ShaoBo <bobo.shaobowang@huawei.com> Signed-off-by: Jialin Zhang <zhangjialin11@huawei.com> Conflicts: arch/arm64/include/asm/resctrl.h fs/resctrlfs.c Signed-off-by: Zeng Heng <zengheng4@huawei.com> --- arch/arm64/include/asm/resctrl.h | 18 ++++++++++++++++ arch/arm64/kernel/mpam/mpam_ctrlmon.c | 8 ------- arch/arm64/kernel/mpam/mpam_resctrl.c | 2 +- fs/resctrlfs.c | 31 ++++++--------------------- 4 files changed, 26 insertions(+), 33 deletions(-) diff --git a/arch/arm64/include/asm/resctrl.h b/arch/arm64/include/asm/resctrl.h index d46b9ffe6c88..bdf52f104d2e 100644 --- a/arch/arm64/include/asm/resctrl.h +++ b/arch/arm64/include/asm/resctrl.h @@ -505,5 +505,23 @@ static inline u32 resctrl_navie_closid(struct sd_closid closid) return closid.intpartid; } +/** + * 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 inline void rdtgroup_remove(struct rdtgroup *rdtgrp) +{ + kernfs_put(rdtgrp->kn); + kfree(rdtgrp); +} + #endif #endif /* _ASM_ARM64_RESCTRL_H */ diff --git a/arch/arm64/kernel/mpam/mpam_ctrlmon.c b/arch/arm64/kernel/mpam/mpam_ctrlmon.c index e89683a27a98..0aab9f8c9a96 100644 --- a/arch/arm64/kernel/mpam/mpam_ctrlmon.c +++ b/arch/arm64/kernel/mpam/mpam_ctrlmon.c @@ -818,7 +818,6 @@ static int resctrl_group_mkdir_info_resdir(struct resctrl_resource *r, if (IS_ERR(kn_subdir)) return PTR_ERR(kn_subdir); - kernfs_get(kn_subdir); ret = resctrl_group_kn_set_ugid(kn_subdir); if (ret) return ret; @@ -844,7 +843,6 @@ int resctrl_group_create_info_dir(struct kernfs_node *parent_kn, *kn_info = kernfs_create_dir(parent_kn, "info", parent_kn->mode, NULL); if (IS_ERR(*kn_info)) return PTR_ERR(*kn_info); - kernfs_get(*kn_info); ret = resctrl_group_add_files(*kn_info, RF_TOP_INFO); if (ret) @@ -879,12 +877,6 @@ int resctrl_group_create_info_dir(struct kernfs_node *parent_kn, } } - /* - m This extra ref will be put in kernfs_remove() and guarantees - * that @rdtgrp->kn is always accessible. - */ - kernfs_get(*kn_info); - ret = resctrl_group_kn_set_ugid(*kn_info); if (ret) goto out_destroy; diff --git a/arch/arm64/kernel/mpam/mpam_resctrl.c b/arch/arm64/kernel/mpam/mpam_resctrl.c index 7dcb75accf86..5808ece9ed53 100644 --- a/arch/arm64/kernel/mpam/mpam_resctrl.c +++ b/arch/arm64/kernel/mpam/mpam_resctrl.c @@ -1379,7 +1379,7 @@ static void move_myself(struct callback_head *head) (rdtgrp->flags & RDT_DELETED)) { current->closid = 0; current->rmid = 0; - kfree(rdtgrp); + rdtgroup_remove(rdtgrp); } preempt_disable(); diff --git a/fs/resctrlfs.c b/fs/resctrlfs.c index 4e9a1cdb9b13..45259c387347 100644 --- a/fs/resctrlfs.c +++ b/fs/resctrlfs.c @@ -212,8 +212,7 @@ void resctrl_group_kn_unlock(struct kernfs_node *kn) if (atomic_dec_and_test(&rdtgrp->waitcount) && (rdtgrp->flags & RDT_DELETED)) { kernfs_unbreak_active_protection(kn); - kernfs_put(rdtgrp->kn); - kfree(rdtgrp); + rdtgroup_remove(rdtgrp); } else { kernfs_unbreak_active_protection(kn); } @@ -235,12 +234,6 @@ mongroup_create_dir(struct kernfs_node *parent_kn, struct resctrl_group *prgrp, if (dest_kn) *dest_kn = kn; - /* - * This extra ref will be put in kernfs_remove() and guarantees - * that @rdtgrp->kn is always accessible. - */ - kernfs_get(kn); - ret = resctrl_group_kn_set_ugid(kn); if (ret) goto out_destroy; @@ -373,7 +366,6 @@ static struct dentry *resctrl_mount(struct file_system_type *fs_type, dentry = ERR_PTR(ret); goto out_info; } - kernfs_get(kn_mongrp); ret = mkdir_mondata_all_prepare(&resctrl_group_default); if (ret < 0) { @@ -386,7 +378,7 @@ static struct dentry *resctrl_mount(struct file_system_type *fs_type, dentry = ERR_PTR(ret); goto out_mongrp; } - kernfs_get(kn_mondata); + resctrl_group_default.mon.mon_data_kn = kn_mondata; } @@ -474,7 +466,7 @@ static void free_all_child_rdtgrp(struct resctrl_group *rdtgrp) /* rmid may not be used */ rmid_free(sentry->mon.rmid); list_del(&sentry->mon.crdtgrp_list); - kfree(sentry); + rdtgroup_remove(sentry); } } @@ -508,7 +500,7 @@ static void rmdir_all_sub(void) kernfs_remove(rdtgrp->kn); list_del(&rdtgrp->resctrl_group_list); - kfree(rdtgrp); + rdtgroup_remove(rdtgrp); } /* Notify online CPUs to update per cpu storage and PQR_ASSOC MSR */ update_closid_rmid(cpu_online_mask, &resctrl_group_default); @@ -645,7 +637,7 @@ static int mkdir_resctrl_prepare(struct kernfs_node *parent_kn, * kernfs_remove() will drop the reference count on "kn" which * will free it. But we still need it to stick around for the * resctrl_group_kn_unlock(kn} call below. Take one extra reference - * here, which will be dropped inside resctrl_group_kn_unlock(). + * here, which will be dropped inside rdtgroup_remove(). */ kernfs_get(kn); @@ -685,6 +677,7 @@ static int mkdir_resctrl_prepare(struct kernfs_node *parent_kn, out_prepare_clean: mkdir_mondata_all_prepare_clean(rdtgrp); out_destroy: + kernfs_put(rdtgrp->kn); kernfs_remove(rdtgrp->kn); out_free_rmid: rmid_free(rdtgrp->mon.rmid); @@ -701,7 +694,7 @@ static int mkdir_resctrl_prepare(struct kernfs_node *parent_kn, static void mkdir_resctrl_prepare_clean(struct resctrl_group *rgrp) { kernfs_remove(rgrp->kn); - kfree(rgrp); + rdtgroup_remove(rgrp); } /* @@ -866,11 +859,6 @@ static int resctrl_group_rmdir_mon(struct kernfs_node *kn, struct resctrl_group { resctrl_group_rm_mon(rdtgrp, tmpmask); - /* - * one extra hold on this, will drop when we kfree(rdtgrp) - * in resctrl_group_kn_unlock() - */ - kernfs_get(kn); kernfs_remove(rdtgrp->kn); return 0; @@ -919,11 +907,6 @@ static int resctrl_group_rmdir_ctrl(struct kernfs_node *kn, struct resctrl_group { resctrl_group_rm_ctrl(rdtgrp, tmpmask); - /* - * one extra hold on this, will drop when we kfree(rdtgrp) - * in resctrl_group_kn_unlock() - */ - kernfs_get(kn); kernfs_remove(rdtgrp->kn); return 0; -- 2.25.1
hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I975PZ -------------------------------- Refer to the below commit: commit b8511ccc75c0 ("x86/resctrl: Fix use-after-free when deleting resource groups") Before removing rdtgroup, we need to refer to waitcount counter, otherwise when unmounting the resctrl file system or deleting ctrl_mon groups, and there were a waiter on resctrl system, then a use-after-free issue would occurs. Fix that by removing rdtgroup after checking the waitcount. Fixes: 3b856c03b36a ("arm64/mpam: remove kernfs_get() calls() and add kernfs_put() calls to prevent refcount leak") Signed-off-by: Zeng Heng <zengheng4@huawei.com> --- fs/resctrlfs.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/fs/resctrlfs.c b/fs/resctrlfs.c index 45259c387347..bdb095f16f13 100644 --- a/fs/resctrlfs.c +++ b/fs/resctrlfs.c @@ -466,7 +466,10 @@ static void free_all_child_rdtgrp(struct resctrl_group *rdtgrp) /* rmid may not be used */ rmid_free(sentry->mon.rmid); list_del(&sentry->mon.crdtgrp_list); - rdtgroup_remove(sentry); + if (atomic_read(&sentry->waitcount) != 0) + sentry->flags = RDT_DELETED; + else + rdtgroup_remove(sentry); } } @@ -500,7 +503,10 @@ static void rmdir_all_sub(void) kernfs_remove(rdtgrp->kn); list_del(&rdtgrp->resctrl_group_list); - rdtgroup_remove(rdtgrp); + if (atomic_read(&rdtgrp->waitcount) != 0) + rdtgrp->flags = RDT_DELETED; + else + rdtgroup_remove(rdtgrp); } /* Notify online CPUs to update per cpu storage and PQR_ASSOC MSR */ update_closid_rmid(cpu_online_mask, &resctrl_group_default); -- 2.25.1
反馈： 您发送到kernel@openeuler.org的补丁/补丁集，已成功转换为PR！ PR链接地址： https://gitee.com/openeuler/kernel/pulls/5158 邮件列表地址：https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/B... FeedBack: The patch(es) which you have sent to kernel@openeuler.org mailing list has been converted to a pull request successfully! Pull request link: https://gitee.com/openeuler/kernel/pulls/5158 Mailing list address: https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/B...
participants (2)
-
patchwork bot -
Zeng Heng