fix list_del corruption warning on USB audio with twinkle (rhbz 871078)
This commit is contained in:
parent
123d75be63
commit
02c38dbef5
|
@ -0,0 +1,72 @@
|
||||||
|
commit 2656a9abcf1ec8dd5fee6a75d6997a0f2fa0094e
|
||||||
|
Author: Alan Stern <stern@rowland.harvard.edu>
|
||||||
|
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 <stern@rowland.harvard.edu>
|
||||||
|
Reported-by: Artem S. Tashkinov <t.artem@lycos.com>
|
||||||
|
Reported-by: Christof Meerwald <cmeerw@cmeerw.org>
|
||||||
|
CC: <stable@vger.kernel.org>
|
||||||
|
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
||||||
|
|
||||||
|
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);
|
|
@ -0,0 +1,46 @@
|
||||||
|
commit 2f02bc8af3abb846823811af65ec6cc46a4d525d
|
||||||
|
Author: Alan Stern <stern@rowland.harvard.edu>
|
||||||
|
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 <stern@rowland.harvard.edu>
|
||||||
|
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
||||||
|
|
||||||
|
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;
|
15
kernel.spec
15
kernel.spec
|
@ -54,7 +54,7 @@ Summary: The Linux kernel
|
||||||
# For non-released -rc kernels, this will be appended after the rcX and
|
# 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"
|
# gitX tags, so a 3 here would become part of release "0.rcX.gitX.3"
|
||||||
#
|
#
|
||||||
%global baserelease 1
|
%global baserelease 2
|
||||||
%global fedora_build %{baserelease}
|
%global fedora_build %{baserelease}
|
||||||
|
|
||||||
# base_sublevel is the kernel version we're starting with and patching
|
# base_sublevel is the kernel version we're starting with and patching
|
||||||
|
@ -705,6 +705,11 @@ Patch22092: net-fix-divide-by-zero-in-tcp-algorithm-illinois.patch
|
||||||
Patch30000: weird-root-dentry-name-debug.patch
|
Patch30000: weird-root-dentry-name-debug.patch
|
||||||
Patch30010: debug-808990.patch
|
Patch30010: debug-808990.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
|
# END OF PATCH DEFINITIONS
|
||||||
|
|
||||||
%endif
|
%endif
|
||||||
|
@ -1327,6 +1332,11 @@ ApplyPatch 0012-ext4-serialize-fallocate-with-ext4_convert_unwritten.patch
|
||||||
#rhbz 871923 871848 CVE-2012-4565
|
#rhbz 871923 871848 CVE-2012-4565
|
||||||
ApplyPatch net-fix-divide-by-zero-in-tcp-algorithm-illinois.patch
|
ApplyPatch net-fix-divide-by-zero-in-tcp-algorithm-illinois.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
|
# END OF PATCH APPLICATIONS
|
||||||
|
|
||||||
%endif
|
%endif
|
||||||
|
@ -2027,6 +2037,9 @@ fi
|
||||||
# and build.
|
# and build.
|
||||||
|
|
||||||
%changelog
|
%changelog
|
||||||
|
* Mon Nov 12 2012 Justin M. Forbes <jforbes@redhat.com>
|
||||||
|
- fix list_del corruption warning on USB audio with twinkle (rhbz 871078)
|
||||||
|
|
||||||
* Mon Nov 05 2012 Justin M. Forbes <jforbes@redhat.com> 3.6.6-1
|
* Mon Nov 05 2012 Justin M. Forbes <jforbes@redhat.com> 3.6.6-1
|
||||||
- Linux 3.6.6
|
- Linux 3.6.6
|
||||||
|
|
||||||
|
|
|
@ -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 <tiwai@suse.de>
|
||||||
|
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 <t.artem@lycos.com>
|
||||||
|
Cc: <stable@vger.kernel.org> [v3.6]
|
||||||
|
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
||||||
|
---
|
||||||
|
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/
|
Loading…
Reference in New Issue