sound: deadlock between snd_pcm_oss_write/snd_pcm_oss_mmap
diff mbox

Message ID s5h1t8yknj3.wl-tiwai@suse.de
State New
Headers show

Commit Message

Takashi Iwai Jan. 31, 2016, 11:15 a.m. UTC
On Sat, 30 Jan 2016 21:28:27 +0100,
Dmitry Vyukov wrote:
> 
> Hello,
> 
> I've got the following deadlock report while running syzkaller fuzzer:
> 
> [ INFO: possible circular locking dependency detected ]
> 4.5.0-rc1+ #305 Not tainted
> -------------------------------------------------------
> syz-executor/14254 is trying to acquire lock:
>  (&runtime->oss.params_lock){+.+.+.}, at: [<ffffffff8528a504>]
> snd_pcm_oss_change_params+0xd4/0x3540 sound/core/oss/pcm_oss.c:852
> 
> but task is already holding lock:
>  (&mm->mmap_sem){++++++}, at: [<ffffffff816b267c>] vm_mmap_pgoff
> 
> which lock already depends on the new lock.
> 
> 
> the existing dependency chain (in reverse order) is:
> 
> -> #1 (&mm->mmap_sem){++++++}:
>        [<ffffffff8145b9dc>] lock_acquire+0x1dc/0x430
> kernel/locking/lockdep.c:3587
>        [<ffffffff816def51>] __might_fault+0x141/0x1d0 mm/memory.c:3802
>        [<     inline     >] copy_from_user ./arch/x86/include/asm/uaccess.h:714
>        [<     inline     >] snd_pcm_oss_write1 sound/core/oss/pcm_oss.c:1376
>        [<ffffffff852940f0>] snd_pcm_oss_write+0x250/0x700
> sound/core/oss/pcm_oss.c:2694
>        [<ffffffff817b90b3>] __vfs_write+0x113/0x480 fs/read_write.c:528
>        [<ffffffff817bab47>] vfs_write+0x167/0x4a0 fs/read_write.c:577
>        [<     inline     >] SYSC_write fs/read_write.c:624
>        [<ffffffff817bde31>] SyS_write+0x111/0x220 fs/read_write.c:616
>        [<ffffffff866531b6>] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185
> 
> -> #0 (&runtime->oss.params_lock){+.+.+.}:
>        [<     inline     >] check_prev_add kernel/locking/lockdep.c:1855
>        [<     inline     >] check_prevs_add kernel/locking/lockdep.c:1960
>        [<     inline     >] validate_chain kernel/locking/lockdep.c:2146
>        [<ffffffff8145807b>] __lock_acquire+0x31eb/0x4700
> kernel/locking/lockdep.c:3208
>        [<ffffffff8145b9dc>] lock_acquire+0x1dc/0x430
> kernel/locking/lockdep.c:3587
>        [<     inline     >] __mutex_lock_common kernel/locking/mutex.c:518
>        [<ffffffff8664891c>] mutex_lock_interruptible_nested+0xbc/0xbe0
> kernel/locking/mutex.c:647
>        [<ffffffff8528a504>] snd_pcm_oss_change_params+0xd4/0x3540
> sound/core/oss/pcm_oss.c:852
>        [<ffffffff8528f01d>] snd_pcm_oss_mmap+0x3dd/0x4c0
> sound/core/oss/pcm_oss.c:2807
>        [<ffffffff81705747>] mmap_region+0x897/0x1010 mm/mmap.c:1624
>        [<ffffffff81706614>] do_mmap+0x754/0x990 mm/mmap.c:1403
>        [<     inline     >] do_mmap_pgoff include/linux/mm.h:1982
>        [<ffffffff816b26af>] vm_mmap_pgoff+0x15f/0x1b0 mm/util.c:328
>        [<     inline     >] SYSC_mmap_pgoff mm/mmap.c:1453
>        [<ffffffff816ff85a>] SyS_mmap_pgoff+0x34a/0x580 mm/mmap.c:1411
>        [<     inline     >] SYSC_mmap arch/x86/kernel/sys_x86_64.c:95
>        [<ffffffff811aeeb6>] SyS_mmap+0x16/0x20 arch/x86/kernel/sys_x86_64.c:86
>        [<ffffffff866531b6>] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185
> 
> other info that might help us debug this:
> 
>  Possible unsafe locking scenario:
> 
>        CPU0                    CPU1
>        ----                    ----
>   lock(&mm->mmap_sem);
>                                lock(&runtime->oss.params_lock);
>                                lock(&mm->mmap_sem);
>   lock(&runtime->oss.params_lock);
> 
>  *** DEADLOCK ***
> 
> 1 lock held by syz-executor/14254:
>  #0:  (&mm->mmap_sem){++++++}, at: [<ffffffff816b267c>]
> vm_mmap_pgoff+0x12c/0x1b0 mm/util.c:327
> 
> stack backtrace:
> CPU: 2 PID: 14254 Comm: syz-executor Not tainted 4.5.0-rc1+ #305
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>  00000000ffffffff ffff88003214f780 ffffffff82be11ad ffffffff8959ac60
>  ffffffff8959ac60 ffffffff89573f60 ffff88003214f7d0 ffffffff814512a8
>  ffff8800333cdf00 ffff8800333ce742 0000000000000000 ffff8800333ce720
> Call Trace:
>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>  [<ffffffff82be11ad>] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
>  [<ffffffff814512a8>] print_circular_bug+0x288/0x340
> kernel/locking/lockdep.c:1228
>  [<     inline     >] check_prev_add kernel/locking/lockdep.c:1855
>  [<     inline     >] check_prevs_add kernel/locking/lockdep.c:1960
>  [<     inline     >] validate_chain kernel/locking/lockdep.c:2146
>  [<ffffffff8145807b>] __lock_acquire+0x31eb/0x4700 kernel/locking/lockdep.c:3208
>  [<ffffffff8145b9dc>] lock_acquire+0x1dc/0x430 kernel/locking/lockdep.c:3587
>  [<     inline     >] __mutex_lock_common kernel/locking/mutex.c:518
>  [<ffffffff8664891c>] mutex_lock_interruptible_nested+0xbc/0xbe0
> kernel/locking/mutex.c:647
>  [<ffffffff8528a504>] snd_pcm_oss_change_params+0xd4/0x3540
> sound/core/oss/pcm_oss.c:852
>  [<ffffffff8528f01d>] snd_pcm_oss_mmap+0x3dd/0x4c0 sound/core/oss/pcm_oss.c:2807
>  [<ffffffff81705747>] mmap_region+0x897/0x1010 mm/mmap.c:1624
>  [<ffffffff81706614>] do_mmap+0x754/0x990 mm/mmap.c:1403
>  [<     inline     >] do_mmap_pgoff include/linux/mm.h:1982
>  [<ffffffff816b26af>] vm_mmap_pgoff+0x15f/0x1b0 mm/util.c:328
>  [<     inline     >] SYSC_mmap_pgoff mm/mmap.c:1453
>  [<ffffffff816ff85a>] SyS_mmap_pgoff+0x34a/0x580 mm/mmap.c:1411
>  [<     inline     >] SYSC_mmap arch/x86/kernel/sys_x86_64.c:95
>  [<ffffffff811aeeb6>] SyS_mmap+0x16/0x20 arch/x86/kernel/sys_x86_64.c:86
> 
> 
> On commit 26cd83670f2f5a3d5b5514a1f7d96567cdb9558b.

