Re: some missing spin_unlocks
At Wed, 24 Aug 2005 11:56:58 -0400,
Lee Revell wrote:
>
> [added alsa-devel to cc:]
>
> On Mon, 2005-08-22 at 15:26 -0700, Ted Unangst wrote:
> > I think these are all real bugs.
> >
> > sound/synth/emux/emux_synth.c snd_emux_note_on, line 101
> > snd_assert will return without unlocking emu->voice_lock (line 89)
>
> This one is probably a real bug.
The patch below fixes them (already applied to ALSA CVS tree).
[PATCH] Fix missing spin_unlock
Fixed missing spin_unlock.
Signed-off-by: Takashi Iwai <[EMAIL PROTECTED]>
--- linux/sound/pci/au88x0/au88x0_pcm.c:1.9 Thu Aug 18 05:43:12 2005
+++ linux/sound/pci/au88x0/au88x0_pcm.c Wed Aug 24 09:57:25 2005
@@ -220,8 +220,10 @@
vortex_adb_allocroute(chip, -1,
params_channels(hw_params),
substream->stream, type);
- if (dma < 0)
+ if (dma < 0) {
+ spin_unlock_irq(>lock);
return dma;
+ }
stream = substream->runtime->private_data = >dma_adb[dma];
stream->substream = substream;
/* Setup Buffers. */
--- linux/sound/synth/emux/emux_synth.c:1.11Tue Dec 7 07:36:07 2004
+++ linux/sound/synth/emux/emux_synth.c Wed Aug 24 09:57:25 2005
@@ -98,7 +98,6 @@
vp = emu->ops.get_voice(emu, port);
if (vp == NULL || vp->ch < 0)
continue;
- snd_assert(vp->emu != NULL && vp->hw != NULL, return);
if (STATE_IS_PLAYING(vp->state))
emu->ops.terminate(vp);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: some missing spin_unlocks
[added alsa-devel to cc:] On Mon, 2005-08-22 at 15:26 -0700, Ted Unangst wrote: > I think these are all real bugs. > > sound/synth/emux/emux_synth.c snd_emux_note_on, line 101 > snd_assert will return without unlocking emu->voice_lock (line 89) This one is probably a real bug. Lee - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: some missing spin_unlocks
[added alsa-devel to cc:] On Mon, 2005-08-22 at 15:26 -0700, Ted Unangst wrote: I think these are all real bugs. sound/synth/emux/emux_synth.c snd_emux_note_on, line 101 snd_assert will return without unlocking emu-voice_lock (line 89) This one is probably a real bug. Lee - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: some missing spin_unlocks
At Wed, 24 Aug 2005 11:56:58 -0400,
Lee Revell wrote:
[added alsa-devel to cc:]
On Mon, 2005-08-22 at 15:26 -0700, Ted Unangst wrote:
I think these are all real bugs.
sound/synth/emux/emux_synth.c snd_emux_note_on, line 101
snd_assert will return without unlocking emu-voice_lock (line 89)
This one is probably a real bug.
The patch below fixes them (already applied to ALSA CVS tree).
[PATCH] Fix missing spin_unlock
Fixed missing spin_unlock.
Signed-off-by: Takashi Iwai [EMAIL PROTECTED]
--- linux/sound/pci/au88x0/au88x0_pcm.c:1.9 Thu Aug 18 05:43:12 2005
+++ linux/sound/pci/au88x0/au88x0_pcm.c Wed Aug 24 09:57:25 2005
@@ -220,8 +220,10 @@
vortex_adb_allocroute(chip, -1,
params_channels(hw_params),
substream-stream, type);
- if (dma 0)
+ if (dma 0) {
+ spin_unlock_irq(chip-lock);
return dma;
+ }
stream = substream-runtime-private_data = chip-dma_adb[dma];
stream-substream = substream;
/* Setup Buffers. */
--- linux/sound/synth/emux/emux_synth.c:1.11Tue Dec 7 07:36:07 2004
+++ linux/sound/synth/emux/emux_synth.c Wed Aug 24 09:57:25 2005
@@ -98,7 +98,6 @@
vp = emu-ops.get_voice(emu, port);
if (vp == NULL || vp-ch 0)
continue;
- snd_assert(vp-emu != NULL vp-hw != NULL, return);
if (STATE_IS_PLAYING(vp-state))
emu-ops.terminate(vp);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: some missing spin_unlocks
On Tue, 2005-08-23 at 10:30 -0700, David S. Miller wrote: > From: Arjan van de Ven <[EMAIL PROTECTED]> > Date: Tue, 23 Aug 2005 18:54:03 +0200 > > > does it matter? can ANYTHING be spinning on the lock? if not .. can we > > just let the lock go poof and not unlock it... > > I believe socket lookup can, otherwise the code is OK as-is. lookup while the object is in progress of being destroyed sounds really bad though - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: some missing spin_unlocks
From: Arjan van de Ven <[EMAIL PROTECTED]> Subject: Re: some missing spin_unlocks Date: Tue, 23 Aug 2005 19:40:06 +0200 > On Tue, 2005-08-23 at 10:30 -0700, David S. Miller wrote: > > From: Arjan van de Ven <[EMAIL PROTECTED]> > > Date: Tue, 23 Aug 2005 18:54:03 +0200 > > > > > does it matter? can ANYTHING be spinning on the lock? if not .. can we > > > just let the lock go poof and not unlock it... > > > > I believe socket lookup can, otherwise the code is OK as-is. > > lookup while the object is in progress of being destroyed sounds really > bad though This happens all the time with TCP sockets, for example. When we're trying to kill off a socket which is in time wait state, the receive path can find it, grab a reference, and process a packet against it right as we're trying to kill it off. This is completely normal. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: some missing spin_unlocks
> This one needs more care. We can't drop the lock, because > the destroy actions need to be protected by that lock, but > we can't release the lock after rose_destroy_socket() because > the object may not even exist any longer. does it matter? can ANYTHING be spinning on the lock? if not .. can we just let the lock go poof and not unlock it... - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: some missing spin_unlocks
From: Ted Unangst <[EMAIL PROTECTED]>
Date: Mon, 22 Aug 2005 15:26:47 -0700
> net/rose/rose_route.c rose_route_frame, line 998
> returns without unlocking rose_node_list_lock, rose_neigh_list_lock, or
> rose_route_list_lock
I fixed this one with the patch below.
> net/rose/rose_timer.c rose_heartbeat_expiry, line 141
> rose_destroy_socket does not unlock sk as far as i can see
This one needs more care. We can't drop the lock, because
the destroy actions need to be protected by that lock, but
we can't release the lock after rose_destroy_socket() because
the object may not even exist any longer.
The problem there, at the core, is that the timer doesn't
grab a reference to the socket, which would make the solution
to this bug very straight forward.
Someone should work on that :-)
diff-tree 61ef36aa6cf356649863a24a850c2183cb762c61 (from
daf53344fadaa8c47c6b0864e7f34efcbb66e391)
Author: David S. Miller <[EMAIL PROTECTED]>
Date: Tue Aug 23 09:42:38 2005 -0700
[ROSE]: Fix missing unlocks in rose_route_frame()
Noticed by Coverity checker.
Signed-off-by: David S. Miller <[EMAIL PROTECTED]>
diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -994,8 +994,10 @@ int rose_route_frame(struct sk_buff *skb
* 1. The frame isn't for us,
* 2. It isn't "owned" by any existing route.
*/
- if (frametype != ROSE_CALL_REQUEST) /* XXX */
- return 0;
+ if (frametype != ROSE_CALL_REQUEST) { /* XXX */
+ ret = 0;
+ goto out;
+ }
len = (((skb->data[3] >> 4) & 0x0F) + 1) / 2;
len += (((skb->data[3] >> 0) & 0x0F) + 1) / 2;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: some missing spin_unlocks
From: Ted Unangst [EMAIL PROTECTED]
Date: Mon, 22 Aug 2005 15:26:47 -0700
net/rose/rose_route.c rose_route_frame, line 998
returns without unlocking rose_node_list_lock, rose_neigh_list_lock, or
rose_route_list_lock
I fixed this one with the patch below.
net/rose/rose_timer.c rose_heartbeat_expiry, line 141
rose_destroy_socket does not unlock sk as far as i can see
This one needs more care. We can't drop the lock, because
the destroy actions need to be protected by that lock, but
we can't release the lock after rose_destroy_socket() because
the object may not even exist any longer.
The problem there, at the core, is that the timer doesn't
grab a reference to the socket, which would make the solution
to this bug very straight forward.
Someone should work on that :-)
diff-tree 61ef36aa6cf356649863a24a850c2183cb762c61 (from
daf53344fadaa8c47c6b0864e7f34efcbb66e391)
Author: David S. Miller [EMAIL PROTECTED]
Date: Tue Aug 23 09:42:38 2005 -0700
[ROSE]: Fix missing unlocks in rose_route_frame()
Noticed by Coverity checker.
Signed-off-by: David S. Miller [EMAIL PROTECTED]
diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -994,8 +994,10 @@ int rose_route_frame(struct sk_buff *skb
* 1. The frame isn't for us,
* 2. It isn't owned by any existing route.
*/
- if (frametype != ROSE_CALL_REQUEST) /* XXX */
- return 0;
+ if (frametype != ROSE_CALL_REQUEST) { /* XXX */
+ ret = 0;
+ goto out;
+ }
len = (((skb-data[3] 4) 0x0F) + 1) / 2;
len += (((skb-data[3] 0) 0x0F) + 1) / 2;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: some missing spin_unlocks
This one needs more care. We can't drop the lock, because the destroy actions need to be protected by that lock, but we can't release the lock after rose_destroy_socket() because the object may not even exist any longer. does it matter? can ANYTHING be spinning on the lock? if not .. can we just let the lock go poof and not unlock it... - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: some missing spin_unlocks
From: Arjan van de Ven [EMAIL PROTECTED] Subject: Re: some missing spin_unlocks Date: Tue, 23 Aug 2005 19:40:06 +0200 On Tue, 2005-08-23 at 10:30 -0700, David S. Miller wrote: From: Arjan van de Ven [EMAIL PROTECTED] Date: Tue, 23 Aug 2005 18:54:03 +0200 does it matter? can ANYTHING be spinning on the lock? if not .. can we just let the lock go poof and not unlock it... I believe socket lookup can, otherwise the code is OK as-is. lookup while the object is in progress of being destroyed sounds really bad though This happens all the time with TCP sockets, for example. When we're trying to kill off a socket which is in time wait state, the receive path can find it, grab a reference, and process a packet against it right as we're trying to kill it off. This is completely normal. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: some missing spin_unlocks
On Tue, 2005-08-23 at 10:30 -0700, David S. Miller wrote: From: Arjan van de Ven [EMAIL PROTECTED] Date: Tue, 23 Aug 2005 18:54:03 +0200 does it matter? can ANYTHING be spinning on the lock? if not .. can we just let the lock go poof and not unlock it... I believe socket lookup can, otherwise the code is OK as-is. lookup while the object is in progress of being destroyed sounds really bad though - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
