Re: Badness in __mutex_unlock_slowpath

From: Ingo Molnar
Date: Sun Jan 08 2006 - 04:03:29 EST



* Arjan van de Ven <arjan@xxxxxxxxxxxxx> wrote:

> this looks like a really evil alsa bug:
>
> (pre mutex code below)

> up(&file->f_dentry->d_inode->i_sem);
> result = snd_pcm_oss_write1(substream, buf, count);
> down(&file->f_dentry->d_inode->i_sem);

> this is a .write method of a driver, which doesn't run with i_sem held
> at all. Best guess I have is that this code has up() and down()
> confused and switched...

well snd_pcm_oss_read1() is not using the mutex at all - nor any other
functions here. So the patch below removes the i_mutex use. _If_ some
synchronization is needed it would be needed in the read1 case too: it
is destructive to a sound stream when it is 'read' and when it is
'written' just as much.

the bug could cause inode corruption on the VFS level: one thread
unlocks an inode it doesnt own - this could surprise another thread
holding that mutex and could allow a third thread to lock it and thus
two threads would be in a critical section - bad.

Ingo

--
remove bogus i_mutex use from sound/core/oss/pcm_oss.c.

Signed-off-by: Ingo Molnar <mingo@xxxxxxx>

----

sound/core/oss/pcm_oss.c | 2 --
1 files changed, 2 deletions(-)

Index: linux/sound/core/oss/pcm_oss.c
===================================================================
--- linux.orig/sound/core/oss/pcm_oss.c
+++ linux/sound/core/oss/pcm_oss.c
@@ -2135,9 +2135,7 @@ static ssize_t snd_pcm_oss_write(struct
substream = pcm_oss_file->streams[SNDRV_PCM_STREAM_PLAYBACK];
if (substream == NULL)
return -ENXIO;
- mutex_unlock(&file->f_dentry->d_inode->i_mutex);
result = snd_pcm_oss_write1(substream, buf, count);
- mutex_lock(&file->f_dentry->d_inode->i_mutex);
#ifdef OSS_DEBUG
printk("pcm_oss: write %li bytes (wrote %li bytes)\n", (long)count, (long)result);
#endif
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/