Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1268875

[PATCH 3.16.y-ckt 18/94] sched/core: Fix TASK_DEAD race in finish_task_switch()

From Luis Henriques <luis.henriques@canonical.com>
Newsgroups linux.kernel
Subject [PATCH 3.16.y-ckt 18/94] sched/core: Fix TASK_DEAD race in finish_task_switch()
Date 2015-11-13 12:00 +0100
Message-ID <qukfi-5kp-59@gated-at.bofh.it> (permalink)
References <qujCx-56Q-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


3.16.7-ckt20 -stable review patch.  If anyone has any objections, please let me know.

------------------

From: Peter Zijlstra <peterz@infradead.org>

commit 95913d97914f44db2b81271c2e2ebd4d2ac2df83 upstream.

So the problem this patch is trying to address is as follows:

        CPU0                            CPU1

        context_switch(A, B)
                                        ttwu(A)
                                          LOCK A->pi_lock
                                          A->on_cpu == 0
        finish_task_switch(A)
          prev_state = A->state  <-.
          WMB                      |
          A->on_cpu = 0;           |
          UNLOCK rq0->lock         |
                                   |    context_switch(C, A)
                                   `--  A->state = TASK_DEAD
          prev_state == TASK_DEAD
            put_task_struct(A)
                                        context_switch(A, C)
                                        finish_task_switch(A)
                                          A->state == TASK_DEAD
                                            put_task_struct(A)

The argument being that the WMB will allow the load of A->state on CPU0
to cross over and observe CPU1's store of A->state, which will then
result in a double-drop and use-after-free.

Now the comment states (and this was true once upon a long time ago)
that we need to observe A->state while holding rq->lock because that
will order us against the wakeup; however the wakeup will not in fact
acquire (that) rq->lock; it takes A->pi_lock these days.

We can obviously fix this by upgrading the WMB to an MB, but that is
expensive, so we'd rather avoid that.

The alternative this patch takes is: smp_store_release(&A->on_cpu, 0),
which avoids the MB on some archs, but not important ones like ARM.

Reported-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Cc: manfred@colorfullife.com
Cc: will.deacon@arm.com
Fixes: e4a52bcb9a18 ("sched: Remove rq->lock from the first half of ttwu()")
Link: http://lkml.kernel.org/r/20150929124509.GG3816@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
 kernel/sched/core.c  | 10 +++++-----
 kernel/sched/sched.h |  5 +++--
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index f06dcf7dcd00..c1d7818dade9 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2225,11 +2225,11 @@ static void finish_task_switch(struct rq *rq, struct task_struct *prev)
 	 * If a task dies, then it sets TASK_DEAD in tsk->state and calls
 	 * schedule one last time. The schedule call will never return, and
 	 * the scheduled task must drop that reference.
-	 * The test for TASK_DEAD must occur while the runqueue locks are
-	 * still held, otherwise prev could be scheduled on another cpu, die
-	 * there before we look at prev->state, and then the reference would
-	 * be dropped twice.
-	 *		Manfred Spraul <manfred@colorfullife.com>
+	 *
+	 * We must observe prev->state before clearing prev->on_cpu (in
+	 * finish_lock_switch), otherwise a concurrent wakeup can get prev
+	 * running on another CPU and we could rave with its RUNNING -> DEAD
+	 * transition, resulting in a double drop.
 	 */
 	prev_state = prev->state;
 	vtime_task_switch(prev);
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 31cc02ebc54e..d1595c7c282a 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -980,9 +980,10 @@ static inline void finish_lock_switch(struct rq *rq, struct task_struct *prev)
 	 * After ->on_cpu is cleared, the task can be moved to a different CPU.
 	 * We must ensure this doesn't happen until the switch is completely
 	 * finished.
+	 *
+	 * Pairs with the control dependency and rmb in try_to_wake_up().
 	 */
-	smp_wmb();
-	prev->on_cpu = 0;
+	smp_store_release(&prev->on_cpu, 0);
 #endif
 #ifdef CONFIG_DEBUG_SPINLOCK
 	/* this is a valid case when another task releases the spinlock */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[3.16.y-ckt stable] Linux 3.16.7-ckt20 stable review Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 05/94] svcrdma: handle rdma read with a non-zero initial page offset Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 92/94] sfc: Fix memcpy() with const destination compiler warning. Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 89/94] ppp, slip: Validate VJ compression slot parameters completely Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 88/94] isdn_ppp: Add checks for allocation failure in isdn_ppp_open() Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 52/94] ASoC: Add info callback for SX_TLV controls Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 46/94] ARM: orion: Fix DSA platform device after mvmdio conversion Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 43/94] iwlwifi: mvm: fix D3 firmware PN programming Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 35/94] i2c: designware: Do not use parameters from ACPI on Dell Inspiron 7348 Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 14/94] usb: Add device quirk for Logitech PTZ cameras Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 22/94] drm/radeon: add pm sysfs files late Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 39/94] rbd: fix double free on rbd_dev->header_name Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 71/94] i2c: mv64xxx: really allow I2C offloading Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 91/94] power: bq24190_charger: suppress build warning Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 90/94] staging/dgnc: fix info leak in ioctl Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 33/94] i2c: s3c2410: enable RuntimePM before registering to the core Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 86/94] serial: 8250_pci: Add support for 12 port Exar boards Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 94/94] sched/preempt: Fix cond_resched_lock() and cond_resched_softirq() Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 93/94] sched/preempt: Rename PREEMPT_CHECK_OFFSET to PREEMPT_DISABLE_OFFSET Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:20 +0100
  [PATCH 3.16.y-ckt 65/94] iwlwifi: mvm: init card correctly on ctkill exit check Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:30 +0100
  [PATCH 3.16.y-ckt 70/94] iommu/amd: Don't clear DTE flags when modifying it Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:30 +0100
  [PATCH 3.16.y-ckt 77/94] drm/radeon: fix dpms when driver backlight control is disabled Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:30 +0100
  [PATCH 3.16.y-ckt 69/94] md/raid10: submit_bio_wait() returns 0 on success Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:30 +0100
  [PATCH 3.16.y-ckt 67/94] crypto: api - Only abort operations on fatal signal Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:30 +0100
  [PATCH 3.16.y-ckt 66/94] module: Fix locking in symbol_put_addr() Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:30 +0100
  [PATCH 3.16.y-ckt 74/94] Revert "ARM64: unwind: Fix PC calculation" Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:30 +0100
  [PATCH 3.16.y-ckt 82/94] ppp: fix pppoe_dev deletion condition in pppoe_release() Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:30 +0100
  [PATCH 3.16.y-ckt 72/94] drm/radeon: don't try to recreate sysfs entries on resume Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:30 +0100
  [PATCH 3.16.y-ckt 63/94] dm btree: fix leak of bufio-backed block in btree_split_beneath error path Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:30 +0100
  [PATCH 3.16.y-ckt 55/94] xhci: Add spurious wakeup quirk for LynxPoint-LP controllers Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:30 +0100
  [PATCH 3.16.y-ckt 80/94] Revert "md: allow a partially recovered device to be hot-added to an array." Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:30 +0100
  [PATCH 3.16.y-ckt 76/94] drm/radeon: move bl encoder assignment into bl init Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:30 +0100
  [PATCH 3.16.y-ckt 83/94] ipv6: gre: support SIT encapsulation Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:30 +0100
  [PATCH 3.16.y-ckt 50/94] drm/i915: Deny wrapping an userptr into a framebuffer Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 58/94] powerpc/rtas: Validate rtas.entry before calling enter_rtas() Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 53/94] xhci: don't finish a TD if we get a short transfer event mid TD Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 48/94] iio: mxs-lradc: Fix temperature offset Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 62/94] dm btree remove: fix a bug when rebalancing nodes after removal Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 57/94] drm/nouveau/gem: return only valid domain when there's only one Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 59/94] mm: make sendfile(2) killable Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 56/94] ASoC: wm8904: Correct number of EQ registers Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 44/94] iwlwifi: fix firmware filename for 3160 Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 45/94] iwlwifi: pci: add a few more PCI subvendor IDs for the 7265 series Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 54/94] xhci: handle no ping response error properly Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 47/94] xen-blkfront: check for null drvdata in blkback_changed (XenbusStateClosing) Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 51/94] x86/efi: Fix multiple GOP device support Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 49/94] ALSA: hda - Fix inverted internal mic on Lenovo G50-80 Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 61/94] rbd: prevent kernel stack blow up on rbd map Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:40 +0100
  [PATCH 3.16.y-ckt 23/94] drm/radeon: add quirk for MSI R7 370 Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:50 +0100
  [PATCH 3.16.y-ckt 34/94] i2c: designware-platdrv: enable RuntimePM before registering to the core Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:50 +0100
  [PATCH 3.16.y-ckt 36/94] btrfs: fix possible leak in btrfs_ioctl_balance() Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:50 +0100
  [PATCH 3.16.y-ckt 27/94] arm64: errata: use KBUILD_CFLAGS_MODULE for erratum #843419 Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:50 +0100
  [PATCH 3.16.y-ckt 41/94] iio: accel: sca3000: memory corruption in sca3000_read_first_n_hw_rb() Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:50 +0100
  [PATCH 3.16.y-ckt 31/94] btrfs: fix use after free iterating extrefs Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:50 +0100
  [PATCH 3.16.y-ckt 29/94] dm thin: fix missing pool reference count decrement in pool_ctr error path Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:50 +0100
  [PATCH 3.16.y-ckt 38/94] usb: xhci: Add support for URB_ZERO_PACKET to bulk/sg transfers Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:50 +0100
  [PATCH 3.16.y-ckt 30/94] btrfs: check unsupported filters in balance arguments Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:50 +0100
  [PATCH 3.16.y-ckt 42/94] iwlwifi: dvm: fix D3 firmware PN programming Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:50 +0100
  [PATCH 3.16.y-ckt 28/94] crypto: ahash - ensure statesize is non-zero Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:50 +0100
  [PATCH 3.16.y-ckt 40/94] ath9k: declare required extra tx headroom Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 11:50 +0100
  [PATCH 3.16.y-ckt 25/94] crypto: sparc - initialize blkcipher.ivsize Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 21/94] workqueue: make sure delayed work run in local cpu Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 04/94] s390/boot/decompression: disable floating point in decompressor Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 10/94] dm raid: fix round up of default region size Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 11/94] ALSA: hda: Add dock support for ThinkPad T550 Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 16/94] ALSA: synth: Fix conflicting OSS device registration on AWE32 Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 13/94] USB: Add reset-resume quirk for two Plantronics usb headphones. Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 03/94] x86/xen: Support kexec/kdump in HVM guests by doing a soft reset Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 02/94] regmap: debugfs: Don't bother actually printing when calculating max length Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 09/94] ASoC: dwc: correct irq clear method Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 12/94] ALSA: hda - Apply SPDIF pin ctl to MacBookPro 12,1 Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 06/94] dm: fix AB-BA deadlock in __dm_destroy() Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 26/94] drm/nouveau/fbcon: take runpm reference when userspace has an open fd Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 15/94] staging: speakup: fix speakup-r regression Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 20/94] 3w-9xxx: don't unmap bounce buffered commands Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 17/94] arm64: readahead: fault retry breaks mmap file read random detection Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 01/94] regmap: debugfs: Ensure we don't underflow when printing access masks Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 24/94] drm/radeon: add quirk for ASUS R7 370 Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 07/94] [SMB3] Do not fall back to SMBWriteX in set_file_size error cases Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100
  [PATCH 3.16.y-ckt 18/94] sched/core: Fix TASK_DEAD race in finish_task_switch() Luis Henriques <luis.henriques@canonical.com> - 2015-11-13 12:00 +0100

csiph-web