Re: [Alsa-devel] Slab corruptions & Re: 2.6.17-rc1: Oops in sound applications

[Takashi Iwai <tiwai@suse.de>]

At Wed, 5 Apr 2006 11:01:17 +0200,
Jan Niehusmann wrote:
> 
> On Wed, Apr 05, 2006 at 02:28:47AM +0200, Jan Niehusmann wrote:
> > If I now add the patch you suggested, correcting the check in
> > snd_pcm_oss_open_file(), accessing /dev/dsp instead leads to EINVAL.
> > 
> > So I guess git bisect really lead me to this already known bug.
> 
> And another update. Sorry for sending so many small mails, but I want to
> keep you informed to avoid unnecessary duplication of work.
> 
> To make sure I didn't do something stupid like confusing kernel
> versions, I retried with 2.6.17-rc1 and the mentioned patch. It oopses
> again, but the behaviour is different:
> 
> Versions 2.6.16 to commit bf1bbb5a49eec51c30d341606885507b501b37e8 only
> allow a single open of /dev/dsp, and do not oops.
> 
> Commit 3bf75f9b90c981f18f27a0d35a44f488ab68c8ea and later do oops with
> commands as simple as 'yes >/dev/dsp'.
> 
> Commit 3bf75f9b90c981f18f27a0d35a44f488ab68c8ea with the patch to
> snd_pcm_oss_open_file() applied do not oops, but block every access to
> /dev/dsp with EINVAL.
> 
> 2.6.17-rc1 with the patch to snd_pcm_oss_open_file(), again, allows
> opening of /dev/dsp, 'yes >/dev/dsp' does work as expected, but for
> example twinkle (a VoIP application) gives garbled sound. Additionally,
> I am now able to open /dev/dsp a second time (eg. 'yes >/dev/dsp' while
> twinkle uses the sound device), immediately leading to an oops.
> 
> My guess is that this bug is just not triggered in commit
> 3bf75f9b90c981f18f27a0d35a44f488ab68c8ea because, for some other reason,
> /dev/dsp is completely unusable.

Thanks for debugging.
I think I found the culprit.  This bug happens only on a chip with
multiple playback capability, and when you open OSS devices multiple
times.

Try the patch below.  The change in pcm_native.c may be unnecessary,
but it's better so.
If it works, I'll submit the patches with a proper log.


thanks,

Takashi


diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 66b1f08..e9ab455 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -367,7 +367,7 @@ struct snd_pcm_substream {
 	struct snd_pcm_group self_group;	/* fake group for non linked substream (with substream lock inside) */
 	struct snd_pcm_group *group;		/* pointer to current group */
 	/* -- assigned files -- */
-	struct snd_pcm_file *file;
+	void *file;
 	struct file *ffile;
 	void (*pcm_release)(struct snd_pcm_substream *);
 #if defined(CONFIG_SND_PCM_OSS) || defined(CONFIG_SND_PCM_OSS_MODULE)
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index 91114c7..1f8ff7d 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -1757,10 +1757,11 @@ static int snd_pcm_oss_open_file(struct 
 		}
 
 		pcm_oss_file->streams[idx] = substream;
+		substream->file = pcm_oss_file;
 		snd_pcm_oss_init_substream(substream, &setup[idx], minor);
 	}
 	
-	if (! pcm_oss_file->streams[0] && pcm_oss_file->streams[1]) {
+	if (!pcm_oss_file->streams[0] && !pcm_oss_file->streams[1]) {
 		snd_pcm_oss_release_file(pcm_oss_file);
 		return -EINVAL;
 	}
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 964e4c4..0860c5a 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2007,14 +2007,16 @@ static void pcm_release_private(struct s
 void snd_pcm_release_substream(struct snd_pcm_substream *substream)
 {
 	snd_pcm_drop(substream);
-	if (substream->pcm_release)
-		substream->pcm_release(substream);
 	if (substream->hw_opened) {
 		if (substream->ops->hw_free != NULL)
 			substream->ops->hw_free(substream);
 		substream->ops->close(substream);
 		substream->hw_opened = 0;
 	}
+	if (substream->pcm_release) {
+		substream->pcm_release(substream);
+		substream->pcm_release = NULL;
+	}
 	snd_pcm_detach_substream(substream);
 }