From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org,
Reinette Chatre <reinette.chatre@intel.com>,
Xiaochen Shen <xiaochen.shen@intel.com>,
Borislav Petkov <bp@suse.de>, Tony Luck <tony.luck@intel.com>,
Thomas Gleixner <tglx@linutronix.de>,
Sasha Levin <sashal@kernel.org>
Subject: [PATCH 5.5 05/23] x86/resctrl: Fix use-after-free due to inaccurate refcount of rdtgroup
Date: Mon, 3 Feb 2020 16:20:25 +0000 [thread overview]
Message-ID: <20200203161903.828292226@linuxfoundation.org> (raw)
In-Reply-To: <20200203161902.288335885@linuxfoundation.org>
From: Xiaochen Shen <xiaochen.shen@intel.com>
[ Upstream commit 074fadee59ee7a9d2b216e9854bd4efb5dad679f ]
There is a race condition in the following scenario which results in an
use-after-free issue when reading a monitoring file and deleting the
parent ctrl_mon group concurrently:
Thread 1 calls atomic_inc() to take refcount of rdtgrp and then calls
kernfs_break_active_protection() to drop the active reference of kernfs
node in rdtgroup_kn_lock_live().
In Thread 2, kernfs_remove() is a blocking routine. It waits on all sub
kernfs nodes to drop the active reference when removing all subtree
kernfs nodes recursively. Thread 2 could block on kernfs_remove() until
Thread 1 calls kernfs_break_active_protection(). Only after
kernfs_remove() completes the refcount of rdtgrp could be trusted.
Before Thread 1 calls atomic_inc() and kernfs_break_active_protection(),
Thread 2 could call kfree() when the refcount of rdtgrp (sentry) is 0
instead of 1 due to the race.
In Thread 1, in rdtgroup_kn_unlock(), referring to earlier rdtgrp memory
(rdtgrp->waitcount) which was already freed in Thread 2 results in
use-after-free issue.
Thread 1 (rdtgroup_mondata_show) Thread 2 (rdtgroup_rmdir)
-------------------------------- -------------------------
rdtgroup_kn_lock_live
/*
* kn active protection until
* kernfs_break_active_protection(kn)
*/
rdtgrp = kernfs_to_rdtgroup(kn)
rdtgroup_kn_lock_live
atomic_inc(&rdtgrp->waitcount)
mutex_lock
rdtgroup_rmdir_ctrl
free_all_child_rdtgrp
/*
* sentry->waitcount should be 1
* but is 0 now due to the race.
*/
kfree(sentry)*[1]
/*
* Only after kernfs_remove()
* completes, the refcount of
* rdtgrp could be trusted.
*/
atomic_inc(&rdtgrp->waitcount)
/* kn->active-- */
kernfs_break_active_protection(kn)
rdtgroup_ctrl_remove
rdtgrp->flags = RDT_DELETED
/*
* Blocking routine, wait for
* all sub kernfs nodes to drop
* active reference in
* kernfs_break_active_protection.
*/
kernfs_remove(rdtgrp->kn)
rdtgroup_kn_unlock
mutex_unlock
atomic_dec_and_test(
&rdtgrp->waitcount)
&& (flags & RDT_DELETED)
kernfs_unbreak_active_protection(kn)
kfree(rdtgrp)
mutex_lock
mon_event_read
rdtgroup_kn_unlock
mutex_unlock
/*
* Use-after-free: refer to earlier rdtgrp
* memory which was freed in [1].
*/
atomic_dec_and_test(&rdtgrp->waitcount)
&& (flags & RDT_DELETED)
/* kn->active++ */
kernfs_unbreak_active_protection(kn)
kfree(rdtgrp)
Fix it by moving free_all_child_rdtgrp() to after kernfs_remove() in
rdtgroup_rmdir_ctrl() to ensure it has the accurate refcount of rdtgrp.
Fixes: f3cbeacaa06e ("x86/intel_rdt/cqm: Add rmdir support")
Suggested-by: Reinette Chatre <reinette.chatre@intel.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>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/1578500886-21771-3-git-send-email-xiaochen.shen@intel.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index c7564294a12a8..954fd048ad9bd 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -2960,13 +2960,13 @@ static int rdtgroup_rmdir_ctrl(struct kernfs_node *kn, struct rdtgroup *rdtgrp,
closid_free(rdtgrp->closid);
free_rmid(rdtgrp->mon.rmid);
+ rdtgroup_ctrl_remove(kn, rdtgrp);
+
/*
* Free all the child monitor group rmids.
*/
free_all_child_rdtgrp(rdtgrp);
- rdtgroup_ctrl_remove(kn, rdtgrp);
-
return 0;
}
--
2.20.1
next prev parent reply other threads:[~2020-02-03 16:38 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-03 16:20 [PATCH 5.5 00/23] 5.5.2-stable review Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 01/23] vfs: fix do_last() regression Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 02/23] cifs: fix soft mounts hanging in the reconnect code Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 03/23] x86/resctrl: Fix a deadlock due to inaccurate reference Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 04/23] x86/resctrl: Fix use-after-free when deleting resource groups Greg Kroah-Hartman
2020-02-03 16:20 ` Greg Kroah-Hartman [this message]
2020-02-03 16:20 ` [PATCH 5.5 06/23] KVM: PPC: Book3S PR: Fix -Werror=return-type build failure Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 07/23] gfs2: Another gfs2_find_jhead fix Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 08/23] lib/test_bitmap: correct test data offsets for 32-bit Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 09/23] perf c2c: Fix return type for histogram sorting comparision functions Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 10/23] PM / devfreq: Add new name attribute for sysfs Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 11/23] tools lib: Fix builds when glibc contains strlcpy() Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 12/23] arm64: kbuild: remove compressed images on make ARCH=arm64 (dist)clean Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 13/23] mm/mempolicy.c: fix out of bounds write in mpol_parse_str() Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 14/23] reiserfs: Fix memory leak of journal device string Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 15/23] media: digitv: dont continue if remote control state cant be read Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 16/23] media: af9005: uninitialized variable printked Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 17/23] media: vp7045: do not read uninitialized values if usb transfer fails Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 18/23] media: gspca: zero usb_buf Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 19/23] media: dvb-usb/dvb-usb-urb.c: initialize actlen to 0 Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 20/23] tomoyo: Use atomic_t for statistics counter Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 21/23] ttyprintk: fix a potential deadlock in interrupt context issue Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 22/23] Bluetooth: Fix race condition in hci_release_sock() Greg Kroah-Hartman
2020-02-03 16:20 ` [PATCH 5.5 23/23] cgroup: Prevent double killing of css when enabling threaded cgroup Greg Kroah-Hartman
2020-02-03 21:40 ` [PATCH 5.5 00/23] 5.5.2-stable review Jon Hunter
2020-02-03 22:51 ` Greg Kroah-Hartman
2020-02-04 15:45 ` Naresh Kamboju
2020-02-05 13:07 ` Greg Kroah-Hartman
2020-02-08 16:13 ` Daniel Díaz
2020-02-04 17:20 ` Guenter Roeck
2020-02-04 22:56 ` Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200203161903.828292226@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=bp@suse.de \
--cc=linux-kernel@vger.kernel.org \
--cc=reinette.chatre@intel.com \
--cc=sashal@kernel.org \
--cc=stable@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=tony.luck@intel.com \
--cc=xiaochen.shen@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).