linux-kernel.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,
	Shigeru Yoshida <Shigeru.Yoshida@windriver.com>,
	Haiqing Bai <Haiqing.Bai@windriver.com>,
	Alan Stern <stern@rowland.harvard.edu>
Subject: [PATCH 4.9 13/39] ohci-hcd: Fix race condition caused by ohci_urb_enqueue() and io_watchdog_func()
Date: Mon, 26 Feb 2018 21:20:34 +0100	[thread overview]
Message-ID: <20180226201644.257868973@linuxfoundation.org> (raw)
In-Reply-To: <20180226201643.660109883@linuxfoundation.org>

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

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

From: Shigeru Yoshida <shigeru.yoshida@windriver.com>

commit b2685bdacdaab065c172b97b55ab46c6be77a037 upstream.

Running io_watchdog_func() while ohci_urb_enqueue() is running can
cause a race condition where ohci->prev_frame_no is corrupted and the
watchdog can mis-detect following error:

  ohci-platform 664a0800.usb: frame counter not updating; disabled
  ohci-platform 664a0800.usb: HC died; cleaning up

Specifically, following scenario causes a race condition:

  1. ohci_urb_enqueue() calls spin_lock_irqsave(&ohci->lock, flags)
     and enters the critical section
  2. ohci_urb_enqueue() calls timer_pending(&ohci->io_watchdog) and it
     returns false
  3. ohci_urb_enqueue() sets ohci->prev_frame_no to a frame number
     read by ohci_frame_no(ohci)
  4. ohci_urb_enqueue() schedules io_watchdog_func() with mod_timer()
  5. ohci_urb_enqueue() calls spin_unlock_irqrestore(&ohci->lock,
     flags) and exits the critical section
  6. Later, ohci_urb_enqueue() is called
  7. ohci_urb_enqueue() calls spin_lock_irqsave(&ohci->lock, flags)
     and enters the critical section
  8. The timer scheduled on step 4 expires and io_watchdog_func() runs
  9. io_watchdog_func() calls spin_lock_irqsave(&ohci->lock, flags)
     and waits on it because ohci_urb_enqueue() is already in the
     critical section on step 7
 10. ohci_urb_enqueue() calls timer_pending(&ohci->io_watchdog) and it
     returns false
 11. ohci_urb_enqueue() sets ohci->prev_frame_no to new frame number
     read by ohci_frame_no(ohci) because the frame number proceeded
     between step 3 and 6
 12. ohci_urb_enqueue() schedules io_watchdog_func() with mod_timer()
 13. ohci_urb_enqueue() calls spin_unlock_irqrestore(&ohci->lock,
     flags) and exits the critical section, then wake up
     io_watchdog_func() which is waiting on step 9
 14. io_watchdog_func() enters the critical section
 15. io_watchdog_func() calls ohci_frame_no(ohci) and set frame_no
     variable to the frame number
 16. io_watchdog_func() compares frame_no and ohci->prev_frame_no

On step 16, because this calling of io_watchdog_func() is scheduled on
step 4, the frame number set in ohci->prev_frame_no is expected to the
number set on step 3.  However, ohci->prev_frame_no is overwritten on
step 11.  Because step 16 is executed soon after step 11, the frame
number might not proceed, so ohci->prev_frame_no must equals to
frame_no.

To address above scenario, this patch introduces a special sentinel
value IO_WATCHDOG_OFF and set this value to ohci->prev_frame_no when
the watchdog is not pending or running.  When ohci_urb_enqueue()
schedules the watchdog (step 4 and 12 above), it compares
ohci->prev_frame_no to IO_WATCHDOG_OFF so that ohci->prev_frame_no is
not overwritten while io_watchdog_func() is running.

Signed-off-by: Shigeru Yoshida <Shigeru.Yoshida@windriver.com>
Signed-off-by: Haiqing Bai <Haiqing.Bai@windriver.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/usb/host/ohci-hcd.c |   10 +++++++---
 drivers/usb/host/ohci-hub.c |    4 +++-
 2 files changed, 10 insertions(+), 4 deletions(-)

--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -73,6 +73,7 @@ static const char	hcd_name [] = "ohci_hc
 
 #define	STATECHANGE_DELAY	msecs_to_jiffies(300)
 #define	IO_WATCHDOG_DELAY	msecs_to_jiffies(275)
+#define	IO_WATCHDOG_OFF		0xffffff00
 
 #include "ohci.h"
 #include "pci-quirks.h"
@@ -230,7 +231,7 @@ static int ohci_urb_enqueue (
 		}
 
 		/* Start up the I/O watchdog timer, if it's not running */
