* [PATCH 2.6.27.8 1/1] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer
@ 2009-01-14 18:04 Robert Krakora
2009-01-14 18:31 ` Robert Krakora
2009-01-15 11:06 ` [PATCH 2.6.27.8 1/1] " Pádraig Brady
0 siblings, 2 replies; 14+ messages in thread
From: Robert Krakora @ 2009-01-14 18:04 UTC (permalink / raw)
To: video4linux-list
em28xx: Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
Signed-off-by: Robert V. Krakora <rob.krakora@messagenetsystems.com>
diff -r 6896782d783d linux/drivers/media/video/em28xx/em28xx-audio.c
--- a/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
10:06:12 2009 -0200
+++ b/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
12:47:00 2009 -0500
@@ -62,11 +62,20 @@
int i;
dprintk("Stopping isoc\n");
- for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
- usb_unlink_urb(dev->adev.urb[i]);
- usb_free_urb(dev->adev.urb[i]);
- dev->adev.urb[i] = NULL;
- }
+ for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
+ usb_unlink_urb(dev->adev.urb[i]);
+ usb_free_urb(dev->adev.urb[i]);
+ dev->adev.urb[i] = NULL;
+ if (dev->adev.urb[i]) {
+ usb_unlink_urb(dev->adev.urb[i]);
+ usb_free_urb(dev->adev.urb[i]);
+ dev->adev.urb[i] = NULL;
+ }
+ if (dev->adev.transfer_buffer) {
+ kfree(dev->adev.transfer_buffer[i]);
+ dev->adev.transfer_buffer[i] = NULL;
+ }
+ }
return 0;
}
@@ -458,11 +467,15 @@
*substream)
#endif
{
+ unsigned long flags;
+
struct em28xx *dev;
-
snd_pcm_uframes_t hwptr_done;
+
dev = snd_pcm_substream_chip(substream);
+ spin_lock_irqsave(&dev->adev.slock, flags);
hwptr_done = dev->adev.hwptr_done_capture;
+ spin_unlock_irqrestore(&dev->adev.slock, flags);
return hwptr_done;
}
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 2.6.27.8 1/1] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer
2009-01-14 18:04 [PATCH 2.6.27.8 1/1] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer Robert Krakora
@ 2009-01-14 18:31 ` Robert Krakora
2009-01-14 18:44 ` Robert Krakora
2009-01-15 11:06 ` [PATCH 2.6.27.8 1/1] " Pádraig Brady
1 sibling, 1 reply; 14+ messages in thread
From: Robert Krakora @ 2009-01-14 18:31 UTC (permalink / raw)
To: video4linux-list
====
em28xx: Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
From: Robert Krakora <rob.krakora@messagenetsystems.com>
Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
Priority: normal
Signed-off-by: Robert Krakora <rob.krakora@messagenetsystems.com>
diff -r 6896782d783d linux/drivers/media/video/em28xx/em28xx-audio.c
--- a/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
10:06:12 2009 -0200
+++ b/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
12:47:00 2009 -0500
@@ -62,11 +62,20 @@
int i;
dprintk("Stopping isoc\n");
- for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
- usb_unlink_urb(dev->adev.urb[i]);
- usb_free_urb(dev->adev.urb[i]);
- dev->adev.urb[i] = NULL;
- }
+ for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
+ usb_unlink_urb(dev->adev.urb[i]);
+ usb_free_urb(dev->adev.urb[i]);
+ dev->adev.urb[i] = NULL;
+ if (dev->adev.urb[i]) {
+ usb_unlink_urb(dev->adev.urb[i]);
+ usb_free_urb(dev->adev.urb[i]);
+ dev->adev.urb[i] = NULL;
+ }
+ if (dev->adev.transfer_buffer) {
+ kfree(dev->adev.transfer_buffer[i]);
+ dev->adev.transfer_buffer[i] = NULL;
+ }
+ }
return 0;
}
@@ -458,11 +467,15 @@
*substream)
#endif
{
+ unsigned long flags;
+
struct em28xx *dev;
-
snd_pcm_uframes_t hwptr_done;
+
dev = snd_pcm_substream_chip(substream);
+ spin_lock_irqsave(&dev->adev.slock, flags);
hwptr_done = dev->adev.hwptr_done_capture;
+ spin_unlock_irqrestore(&dev->adev.slock, flags);
return hwptr_done;
}
--
Rob Krakora
Software Engineer
MessageNet Systems
101 East Carmel Dr. Suite 105
Carmel, IN 46032
(317)566-1677 Ext. 206
(317)663-0808 Fax
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 2.6.27.8 1/1] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer
2009-01-14 18:31 ` Robert Krakora
@ 2009-01-14 18:44 ` Robert Krakora
2009-01-14 18:55 ` [PATCH 1/4] " Robert Krakora
0 siblings, 1 reply; 14+ messages in thread
From: Robert Krakora @ 2009-01-14 18:44 UTC (permalink / raw)
To: video4linux-list
em28xx: Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
From: Robert Krakora <rob.krakora@messagenetsystems.com>
Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
Priority: normal
Signed-off-by: Robert Krakora <rob.krakora@messagenetsystems.com>
diff -r 6896782d783d linux/drivers/media/video/em28xx/em28xx-audio.c
--- a/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
10:06:12 2009 -0200
+++ b/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
12:47:00 2009 -0500
@@ -62,11 +62,20 @@
int i;
dprintk("Stopping isoc\n");
- for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
- usb_unlink_urb(dev->adev.urb[i]);
- usb_free_urb(dev->adev.urb[i]);
- dev->adev.urb[i] = NULL;
- }
+ for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
+ usb_unlink_urb(dev->adev.urb[i]);
+ usb_free_urb(dev->adev.urb[i]);
+ dev->adev.urb[i] = NULL;
+ if (dev->adev.urb[i]) {
+ usb_unlink_urb(dev->adev.urb[i]);
+ usb_free_urb(dev->adev.urb[i]);
+ dev->adev.urb[i] = NULL;
+ }
+ if (dev->adev.transfer_buffer) {
+ kfree(dev->adev.transfer_buffer[i]);
+ dev->adev.transfer_buffer[i] = NULL;
+ }
+ }
return 0;
}
@@ -458,11 +467,15 @@
*substream)
#endif
{
+ unsigned long flags;
+
struct em28xx *dev;
-
snd_pcm_uframes_t hwptr_done;
+
dev = snd_pcm_substream_chip(substream);
+ spin_lock_irqsave(&dev->adev.slock, flags);
hwptr_done = dev->adev.hwptr_done_capture;
+ spin_unlock_irqrestore(&dev->adev.slock, flags);
return hwptr_done;
}
--
Rob Krakora
Software Engineer
MessageNet Systems
101 East Carmel Dr. Suite 105
Carmel, IN 46032
(317)566-1677 Ext. 206
(317)663-0808 Fax
--
Rob Krakora
Software Engineer
MessageNet Systems
101 East Carmel Dr. Suite 105
Carmel, IN 46032
(317)566-1677 Ext. 206
(317)663-0808 Fax
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/4] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer
2009-01-14 18:44 ` Robert Krakora
@ 2009-01-14 18:55 ` Robert Krakora
2009-01-14 20:02 ` Fwd: " Robert Krakora
0 siblings, 1 reply; 14+ messages in thread
From: Robert Krakora @ 2009-01-14 18:55 UTC (permalink / raw)
To: video4linux-list
em28xx: Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
From: Robert Krakora <rob.krakora@messagenetsystems.com>
Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
Priority: normal
Signed-off-by: Robert Krakora <rob.krakora@messagenetsystems.com>
diff -r 6896782d783d linux/drivers/media/video/em28xx/em28xx-audio.c
--- a/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
10:06:12 2009 -0200
+++ b/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
12:47:00 2009 -0500
@@ -62,11 +62,20 @@
int i;
dprintk("Stopping isoc\n");
- for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
- usb_unlink_urb(dev->adev.urb[i]);
- usb_free_urb(dev->adev.urb[i]);
- dev->adev.urb[i] = NULL;
- }
+ for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
+ usb_unlink_urb(dev->adev.urb[i]);
+ usb_free_urb(dev->adev.urb[i]);
+ dev->adev.urb[i] = NULL;
+ if (dev->adev.urb[i]) {
+ usb_unlink_urb(dev->adev.urb[i]);
+ usb_free_urb(dev->adev.urb[i]);
+ dev->adev.urb[i] = NULL;
+ }
+ if (dev->adev.transfer_buffer) {
+ kfree(dev->adev.transfer_buffer[i]);
+ dev->adev.transfer_buffer[i] = NULL;
+ }
+ }
return 0;
}
@@ -458,11 +467,15 @@
*substream)
#endif
{
+ unsigned long flags;
+
struct em28xx *dev;
-
snd_pcm_uframes_t hwptr_done;
+
dev = snd_pcm_substream_chip(substream);
+ spin_lock_irqsave(&dev->adev.slock, flags);
hwptr_done = dev->adev.hwptr_done_capture;
+ spin_unlock_irqrestore(&dev->adev.slock, flags);
return hwptr_done;
}
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 14+ messages in thread* Fwd: [PATCH 1/4] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer
2009-01-14 18:55 ` [PATCH 1/4] " Robert Krakora
@ 2009-01-14 20:02 ` Robert Krakora
2009-01-14 20:08 ` Robert Krakora
2009-01-15 22:31 ` Robert Krakora
0 siblings, 2 replies; 14+ messages in thread
From: Robert Krakora @ 2009-01-14 20:02 UTC (permalink / raw)
To: video4linux-list
em28xx: Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
From: Robert Krakora <rob.krakora@messagenetsystems.com>
Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
Priority: normal
Signed-off-by: Robert Krakora <rob.krakora@messagenetsystems.com>
diff -r 6896782d783d linux/drivers/media/video/em28xx/em28xx-audio.c
--- a/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
10:06:12 2009 -0200
+++ b/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
15:01:20 2009 -0500
@@ -62,11 +62,17 @@
int i;
dprintk("Stopping isoc\n");
- for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
- usb_unlink_urb(dev->adev.urb[i]);
- usb_free_urb(dev->adev.urb[i]);
- dev->adev.urb[i] = NULL;
- }
+ for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
+ if (dev->adev.urb[i]) {
+ usb_unlink_urb(dev->adev.urb[i]);
+ usb_free_urb(dev->adev.urb[i]);
+ dev->adev.urb[i] = NULL;
+ }
+ if (dev->adev.transfer_buffer) {
+ kfree(dev->adev.transfer_buffer[i]);
+ dev->adev.transfer_buffer[i] = NULL;
+ }
+ }
return 0;
}
@@ -458,11 +464,15 @@
*substream)
#endif
{
+ unsigned long flags;
+
struct em28xx *dev;
-
snd_pcm_uframes_t hwptr_done;
+
dev = snd_pcm_substream_chip(substream);
+ spin_lock_irqsave(&dev->adev.slock, flags);
hwptr_done = dev->adev.hwptr_done_capture;
+ spin_unlock_irqrestore(&dev->adev.slock, flags);
return hwptr_done;
}
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/4] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer
2009-01-14 20:02 ` Fwd: " Robert Krakora
@ 2009-01-14 20:08 ` Robert Krakora
2009-01-15 22:31 ` Robert Krakora
1 sibling, 0 replies; 14+ messages in thread
From: Robert Krakora @ 2009-01-14 20:08 UTC (permalink / raw)
To: video4linux-list
em28xx: Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
From: Robert Krakora <rob.krakora@messagenetsystems.com>
Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
Priority: normal
Signed-off-by: Robert Krakora <rob.krakora@messagenetsystems.com>
diff -r 6896782d783d linux/drivers/media/video/em28xx/em28xx-audio.c
--- a/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
10:06:12 2009 -0200
+++ b/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
15:01:20 2009 -0500
@@ -62,11 +62,17 @@
int i;
dprintk("Stopping isoc\n");
- for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
- usb_unlink_urb(dev->adev.urb[i]);
- usb_free_urb(dev->adev.urb[i]);
- dev->adev.urb[i] = NULL;
- }
+ for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
+ if (dev->adev.urb[i]) {
+ usb_unlink_urb(dev->adev.urb[i]);
+ usb_free_urb(dev->adev.urb[i]);
+ dev->adev.urb[i] = NULL;
+ }
+ if (dev->adev.transfer_buffer[i]) {
+ kfree(dev->adev.transfer_buffer[i]);
+ dev->adev.transfer_buffer[i] = NULL;
+ }
+ }
return 0;
}
@@ -458,11 +464,15 @@
*substream)
#endif
{
+ unsigned long flags;
+
struct em28xx *dev;
-
snd_pcm_uframes_t hwptr_done;
+
dev = snd_pcm_substream_chip(substream);
+ spin_lock_irqsave(&dev->adev.slock, flags);
hwptr_done = dev->adev.hwptr_done_capture;
+ spin_unlock_irqrestore(&dev->adev.slock, flags);
return hwptr_done;
}
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/4] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer
2009-01-14 20:02 ` Fwd: " Robert Krakora
2009-01-14 20:08 ` Robert Krakora
@ 2009-01-15 22:31 ` Robert Krakora
2009-01-16 14:23 ` Robert Krakora
1 sibling, 1 reply; 14+ messages in thread
From: Robert Krakora @ 2009-01-15 22:31 UTC (permalink / raw)
To: video4linux-list, Pádraig Brady
em28xx: Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
From: Robert Krakora <rob.krakora@messagenetsystems.com>
Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
Leak fix kindly contributed by Pádraig Brady.
Priority: normal
Signed-off-by: Robert Krakora <rob.krakora@messagenetsystems.com>
diff -r 7981bdd4e25a linux/drivers/media/video/em28xx/em28xx-audio.c
--- a/linux/drivers/media/video/em28xx/em28xx-audio.c Mon Jan 12
00:18:04 2009 +0000
+++ b/linux/drivers/media/video/em28xx/em28xx-audio.c Thu Jan 15
17:27:27 2009 -0500
@@ -66,6 +66,9 @@
usb_unlink_urb(dev->adev.urb[i]);
usb_free_urb(dev->adev.urb[i]);
dev->adev.urb[i] = NULL;
+
+ kfree(dev->adev.transfer_buffer[i]);
+ dev->adev.transfer_buffer[i] = NULL;
}
return 0;
@@ -458,11 +461,15 @@
*substream)
#endif
{
+ unsigned long flags;
+
struct em28xx *dev;
-
snd_pcm_uframes_t hwptr_done;
+
dev = snd_pcm_substream_chip(substream);
+ spin_lock_irqsave(&dev->adev.slock, flags);
hwptr_done = dev->adev.hwptr_done_capture;
+ spin_unlock_irqrestore(&dev->adev.slock, flags);
return hwptr_done;
}
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/4] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer
2009-01-15 22:31 ` Robert Krakora
@ 2009-01-16 14:23 ` Robert Krakora
0 siblings, 0 replies; 14+ messages in thread
From: Robert Krakora @ 2009-01-16 14:23 UTC (permalink / raw)
To: linux-media
em28xx: Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
From: Robert Krakora <rob.krakora@messagenetsystems.com>
Fix audio URB transfer buffer memory leak and race
condition/corruption of capture pointer
Leak fix kindly contributed by Pádraig Brady.
Priority: normal
Signed-off-by: Robert Krakora <rob.krakora@messagenetsystems.com>
diff -r 7981bdd4e25a linux/drivers/media/video/em28xx/em28xx-audio.c
--- a/linux/drivers/media/video/em28xx/em28xx-audio.c Mon Jan 12
00:18:04 2009 +0000
+++ b/linux/drivers/media/video/em28xx/em28xx-audio.c Thu Jan 15
17:27:27 2009 -0500
@@ -66,6 +66,9 @@
usb_unlink_urb(dev->adev.urb[i]);
usb_free_urb(dev->adev.urb[i]);
dev->adev.urb[i] = NULL;
+
+ kfree(dev->adev.transfer_buffer[i]);
+ dev->adev.transfer_buffer[i] = NULL;
}
return 0;
@@ -458,11 +461,15 @@
*substream)
#endif
{
+ unsigned long flags;
+
struct em28xx *dev;
-
snd_pcm_uframes_t hwptr_done;
+
dev = snd_pcm_substream_chip(substream);
+ spin_lock_irqsave(&dev->adev.slock, flags);
hwptr_done = dev->adev.hwptr_done_capture;
+ spin_unlock_irqrestore(&dev->adev.slock, flags);
return hwptr_done;
}
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2.6.27.8 1/1] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer
2009-01-14 18:04 [PATCH 2.6.27.8 1/1] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer Robert Krakora
2009-01-14 18:31 ` Robert Krakora
@ 2009-01-15 11:06 ` Pádraig Brady
2009-01-15 11:27 ` Markus Rechberger
2009-01-15 14:32 ` Robert Krakora
1 sibling, 2 replies; 14+ messages in thread
From: Pádraig Brady @ 2009-01-15 11:06 UTC (permalink / raw)
To: Robert Krakora; +Cc: video4linux-list
Robert Krakora wrote:
> em28xx: Fix audio URB transfer buffer memory leak and race
> condition/corruption of capture pointer
>
>
> Signed-off-by: Robert V. Krakora <rob.krakora@messagenetsystems.com>
>
> diff -r 6896782d783d linux/drivers/media/video/em28xx/em28xx-audio.c
> --- a/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
> 10:06:12 2009 -0200
> +++ b/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
> 12:47:00 2009 -0500
> @@ -62,11 +62,20 @@
> int i;
>
> dprintk("Stopping isoc\n");
> - for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
> - usb_unlink_urb(dev->adev.urb[i]);
> - usb_free_urb(dev->adev.urb[i]);
> - dev->adev.urb[i] = NULL;
> - }
> + for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
> + usb_unlink_urb(dev->adev.urb[i]);
> + usb_free_urb(dev->adev.urb[i]);
> + dev->adev.urb[i] = NULL;
> + if (dev->adev.urb[i]) {
> + usb_unlink_urb(dev->adev.urb[i]);
> + usb_free_urb(dev->adev.urb[i]);
> + dev->adev.urb[i] = NULL;
> + }
> + if (dev->adev.transfer_buffer) {
> + kfree(dev->adev.transfer_buffer[i]);
> + dev->adev.transfer_buffer[i] = NULL;
> + }
> + }
>
> return 0;
> }
That looks a bit incorrect. I fixed this last week in Markus'
repository, as I thought the leak was specific to that tree:
http://mcentral.de/hg/~mrec/em28xx-new/diff/1cfd9010a552/em28xx-audio.c
Pádraig.
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2.6.27.8 1/1] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer
2009-01-15 11:06 ` [PATCH 2.6.27.8 1/1] " Pádraig Brady
@ 2009-01-15 11:27 ` Markus Rechberger
2009-01-15 14:49 ` Robert Krakora
2009-01-15 14:32 ` Robert Krakora
1 sibling, 1 reply; 14+ messages in thread
From: Markus Rechberger @ 2009-01-15 11:27 UTC (permalink / raw)
To: Pádraig Brady; +Cc: video4linux-list
On Thu, Jan 15, 2009 at 12:06 PM, Pádraig Brady <P@draigbrady.com> wrote:
> Robert Krakora wrote:
>> em28xx: Fix audio URB transfer buffer memory leak and race
>> condition/corruption of capture pointer
>>
>>
>> Signed-off-by: Robert V. Krakora <rob.krakora@messagenetsystems.com>
>>
>> diff -r 6896782d783d linux/drivers/media/video/em28xx/em28xx-audio.c
>> --- a/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
>> 10:06:12 2009 -0200
>> +++ b/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
>> 12:47:00 2009 -0500
>> @@ -62,11 +62,20 @@
>> int i;
>>
>> dprintk("Stopping isoc\n");
>> - for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
>> - usb_unlink_urb(dev->adev.urb[i]);
>> - usb_free_urb(dev->adev.urb[i]);
>> - dev->adev.urb[i] = NULL;
>> - }
>> + for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
>> + usb_unlink_urb(dev->adev.urb[i]);
>> + usb_free_urb(dev->adev.urb[i]);
>> + dev->adev.urb[i] = NULL;
>> + if (dev->adev.urb[i]) {
>> + usb_unlink_urb(dev->adev.urb[i]);
>> + usb_free_urb(dev->adev.urb[i]);
>> + dev->adev.urb[i] = NULL;
>> + }
>> + if (dev->adev.transfer_buffer) {
>> + kfree(dev->adev.transfer_buffer[i]);
>> + dev->adev.transfer_buffer[i] = NULL;
>> + }
>> + }
>>
>> return 0;
>> }
>
> That looks a bit incorrect. I fixed this last week in Markus'
> repository, as I thought the leak was specific to that tree:
> http://mcentral.de/hg/~mrec/em28xx-new/diff/1cfd9010a552/em28xx-audio.c
>
The audio implementation was initially taken from my tree, so all the
initial bugs
of course are inherited too there of course.
regards,
Markus
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2.6.27.8 1/1] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer
2009-01-15 11:27 ` Markus Rechberger
@ 2009-01-15 14:49 ` Robert Krakora
0 siblings, 0 replies; 14+ messages in thread
From: Robert Krakora @ 2009-01-15 14:49 UTC (permalink / raw)
To: Markus Rechberger; +Cc: video4linux-list
On Thu, Jan 15, 2009 at 6:27 AM, Markus Rechberger
<mrechberger@gmail.com> wrote:
> On Thu, Jan 15, 2009 at 12:06 PM, Pádraig Brady <P@draigbrady.com> wrote:
>> Robert Krakora wrote:
>>> em28xx: Fix audio URB transfer buffer memory leak and race
>>> condition/corruption of capture pointer
>>>
>>>
>>> Signed-off-by: Robert V. Krakora <rob.krakora@messagenetsystems.com>
>>>
>>> diff -r 6896782d783d linux/drivers/media/video/em28xx/em28xx-audio.c
>>> --- a/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
>>> 10:06:12 2009 -0200
>>> +++ b/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
>>> 12:47:00 2009 -0500
>>> @@ -62,11 +62,20 @@
>>> int i;
>>>
>>> dprintk("Stopping isoc\n");
>>> - for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
>>> - usb_unlink_urb(dev->adev.urb[i]);
>>> - usb_free_urb(dev->adev.urb[i]);
>>> - dev->adev.urb[i] = NULL;
>>> - }
>>> + for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
>>> + usb_unlink_urb(dev->adev.urb[i]);
>>> + usb_free_urb(dev->adev.urb[i]);
>>> + dev->adev.urb[i] = NULL;
>>> + if (dev->adev.urb[i]) {
>>> + usb_unlink_urb(dev->adev.urb[i]);
>>> + usb_free_urb(dev->adev.urb[i]);
>>> + dev->adev.urb[i] = NULL;
>>> + }
>>> + if (dev->adev.transfer_buffer) {
>>> + kfree(dev->adev.transfer_buffer[i]);
>>> + dev->adev.transfer_buffer[i] = NULL;
>>> + }
>>> + }
>>>
>>> return 0;
>>> }
>>
>> That looks a bit incorrect. I fixed this last week in Markus'
>> repository, as I thought the leak was specific to that tree:
>> http://mcentral.de/hg/~mrec/em28xx-new/diff/1cfd9010a552/em28xx-audio.c
>>
>
> The audio implementation was initially taken from my tree, so all the
> initial bugs
> of course are inherited too there of course.
>
> regards,
> Markus
>
>
Guys:
I will look at Markus' tree first to see if a potential em28xx fix has
already been done. Yes, my first patch had a double free problem. I
fixed last night but did not resubmit until this morning. I would
have probably forgotten if it were not for Padraig's e-mail. I am
sorry if I stepped on anyone's toes.
Best Regards,
--
Rob Krakora
Software Engineer
MessageNet Systems
101 East Carmel Dr. Suite 105
Carmel, IN 46032
(317)566-1677 Ext. 206
(317)663-0808 Fax
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2.6.27.8 1/1] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer
2009-01-15 11:06 ` [PATCH 2.6.27.8 1/1] " Pádraig Brady
2009-01-15 11:27 ` Markus Rechberger
@ 2009-01-15 14:32 ` Robert Krakora
2009-01-15 14:58 ` Pádraig Brady
1 sibling, 1 reply; 14+ messages in thread
From: Robert Krakora @ 2009-01-15 14:32 UTC (permalink / raw)
To: Pádraig Brady; +Cc: video4linux-list
On Thu, Jan 15, 2009 at 6:06 AM, Pádraig Brady <P@draigbrady.com> wrote:
> Robert Krakora wrote:
>> em28xx: Fix audio URB transfer buffer memory leak and race
>> condition/corruption of capture pointer
>>
>>
>> Signed-off-by: Robert V. Krakora <rob.krakora@messagenetsystems.com>
>>
>> diff -r 6896782d783d linux/drivers/media/video/em28xx/em28xx-audio.c
>> --- a/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
>> 10:06:12 2009 -0200
>> +++ b/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
>> 12:47:00 2009 -0500
>> @@ -62,11 +62,20 @@
>> int i;
>>
>> dprintk("Stopping isoc\n");
>> - for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
>> - usb_unlink_urb(dev->adev.urb[i]);
>> - usb_free_urb(dev->adev.urb[i]);
>> - dev->adev.urb[i] = NULL;
>> - }
>> + for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
>> + usb_unlink_urb(dev->adev.urb[i]);
>> + usb_free_urb(dev->adev.urb[i]);
>> + dev->adev.urb[i] = NULL;
>> + if (dev->adev.urb[i]) {
>> + usb_unlink_urb(dev->adev.urb[i]);
>> + usb_free_urb(dev->adev.urb[i]);
>> + dev->adev.urb[i] = NULL;
>> + }
>> + if (dev->adev.transfer_buffer) {
>> + kfree(dev->adev.transfer_buffer[i]);
>> + dev->adev.transfer_buffer[i] = NULL;
>> + }
>> + }
>>
>> return 0;
>> }
>
> That looks a bit incorrect. I fixed this last week in Markus'
> repository, as I thought the leak was specific to that tree:
> http://mcentral.de/hg/~mrec/em28xx-new/diff/1cfd9010a552/em28xx-audio.c
>
> Pádraig.
>
>
Padraig:
I fail to see what looks incorrect about testing for NULL pointers
before freeing. I did have a bug where I left the index number off of
the transfer buffer array which Devin kindly pointed out yesterday.
Also, I am working from main, not a branch.
Best Regards,
--
Rob Krakora
Software Engineer
MessageNet Systems
101 East Carmel Dr. Suite 105
Carmel, IN 46032
(317)566-1677 Ext. 206
(317)663-0808 Fax
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2.6.27.8 1/1] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer
2009-01-15 14:32 ` Robert Krakora
@ 2009-01-15 14:58 ` Pádraig Brady
2009-01-15 15:32 ` Robert Krakora
0 siblings, 1 reply; 14+ messages in thread
From: Pádraig Brady @ 2009-01-15 14:58 UTC (permalink / raw)
To: Robert Krakora; +Cc: video4linux-list
Robert Krakora wrote:
> On Thu, Jan 15, 2009 at 6:06 AM, Pádraig Brady <P@draigbrady.com> wrote:
>> Robert Krakora wrote:
>>> em28xx: Fix audio URB transfer buffer memory leak and race
>>> condition/corruption of capture pointer
>>>
>>>
>>> Signed-off-by: Robert V. Krakora <rob.krakora@messagenetsystems.com>
>>>
>>> diff -r 6896782d783d linux/drivers/media/video/em28xx/em28xx-audio.c
>>> --- a/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
>>> 10:06:12 2009 -0200
>>> +++ b/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
>>> 12:47:00 2009 -0500
>>> @@ -62,11 +62,20 @@
>>> int i;
>>>
>>> dprintk("Stopping isoc\n");
>>> - for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
>>> - usb_unlink_urb(dev->adev.urb[i]);
>>> - usb_free_urb(dev->adev.urb[i]);
>>> - dev->adev.urb[i] = NULL;
>>> - }
>>> + for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
>>> + usb_unlink_urb(dev->adev.urb[i]);
>>> + usb_free_urb(dev->adev.urb[i]);
>>> + dev->adev.urb[i] = NULL;
>>> + if (dev->adev.urb[i]) {
>>> + usb_unlink_urb(dev->adev.urb[i]);
>>> + usb_free_urb(dev->adev.urb[i]);
>>> + dev->adev.urb[i] = NULL;
>>> + }
>>> + if (dev->adev.transfer_buffer) {
>>> + kfree(dev->adev.transfer_buffer[i]);
>>> + dev->adev.transfer_buffer[i] = NULL;
>>> + }
>>> + }
>>>
>>> return 0;
>>> }
>> That looks a bit incorrect. I fixed this last week in Markus'
>> repository, as I thought the leak was specific to that tree:
>> http://mcentral.de/hg/~mrec/em28xx-new/diff/1cfd9010a552/em28xx-audio.c
>>
>
> I fail to see what looks incorrect about testing for NULL pointers
> before freeing.
That's redundant/inefficient. kfree(NULL) is fine.
> I did have a bug where I left the index number off of
> the transfer buffer array which Devin kindly pointed out yesterday.
I was referring to that yes.
I was also referring to the fact you have 2 calls to free the URBs.
I suggest you just do what I did in my patch.
I've tested it well. Without it I was leaking 48KiB
every time VLC changed channel.
cheers,
Pádraig.
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2.6.27.8 1/1] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer
2009-01-15 14:58 ` Pádraig Brady
@ 2009-01-15 15:32 ` Robert Krakora
0 siblings, 0 replies; 14+ messages in thread
From: Robert Krakora @ 2009-01-15 15:32 UTC (permalink / raw)
To: Pádraig Brady; +Cc: video4linux-list
On Thu, Jan 15, 2009 at 9:58 AM, Pádraig Brady <P@draigbrady.com> wrote:
> Robert Krakora wrote:
>> On Thu, Jan 15, 2009 at 6:06 AM, Pádraig Brady <P@draigbrady.com> wrote:
>>> Robert Krakora wrote:
>>>> em28xx: Fix audio URB transfer buffer memory leak and race
>>>> condition/corruption of capture pointer
>>>>
>>>>
>>>> Signed-off-by: Robert V. Krakora <rob.krakora@messagenetsystems.com>
>>>>
>>>> diff -r 6896782d783d linux/drivers/media/video/em28xx/em28xx-audio.c
>>>> --- a/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
>>>> 10:06:12 2009 -0200
>>>> +++ b/linux/drivers/media/video/em28xx/em28xx-audio.c Wed Jan 14
>>>> 12:47:00 2009 -0500
>>>> @@ -62,11 +62,20 @@
>>>> int i;
>>>>
>>>> dprintk("Stopping isoc\n");
>>>> - for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
>>>> - usb_unlink_urb(dev->adev.urb[i]);
>>>> - usb_free_urb(dev->adev.urb[i]);
>>>> - dev->adev.urb[i] = NULL;
>>>> - }
>>>> + for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
>>>> + usb_unlink_urb(dev->adev.urb[i]);
>>>> + usb_free_urb(dev->adev.urb[i]);
>>>> + dev->adev.urb[i] = NULL;
>>>> + if (dev->adev.urb[i]) {
>>>> + usb_unlink_urb(dev->adev.urb[i]);
>>>> + usb_free_urb(dev->adev.urb[i]);
>>>> + dev->adev.urb[i] = NULL;
>>>> + }
>>>> + if (dev->adev.transfer_buffer) {
>>>> + kfree(dev->adev.transfer_buffer[i]);
>>>> + dev->adev.transfer_buffer[i] = NULL;
>>>> + }
>>>> + }
>>>>
>>>> return 0;
>>>> }
>>> That looks a bit incorrect. I fixed this last week in Markus'
>>> repository, as I thought the leak was specific to that tree:
>>> http://mcentral.de/hg/~mrec/em28xx-new/diff/1cfd9010a552/em28xx-audio.c
>>>
>>
>> I fail to see what looks incorrect about testing for NULL pointers
>> before freeing.
>
> That's redundant/inefficient. kfree(NULL) is fine.
>
>> I did have a bug where I left the index number off of
>> the transfer buffer array which Devin kindly pointed out yesterday.
>
> I was referring to that yes.
>
> I was also referring to the fact you have 2 calls to free the URBs.
>
> I suggest you just do what I did in my patch.
> I've tested it well. Without it I was leaking 48KiB
> every time VLC changed channel.
>
> cheers,
> Pádraig.
>
>
Padraig:
Cool. I did not know that kfree(NULL) was O.K. Your patch looks like
the way to go then. I noticed the same memory leak on channel
changes. I will edit my patch then to remove that section. Is this
the correct thing to do? Thanks for all of your help.
Best Regards,
--
Rob Krakora
Software Engineer
MessageNet Systems
101 East Carmel Dr. Suite 105
Carmel, IN 46032
(317)566-1677 Ext. 206
(317)663-0808 Fax
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2009-01-16 14:23 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-14 18:04 [PATCH 2.6.27.8 1/1] em28xx: Fix audio URB transfer buffer memory leak and race condition/corruption of capture pointer Robert Krakora
2009-01-14 18:31 ` Robert Krakora
2009-01-14 18:44 ` Robert Krakora
2009-01-14 18:55 ` [PATCH 1/4] " Robert Krakora
2009-01-14 20:02 ` Fwd: " Robert Krakora
2009-01-14 20:08 ` Robert Krakora
2009-01-15 22:31 ` Robert Krakora
2009-01-16 14:23 ` Robert Krakora
2009-01-15 11:06 ` [PATCH 2.6.27.8 1/1] " Pádraig Brady
2009-01-15 11:27 ` Markus Rechberger
2009-01-15 14:49 ` Robert Krakora
2009-01-15 14:32 ` Robert Krakora
2009-01-15 14:58 ` Pádraig Brady
2009-01-15 15:32 ` Robert Krakora
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox