Messages in this thread Patch in this message | | | Date | Wed, 05 Apr 2006 13:14:54 +0200 | From | Takashi Iwai <> | Subject | Re: [Alsa-devel] Slab corruptions & Re: 2.6.17-rc1: Oops in sound applications |
| |
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); } - 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/
| |