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