It's infamous mmap_sem lock in user copy paths.  The quick workaround
is attached below.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: pcm: Fix potential deadlock in OSS emulation

There are potential deadlocks in PCM OSS emulation code while
accessing read/write and mmap concurrently.  This comes from the
infamous mmap_sem usage in copy_from/to_user().  Namely,

   snd_pcm_oss_write() ->
     &runtime->oss.params_lock ->
        copy_to_user() ->
          &mm->mmap_sem
  mmap() ->
    &mm->mmap_sem ->
      snd_pcm_oss_mmap() ->
        &runtime->oss.params_lock

Since we can't avoid taking params_lock from mmap code path, use
trylock variant and aborts with -EAGAIN as a workaround of this AB/BA
deadlock.

BugLink: http://lkml.kernel.org/r/CACT4Y+bVrBKDG0G2_AcUgUQa+X91VKTeS4v+wN7BSHwHtqn3kQ@mail.gmail.com
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/oss/pcm_oss.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

Patch
diff mbox

diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index 0e73d03b30e3..ebc9fdfe64df 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -835,7 +835,8 @@  static int choose_rate(struct snd_pcm_substream *substream,
 	return snd_pcm_hw_param_near(substream, params, SNDRV_PCM_HW_PARAM_RATE, best_rate, NULL);
 }
 
-static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream)
+static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream,
+				     bool trylock)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_pcm_hw_params *params, *sparams;
@@ -849,7 +850,10 @@  static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream)
 	struct snd_mask sformat_mask;
 	struct snd_mask mask;
 
-	if (mutex_lock_interruptible(&runtime->oss.params_lock))
+	if (trylock) {
+		if (!(mutex_trylock(&runtime->oss.params_lock)))
+			return -EAGAIN;
+	} else if (mutex_lock_interruptible(&runtime->oss.params_lock))
 		return -EINTR;
 	sw_params = kzalloc(sizeof(*sw_params), GFP_KERNEL);
 	params = kmalloc(sizeof(*params), GFP_KERNEL);
@@ -1092,7 +1096,7 @@  static int snd_pcm_oss_get_active_substream(struct snd_pcm_oss_file *pcm_oss_fil
 		if (asubstream == NULL)
 			asubstream = substream;
 		if (substream->runtime->oss.params) {
-			err = snd_pcm_oss_change_params(substream);
+			err = snd_pcm_oss_change_params(substream, false);
 			if (err < 0)
 				return err;
 		}
@@ -1132,7 +1136,7 @@  static int snd_pcm_oss_make_ready(struct snd_pcm_substream *substream)
 		return 0;
 	runtime = substream->runtime;
 	if (runtime->oss.params) {
-		err = snd_pcm_oss_change_params(substream);
+		err = snd_pcm_oss_change_params(substream, false);
 		if (err < 0)
 			return err;
 	}
@@ -2163,7 +2167,7 @@  static int snd_pcm_oss_get_space(struct snd_pcm_oss_file *pcm_oss_file, int stre
 	runtime = substream->runtime;
 
 	if (runtime->oss.params &&
-	    (err = snd_pcm_oss_change_params(substream)) < 0)
+	    (err = snd_pcm_oss_change_params(substream, false)) < 0)
 		return err;
 
 	info.fragsize = runtime->oss.period_bytes;
@@ -2804,7 +2808,12 @@  static int snd_pcm_oss_mmap(struct file *file, struct vm_area_struct *area)
 		return -EIO;
 	
 	if (runtime->oss.params) {
-		if ((err = snd_pcm_oss_change_params(substream)) < 0)
+		/* use mutex_trylock() for params_lock for avoiding a deadlock
+		 * between mmap_sem and params_lock taken by
+		 * copy_from/to_user() in snd_pcm_oss_write/read()
+		 */
+		err = snd_pcm_oss_change_params(substream, true);
+		if (err < 0)
 			return err;
 	}
 #ifdef CONFIG_SND_PCM_OSS_PLUGINS