From 40406340d224ff9bc2f15b579c488574ef7b1a24 Mon Sep 17 00:00:00 2001 From: Justin M. Forbes Date: Nov 12 2012 18:07:04 +0000 Subject: fix list_del corruption warning on USB audio with twinkle (rhbz 871078) --- diff --git a/USB-EHCI-urb-hcpriv-should-not-be-NULL.patch b/USB-EHCI-urb-hcpriv-should-not-be-NULL.patch new file mode 100644 index 0000000..4b8e537 --- /dev/null +++ b/USB-EHCI-urb-hcpriv-should-not-be-NULL.patch @@ -0,0 +1,72 @@ +commit 2656a9abcf1ec8dd5fee6a75d6997a0f2fa0094e +Author: Alan Stern +Date: Thu Nov 8 10:17:01 2012 -0500 + + USB: EHCI: bugfix: urb->hcpriv should not be NULL + + This patch (as1632b) fixes a bug in ehci-hcd. The USB core uses + urb->hcpriv to determine whether or not an URB is active; host + controller drivers are supposed to set this pointer to a non-NULL + value when an URB is queued. However ehci-hcd sets it to NULL for + isochronous URBs, which defeats the check in usbcore. + + In itself this isn't a big deal. But people have recently found that + certain sequences of actions will cause the snd-usb-audio driver to + reuse URBs without waiting for them to complete. In the absence of + proper checking by usbcore, the URBs get added to their endpoint list + twice. This leads to list corruption and a system freeze. + + The patch makes ehci-hcd assign a meaningful value to urb->hcpriv for + isochronous URBs. Improving robustness always helps. + + Signed-off-by: Alan Stern + Reported-by: Artem S. Tashkinov + Reported-by: Christof Meerwald + CC: + Signed-off-by: Greg Kroah-Hartman + +diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c +index 4b66374..3d98902 100644 +--- a/drivers/usb/host/ehci-q.c ++++ b/drivers/usb/host/ehci-q.c +@@ -264,15 +264,9 @@ ehci_urb_done(struct ehci_hcd *ehci, struct urb *urb, int status) + __releases(ehci->lock) + __acquires(ehci->lock) + { +- if (likely (urb->hcpriv != NULL)) { +- struct ehci_qh *qh = (struct ehci_qh *) urb->hcpriv; +- +- /* S-mask in a QH means it's an interrupt urb */ +- if ((qh->hw->hw_info2 & cpu_to_hc32(ehci, QH_SMASK)) != 0) { +- +- /* ... update hc-wide periodic stats (for usbfs) */ +- ehci_to_hcd(ehci)->self.bandwidth_int_reqs--; +- } ++ if (usb_pipetype(urb->pipe) == PIPE_INTERRUPT) { ++ /* ... update hc-wide periodic stats */ ++ ehci_to_hcd(ehci)->self.bandwidth_int_reqs--; + } + + if (unlikely(urb->unlinked)) { +diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c +index 2e14714..69ebee7 100644 +--- a/drivers/usb/host/ehci-sched.c ++++ b/drivers/usb/host/ehci-sched.c +@@ -1630,7 +1630,7 @@ static void itd_link_urb( + + /* don't need that schedule data any more */ + iso_sched_free (stream, iso_sched); +- urb->hcpriv = NULL; ++ urb->hcpriv = stream; + + ++ehci->isoc_count; + enable_periodic(ehci); +@@ -2029,7 +2029,7 @@ static void sitd_link_urb( + + /* don't need that schedule data any more */ + iso_sched_free (stream, sched); +- urb->hcpriv = NULL; ++ urb->hcpriv = stream; + + ++ehci->isoc_count; + enable_periodic(ehci); diff --git a/USB-report-submission-of-active-URBs.patch b/USB-report-submission-of-active-URBs.patch new file mode 100644 index 0000000..4496c91 --- /dev/null +++ b/USB-report-submission-of-active-URBs.patch @@ -0,0 +1,46 @@ +commit 2f02bc8af3abb846823811af65ec6cc46a4d525d +Author: Alan Stern +Date: Wed Nov 7 16:35:00 2012 -0500 + + USB: report submission of active URBs + + This patch (as1633) changes slightly the way usbcore handled + submissions of URBs that are already active. It will now return + -EBUSY rather than -EINVAL, and it will call WARN_ONCE to draw + people's attention to the bug. + + Signed-off-by: Alan Stern + Signed-off-by: Greg Kroah-Hartman + +diff --git a/Documentation/usb/error-codes.txt b/Documentation/usb/error-codes.txt +index 8d1e2a9..9c3eb84 100644 +--- a/Documentation/usb/error-codes.txt ++++ b/Documentation/usb/error-codes.txt +@@ -21,6 +21,8 @@ Non-USB-specific: + + USB-specific: + ++-EBUSY The URB is already active. ++ + -ENODEV specified USB-device or bus doesn't exist + + -ENOENT specified interface or endpoint does not exist or +diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c +index 3662287..e0d9d94 100644 +--- a/drivers/usb/core/urb.c ++++ b/drivers/usb/core/urb.c +@@ -321,8 +321,13 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags) + struct usb_host_endpoint *ep; + int is_out; + +- if (!urb || urb->hcpriv || !urb->complete) ++ if (!urb || !urb->complete) + return -EINVAL; ++ if (urb->hcpriv) { ++ WARN_ONCE(1, "URB %p submitted while active\n", urb); ++ return -EBUSY; ++ } ++ + dev = urb->dev; + if ((!dev) || (dev->state < USB_STATE_UNAUTHENTICATED)) + return -ENODEV; diff --git a/kernel.spec b/kernel.spec index 92b4d25..57bddc1 100644 --- a/kernel.spec +++ b/kernel.spec @@ -62,7 +62,7 @@ Summary: The Linux kernel # For non-released -rc kernels, this will be appended after the rcX and # gitX tags, so a 3 here would become part of release "0.rcX.gitX.3" # -%global baserelease 5 +%global baserelease 6 %global fedora_build %{baserelease} # base_sublevel is the kernel version we're starting with and patching @@ -791,6 +791,11 @@ Patch22092: net-fix-divide-by-zero-in-tcp-algorithm-illinois.patch Patch22100: uprobes-upstream-backport.patch +#rhbz 871078 +Patch22110: usb-audio-fix-crash-at-re-preparing-the-PCM-stream.patch +Patch22111: USB-EHCI-urb-hcpriv-should-not-be-NULL.patch +Patch22112: USB-report-submission-of-active-URBs.patch + # END OF PATCH DEFINITIONS %endif @@ -1529,6 +1534,11 @@ ApplyPatch net-fix-divide-by-zero-in-tcp-algorithm-illinois.patch ApplyPatch uprobes-upstream-backport.patch +#rhbz 871078 +ApplyPatch usb-audio-fix-crash-at-re-preparing-the-PCM-stream.patch +ApplyPatch USB-EHCI-urb-hcpriv-should-not-be-NULL.patch +ApplyPatch USB-report-submission-of-active-URBs.patch + # END OF PATCH APPLICATIONS %endif @@ -2382,6 +2392,9 @@ fi # ||----w | # || || %changelog +* Mon Nov 12 2012 Justin M. Forbes +- fix list_del corruption warning on USB audio with twinkle (rhbz 871078) + * Fri Nov 09 2012 Josh Boyer - Fix vanilla kernel builds (reported by Thorsten Leemhuis) diff --git a/usb-audio-fix-crash-at-re-preparing-the-PCM-stream.patch b/usb-audio-fix-crash-at-re-preparing-the-PCM-stream.patch new file mode 100644 index 0000000..6d840ea --- /dev/null +++ b/usb-audio-fix-crash-at-re-preparing-the-PCM-stream.patch @@ -0,0 +1,125 @@ +At Thu, 08 Nov 2012 08:31:35 +0100, +Daniel Mack wrote: +(snip) +> >> We can't simply stop both endpoints in the prepare callback. +> > +> > The new function doesn't stop the stream by itself but it just syncs +> > if the stream is being stopped beforehand. So, it's safe to call it +> > there. +> > +> > Maybe the name was confusing. It should have been like +> > snd_usb_endpoint_sync_pending_stop() or such. +> +> Ah, right. I was errornously looking closer to Alan's patch but then +> replied to yours. Alright then - thanks for explaining :) + +OK, thanks for checking. + +FWIW, below is the patch I applied now to for-linus branch. +Renamed the function, added the comment and put NULL check to the +function to simplify. + + +Takashi + +--- +From: Takashi Iwai +Subject: [PATCH] ALSA: usb-audio: Fix crash at re-preparing the PCM stream + +There are bug reports of a crash with USB-audio devices when PCM +prepare is performed immediately after the stream is stopped via +trigger callback. It turned out that the problem is that we don't +wait until all URBs are killed. + +This patch adds a new function to synchronize the pending stop +operation on an endpoint, and calls in the prepare callback for +avoiding the crash above. + +Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=49181 + +Reported-and-tested-by: Artem S. Tashkinov +Cc: [v3.6] +Signed-off-by: Takashi Iwai +--- + sound/usb/endpoint.c | 13 +++++++++++++ + sound/usb/endpoint.h | 1 + + sound/usb/pcm.c | 3 +++ + 3 files changed, 17 insertions(+) + +diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c +index 7f78c6d..34de6f2 100644 +--- a/sound/usb/endpoint.c ++++ b/sound/usb/endpoint.c +@@ -35,6 +35,7 @@ + + #define EP_FLAG_ACTIVATED 0 + #define EP_FLAG_RUNNING 1 ++#define EP_FLAG_STOPPING 2 + + /* + * snd_usb_endpoint is a model that abstracts everything related to an +@@ -502,10 +503,20 @@ static int wait_clear_urbs(struct snd_usb_endpoint *ep) + if (alive) + snd_printk(KERN_ERR "timeout: still %d active urbs on EP #%x\n", + alive, ep->ep_num); ++ clear_bit(EP_FLAG_STOPPING, &ep->flags); + + return 0; + } + ++/* sync the pending stop operation; ++ * this function itself doesn't trigger the stop operation ++ */ ++void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep) ++{ ++ if (ep && test_bit(EP_FLAG_STOPPING, &ep->flags)) ++ wait_clear_urbs(ep); ++} ++ + /* + * unlink active urbs. + */ +@@ -918,6 +929,8 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep, + + if (wait) + wait_clear_urbs(ep); ++ else ++ set_bit(EP_FLAG_STOPPING, &ep->flags); + } + } + +diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h +index 6376ccf..3d4c970 100644 +--- a/sound/usb/endpoint.h ++++ b/sound/usb/endpoint.h +@@ -19,6 +19,7 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep, + int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, int can_sleep); + void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep, + int force, int can_sleep, int wait); ++void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep); + int snd_usb_endpoint_activate(struct snd_usb_endpoint *ep); + int snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep); + void snd_usb_endpoint_free(struct list_head *head); +diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c +index 37428f7..5c12a3f 100644 +--- a/sound/usb/pcm.c ++++ b/sound/usb/pcm.c +@@ -552,6 +552,9 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) + if (snd_BUG_ON(!subs->data_endpoint)) + return -EIO; + ++ snd_usb_endpoint_sync_pending_stop(subs->sync_endpoint); ++ snd_usb_endpoint_sync_pending_stop(subs->data_endpoint); ++ + /* some unit conversions in runtime */ + subs->data_endpoint->maxframesize = + bytes_to_frames(runtime, subs->data_endpoint->maxpacksize); +-- +1.8.0 + + +-- +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/