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

Takashi Iwai
2006-04-05T11:14:57Z

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);
}