All of lore.kernel.org
 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: 33+ 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
     [not found] ` <20200203161902.288335885-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
2020-02-03 21:40   ` [PATCH 5.5 00/23] 5.5.2-stable review Jon Hunter
2020-02-03 21:40     ` Jon Hunter
     [not found]     ` <10cd3c5f-0a2a-f73c-f071-17d1cc33531b-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2020-02-03 22:51       ` Greg Kroah-Hartman
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.