Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

ohci-hcd: Fix race condition caused by ohci_urb_enqueue() and io_watchdog_func()

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>

authored by

Shigeru Yoshida and committed by
Greg Kroah-Hartman
b2685bda 71a0483d

+10 -4
+7 -3
drivers/usb/host/ohci-hcd.c
··· 74 74 75 75 #define STATECHANGE_DELAY msecs_to_jiffies(300) 76 76 #define IO_WATCHDOG_DELAY msecs_to_jiffies(275) 77 + #define IO_WATCHDOG_OFF 0xffffff00 77 78 78 79 #include "ohci.h" 79 80 #include "pci-quirks.h" ··· 232 231 } 233 232 234 233 /* Start up the I/O watchdog timer, if it's not running */ 235 - if (!timer_pending(&ohci->io_watchdog) && 234 + if (ohci->prev_frame_no == IO_WATCHDOG_OFF && 236 235 list_empty(&ohci->eds_in_use) && 237 236 !(ohci->flags & OHCI_QUIRK_QEMU)) { 238 237 ohci->prev_frame_no = ohci_frame_no(ohci); ··· 502 501 return 0; 503 502 504 503 timer_setup(&ohci->io_watchdog, io_watchdog_func, 0); 504 + ohci->prev_frame_no = IO_WATCHDOG_OFF; 505 505 506 506 ohci->hcca = dma_alloc_coherent (hcd->self.controller, 507 507 sizeof(*ohci->hcca), &ohci->hcca_dma, GFP_KERNEL); ··· 732 730 u32 head; 733 731 struct ed *ed; 734 732 struct td *td, *td_start, *td_next; 735 - unsigned frame_no; 733 + unsigned frame_no, prev_frame_no = IO_WATCHDOG_OFF; 736 734 unsigned long flags; 737 735 738 736 spin_lock_irqsave(&ohci->lock, flags); ··· 837 835 } 838 836 } 839 837 if (!list_empty(&ohci->eds_in_use)) { 840 - ohci->prev_frame_no = frame_no; 838 + prev_frame_no = frame_no; 841 839 ohci->prev_wdh_cnt = ohci->wdh_cnt; 842 840 ohci->prev_donehead = ohci_readl(ohci, 843 841 &ohci->regs->donehead); ··· 847 845 } 848 846 849 847 done: 848 + ohci->prev_frame_no = prev_frame_no; 850 849 spin_unlock_irqrestore(&ohci->lock, flags); 851 850 } 852 851 ··· 976 973 if (quirk_nec(ohci)) 977 974 flush_work(&ohci->nec_work); 978 975 del_timer_sync(&ohci->io_watchdog); 976 + ohci->prev_frame_no = IO_WATCHDOG_OFF; 979 977 980 978 ohci_writel (ohci, OHCI_INTR_MIE, &ohci->regs->intrdisable); 981 979 ohci_usb_reset(ohci);
+3 -1
drivers/usb/host/ohci-hub.c
··· 311 311 rc = ohci_rh_suspend (ohci, 0); 312 312 spin_unlock_irq (&ohci->lock); 313 313 314 - if (rc == 0) 314 + if (rc == 0) { 315 315 del_timer_sync(&ohci->io_watchdog); 316 + ohci->prev_frame_no = IO_WATCHDOG_OFF; 317 + } 316 318 return rc; 317 319 } 318 320