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
Subject: [PATCH 3.10 04/32] libata: prevent HSM state change race between ISR and PIO
Date: Tue, 27 Jan 2015 17:26:34 -0800	[thread overview]
Message-ID: <20150128012627.754444469@linuxfoundation.org> (raw)
In-Reply-To: <20150128012627.081285723@linuxfoundation.org>

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

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

From: David Jeffery <djeffery@redhat.com>

commit ce7514526742c0898b837d4395f515b79dfb5a12 upstream.

It is possible for ata_sff_flush_pio_task() to set ap->hsm_task_state to
HSM_ST_IDLE in between the time __ata_sff_port_intr() checks for HSM_ST_IDLE
and before it calls ata_sff_hsm_move() causing ata_sff_hsm_move() to BUG().

This problem is hard to reproduce making this patch hard to verify, but this
fix will prevent the race.

I have not been able to reproduce the problem, but here is a crash dump from
a 2.6.32 kernel.

On examining the ata port's state, its hsm_task_state field has a value of HSM_ST_IDLE:

crash> struct ata_port.hsm_task_state ffff881c1121c000
  hsm_task_state = 0

Normally, this should not be possible as ata_sff_hsm_move() was called from ata_sff_host_intr(),
which checks hsm_task_state and won't call ata_sff_hsm_move() if it has a HSM_ST_IDLE value.

PID: 11053  TASK: ffff8816e846cae0  CPU: 0   COMMAND: "sshd"
 #0 [ffff88008ba03960] machine_kexec at ffffffff81038f3b
 #1 [ffff88008ba039c0] crash_kexec at ffffffff810c5d92
 #2 [ffff88008ba03a90] oops_end at ffffffff8152b510
 #3 [ffff88008ba03ac0] die at ffffffff81010e0b
 #4 [ffff88008ba03af0] do_trap at ffffffff8152ad74
 #5 [ffff88008ba03b50] do_invalid_op at ffffffff8100cf95
 #6 [ffff88008ba03bf0] invalid_op at ffffffff8100bf9b
    [exception RIP: ata_sff_hsm_move+317]
    RIP: ffffffff813a77ad  RSP: ffff88008ba03ca0  RFLAGS: 00010097
    RAX: 0000000000000000  RBX: ffff881c1121dc60  RCX: 0000000000000000
    RDX: ffff881c1121dd10  RSI: ffff881c1121dc60  RDI: ffff881c1121c000
    RBP: ffff88008ba03d00   R8: 0000000000000000   R9: 000000000000002e
    R10: 000000000001003f  R11: 000000000000009b  R12: ffff881c1121c000
    R13: 0000000000000000  R14: 0000000000000050  R15: ffff881c1121dd78
    ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0018
 #7 [ffff88008ba03d08] ata_sff_host_intr at ffffffff813a7fbd
 #8 [ffff88008ba03d38] ata_sff_interrupt at ffffffff813a821e
 #9 [ffff88008ba03d78] handle_IRQ_event at ffffffff810e6ec0
---
 drivers/ata/libata-sff.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -1333,7 +1333,19 @@ void ata_sff_flush_pio_task(struct ata_p
 	DPRINTK("ENTER\n");
 
 	cancel_delayed_work_sync(&ap->sff_pio_task);
+
+	/*
+	 * We wanna reset the HSM state to IDLE.  If we do so without
+	 * grabbing the port lock, critical sections protected by it which
+	 * expect the HSM state to stay stable may get surprised.  For
+	 * example, we may set IDLE in between the time
+	 * __ata_sff_port_intr() checks for HSM_ST_IDLE and before it calls
+	 * ata_sff_hsm_move() causing ata_sff_hsm_move() to BUG().
+	 */
+	spin_lock_irq(ap->lock);
 	ap->hsm_task_state = HSM_ST_IDLE;
+	spin_unlock_irq(ap->lock);
+
 	ap->sff_pio_task_link = NULL;
 
 	if (ata_msg_ctl(ap))



  parent reply	other threads:[~2015-01-28  1:26 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-28  1:26 [PATCH 3.10 00/32] 3.10.67-stable review Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 01/32] gpio: sysfs: fix gpio-chip device-attribute leak Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 02/32] gpio: sysfs: fix gpio " Greg Kroah-Hartman
2015-01-28 16:05   ` [PATCH v2] " Johan Hovold
2015-01-28 17:42     ` Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 03/32] pinctrl: Fix two deadlocks Greg Kroah-Hartman
2015-01-28  1:26 ` Greg Kroah-Hartman [this message]
2015-01-28  1:26 ` [PATCH 3.10 05/32] ALSA: usb-audio: Add mic volume fix quirk for Logitech Webcam C210 Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 06/32] scripts/recordmcount.pl: There is no -m32 gcc option on Super-H anymore Greg Kroah-Hartman
2015-01-28  1:32   ` John Paul Adrian Glaubitz
2015-01-28  2:12     ` Steven Rostedt
2015-01-28  1:26 ` [PATCH 3.10 07/32] drm/i915: Fix mutex->owner inspection race under DEBUG_MUTEXES Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 08/32] ipr: wait for aborted command responses Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 09/32] dm cache: share cache-metadata object across inactive and active DM tables Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 10/32] time: settimeofday: Validate the values of tv from user Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 11/32] time: adjtimex: Validate the ADJ_FREQUENCY values Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 12/32] ARM: dts: imx25: Fix PWM "per" clocks Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 13/32] bus: mvebu-mbus: fix support of MBus window 13 Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 14/32] can: dev: fix crtlmode_supported check Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 15/32] clocksource: exynos_mct: Fix bitmask regression for exynos4_mct_write Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 16/32] x86, hyperv: Mark the Hyper-V clocksource as being continuous Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 17/32] x86/tsc: Change Fast TSC calibration failed from error to info Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 18/32] KVM: x86: Fix of previously incomplete fix for CVE-2014-8480 Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 19/32] x86, tls, ldt: Stop checking lm in LDT_empty Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 20/32] x86, tls: Interpret an all-zero struct user_desc as "no segment" Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 21/32] x86/asm/traps: Disable tracing and kprobes in fixup_bad_iret and sync_regs Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 22/32] sata_dwc_460ex: fix resource leak on error path Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 23/32] KEYS: close race between key lookup and freeing Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 24/32] ipvs: uninitialized data with IP_VS_IPV6 Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 25/32] Revert "swiotlb-xen: pass dev_addr to swiotlb_tbl_unmap_single" Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 26/32] drbd: merge_bvec_fn: properly remap bvm->bi_bdev Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 27/32] crypto: prefix module autoloading with "crypto-" Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 28/32] crypto: include crypto- module prefix in template Greg Kroah-Hartman
2015-01-28  1:26 ` [PATCH 3.10 29/32] crypto: add missing crypto module aliases Greg Kroah-Hartman
2015-01-28  1:27 ` [PATCH 3.10 30/32] quota: provide interface for readding allocated space into reserved space Greg Kroah-Hartman
2015-01-28  1:27 ` [PATCH 3.10 31/32] ext4: fix warning in ext4_da_update_reserve_space() Greg Kroah-Hartman
2015-01-28  1:27 ` [PATCH 3.10 32/32] md/raid5: fetch_block must fetch all the blocks handle_stripe_dirtying wants Greg Kroah-Hartman
2015-01-28 14:14 ` [PATCH 3.10 00/32] 3.10.67-stable review Guenter Roeck
2015-01-28 16:51 ` Shuah Khan

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=20150128012627.754444469@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /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).