Re: Badness in __mutex_unlock_slowpath

[Ingo Molnar <mingo@elte.hu>]

* Arjan van de Ven <arjan@infradead.org> 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@elte.hu>

----

 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