-		if (!timer_pending(&ohci->io_watchdog) &&
+		if (ohci->prev_frame_no == IO_WATCHDOG_OFF &&
 				list_empty(&ohci->eds_in_use) &&
 				!(ohci->flags & OHCI_QUIRK_QEMU)) {
 			ohci->prev_frame_no = ohci_frame_no(ohci);
@@ -501,6 +502,7 @@ static int ohci_init (struct ohci_hcd *o
 
 	setup_timer(&ohci->io_watchdog, io_watchdog_func,
 			(unsigned long) ohci);
+	ohci->prev_frame_no = IO_WATCHDOG_OFF;
 
 	ohci->hcca = dma_alloc_coherent (hcd->self.controller,
 			sizeof(*ohci->hcca), &ohci->hcca_dma, GFP_KERNEL);
@@ -730,7 +732,7 @@ static void io_watchdog_func(unsigned lo
 	u32		head;
 	struct ed	*ed;
 	struct td	*td, *td_start, *td_next;
-	unsigned	frame_no;
+	unsigned	frame_no, prev_frame_no = IO_WATCHDOG_OFF;
 	unsigned long	flags;
 
 	spin_lock_irqsave(&ohci->lock, flags);
@@ -835,7 +837,7 @@ static void io_watchdog_func(unsigned lo
 			}
 		}
 		if (!list_empty(&ohci->eds_in_use)) {
-			ohci->prev_frame_no = frame_no;
+			prev_frame_no = frame_no;
 			ohci->prev_wdh_cnt = ohci->wdh_cnt;
 			ohci->prev_donehead = ohci_readl(ohci,
 					&ohci->regs->donehead);
@@ -845,6 +847,7 @@ static void io_watchdog_func(unsigned lo
 	}
 
  done:
+	ohci->prev_frame_no = prev_frame_no;
 	spin_unlock_irqrestore(&ohci->lock, flags);
 }
 
@@ -973,6 +976,7 @@ static void ohci_stop (struct usb_hcd *h
 	if (quirk_nec(ohci))
 		flush_work(&ohci->nec_work);
 	del_timer_sync(&ohci->io_watchdog);
+	ohci->prev_frame_no = IO_WATCHDOG_OFF;
 
 	ohci_writel (ohci, OHCI_INTR_MIE, &ohci->regs->intrdisable);
 	ohci_usb_reset(ohci);
--- a/drivers/usb/host/ohci-hub.c
+++ b/drivers/usb/host/ohci-hub.c
@@ -310,8 +310,10 @@ static int ohci_bus_suspend (struct usb_
 		rc = ohci_rh_suspend (ohci, 0);
 	spin_unlock_irq (&ohci->lock);
 
-	if (rc == 0)
+	if (rc == 0) {
 		del_timer_sync(&ohci->io_watchdog);
+		ohci->prev_frame_no = IO_WATCHDOG_OFF;
+	}
 	return rc;
 }
 

  parent reply	other threads:[~2018-02-26 20:20 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-26 20:20 [PATCH 4.9 00/39] 4.9.85-stable review Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 01/39] netfilter: drop outermost socket lock in getsockopt() Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 02/39] xtensa: fix high memory/reserved memory collision Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 03/39] scsi: ibmvfc: fix misdefined reserved field in ibmvfc_fcp_rsp_info Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 04/39] cfg80211: fix cfg80211_beacon_dup Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 05/39] X.509: fix BUG_ON() when hash algorithm is unsupported Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 06/39] PKCS#7: fix certificate chain verification Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 07/39] RDMA/uverbs: Protect from command mask overflow Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 08/39] iio: buffer: check if a buffer has been set up when poll is called Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 09/39] iio: adis_lib: Initialize trigger before requesting interrupt Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 10/39] x86/oprofile: Fix bogus GCC-8 warning in nmi_setup() Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 11/39] irqchip/gic-v3: Use wmb() instead of smb_wmb() in gic_raise_softirq() Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 12/39] PCI/cxgb4: Extend T3 PCI quirk to T4+ devices Greg Kroah-Hartman
2018-02-26 20:20 ` Greg Kroah-Hartman [this message]
2018-02-26 20:20 ` [PATCH 4.9 14/39] usb: ohci: Proper handling of ed_rm_list to handle race condition between usb_kill_urb() and finish_unlinks() Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 15/39] arm64: Disable unhandled signal log messages by default Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 16/39] Add delay-init quirk for Corsair K70 RGB keyboards Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 17/39] drm/edid: Add 6 bpc quirk for CPT panel in Asus UX303LA Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 18/39] usb: dwc3: gadget: Set maxpacket size for ep0 IN Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 19/39] usb: ldusb: add PIDs for new CASSY devices supported by this driver Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 20/39] Revert "usb: musb: host: dont start next rx urb if current one failed" Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 21/39] usb: gadget: f_fs: Process all descriptors during bind Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 22/39] usb: renesas_usbhs: missed the "running" flag in usb_dmac with rx path Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 23/39] drm/amdgpu: Add dpm quirk for Jet PRO (v2) Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 24/39] drm/amdgpu: add atpx quirk handling (v2) Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 25/39] drm/amdgpu: Avoid leaking PM domain on driver unbind (v2) Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 26/39] drm/amdgpu: add new device to use atpx quirk Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 27/39] binder: add missing binder_unlock() Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 28/39] X.509: fix NULL dereference when restricting key with unsupported_sig Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 29/39] mm: avoid spurious bad pmd warning messages Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 30/39] fs/dax.c: fix inefficiency in dax_writeback_mapping_range() Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 31/39] libnvdimm: fix integer overflow static analysis warning Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 32/39] device-dax: implement ->split() to catch invalid munmap attempts Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 33/39] mm: introduce get_user_pages_longterm Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 34/39] v4l2: disable filesystem-dax mapping support Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 35/39] IB/core: disable memory registration of filesystem-dax vmas Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 36/39] libnvdimm, dax: fix 1GB-aligned namespaces vs physical misalignment Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 37/39] mm: Fix devm_memremap_pages() collision handling Greg Kroah-Hartman
2018-02-26 20:20 ` [PATCH 4.9 38/39] mm: fail get_vaddr_frames() for filesystem-dax mappings Greg Kroah-Hartman
2018-02-26 20:21 ` [PATCH 4.9 39/39] x86/entry/64: Clear extra registers beyond syscall arguments, to reduce speculation attack surface Greg Kroah-Hartman
2018-02-27  0:57 ` [PATCH 4.9 00/39] 4.9.85-stable review Shuah Khan
2018-02-27  7:12 ` Naresh Kamboju
2018-02-27 14:56 ` Guenter Roeck
2018-02-27 18:34   ` 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=20180226201644.257868973@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=Haiqing.Bai@windriver.com \
    --cc=Shigeru.Yoshida@windriver.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /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).