stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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




  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).