diff mbox

KASAN: use-after-free Read in snd_timer_user_info_compat

Message ID s5h7evegt84.wl-tiwai@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Takashi Iwai Oct. 29, 2017, 10:10 a.m. UTC
On Fri, 27 Oct 2017 10:34:01 +0200,
syzbot wrote:
> 
> Hello,
> 
> syzkaller hit the following crash on
> 33d930e59a98fa10a0db9f56c7fa2f21a4aef9b9
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
> compiler: gcc (GCC) 7.1.1 20170620
> .config is attached
> Raw console output is attached.
> 
> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
> for information about syzkaller reproducers
> 
> 
> BUG: KASAN: use-after-free in
> snd_timer_user_info_compat.isra.6+0x46c/0x480
> sound/core/timer_compat.c:71
> Read of size 8 at addr ffff8801d0dd9d00 by task syz-executor0/3342

OK, this is a real bug, and obviously a forgotten code path from the
previous fixes.  The patch below should address it, but I checked only
compile-tested, so far.  Let me know if it really works.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: timer: Add missing mutex lock for compat ioctls

The races among ioctl and other operations were protected by the
commit af368027a49a ("ALSA: timer: Fix race among timer ioctls") and
later fixes, but one code path was forgotten in the scenario: the
32bit compat ioctl.  As syzkaller recently spotted, a very similar
use-after-free may happen with the combination of compat ioctls.

The fix is simply to apply the same ioctl_lock to the compat_ioctl
callback, too.

Fixes: af368027a49a ("ALSA: timer: Fix race among timer ioctls")
Reference: http://lkml.kernel.org/r/089e082686ac9b482e055c832617@google.com
Reported-by: syzbot <bot+e5f3c9783e7048a74233054febbe9f1bdf54b6da@syzkaller.appspotmail.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/timer_compat.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

Dmitry Vyukov Nov. 21, 2017, 3:30 p.m. UTC | #1
On Sun, Oct 29, 2017 at 11:10 AM, Takashi Iwai <tiwai@suse.de> wrote:
> On Fri, 27 Oct 2017 10:34:01 +0200,
> syzbot wrote:
>>
>> Hello,
>>
>> syzkaller hit the following crash on
>> 33d930e59a98fa10a0db9f56c7fa2f21a4aef9b9
>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
>> compiler: gcc (GCC) 7.1.1 20170620
>> .config is attached
>> Raw console output is attached.
>>
>> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
>> for information about syzkaller reproducers
>>
>>
>> BUG: KASAN: use-after-free in
>> snd_timer_user_info_compat.isra.6+0x46c/0x480
>> sound/core/timer_compat.c:71
>> Read of size 8 at addr ffff8801d0dd9d00 by task syz-executor0/3342
>
> OK, this is a real bug, and obviously a forgotten code path from the
> previous fixes.  The patch below should address it, but I checked only
> compile-tested, so far.  Let me know if it really works.
>
>
> thanks,
>
> Takashi
>
> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] ALSA: timer: Add missing mutex lock for compat ioctls
>
> The races among ioctl and other operations were protected by the
> commit af368027a49a ("ALSA: timer: Fix race among timer ioctls") and
> later fixes, but one code path was forgotten in the scenario: the
> 32bit compat ioctl.  As syzkaller recently spotted, a very similar
> use-after-free may happen with the combination of compat ioctls.
>
> The fix is simply to apply the same ioctl_lock to the compat_ioctl
> callback, too.
>
> Fixes: af368027a49a ("ALSA: timer: Fix race among timer ioctls")
> Reference: http://lkml.kernel.org/r/089e082686ac9b482e055c832617@google.com
> Reported-by: syzbot <bot+e5f3c9783e7048a74233054febbe9f1bdf54b6da@syzkaller.appspotmail.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/core/timer_compat.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/sound/core/timer_compat.c b/sound/core/timer_compat.c
> index 6a437eb66115..59127b6ef39e 100644
> --- a/sound/core/timer_compat.c
> +++ b/sound/core/timer_compat.c
> @@ -133,7 +133,8 @@ enum {
>  #endif /* CONFIG_X86_X32 */
>  };
>
> -static long snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd, unsigned long arg)
> +static long __snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd,
> +                                         unsigned long arg)
>  {
>         void __user *argp = compat_ptr(arg);
>
> @@ -153,7 +154,7 @@ static long snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd, uns
>         case SNDRV_TIMER_IOCTL_PAUSE:
>         case SNDRV_TIMER_IOCTL_PAUSE_OLD:
>         case SNDRV_TIMER_IOCTL_NEXT_DEVICE:
> -               return snd_timer_user_ioctl(file, cmd, (unsigned long)argp);
> +               return __snd_timer_user_ioctl(file, cmd, (unsigned long)argp);
>         case SNDRV_TIMER_IOCTL_GPARAMS32:
>                 return snd_timer_user_gparams_compat(file, argp);
>         case SNDRV_TIMER_IOCTL_INFO32:
> @@ -167,3 +168,15 @@ static long snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd, uns
>         }
>         return -ENOIOCTLCMD;
>  }
> +
> +static long snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd,
> +                                       unsigned long arg)
> +{
> +       struct snd_timer_user *tu = file->private_data;
> +       long ret;
> +
> +       mutex_lock(&tu->ioctl_lock);
> +       ret = __snd_timer_user_ioctl_compat(file, cmd, arg);
> +       mutex_unlock(&tu->ioctl_lock);
> +       return ret;
> +}


#syz fix: ALSA: timer: Add missing mutex lock for compat ioctls
diff mbox

Patch

diff --git a/sound/core/timer_compat.c b/sound/core/timer_compat.c
index 6a437eb66115..59127b6ef39e 100644
--- a/sound/core/timer_compat.c
+++ b/sound/core/timer_compat.c
@@ -133,7 +133,8 @@  enum {
 #endif /* CONFIG_X86_X32 */
 };
 
-static long snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd, unsigned long arg)
+static long __snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd,
+					  unsigned long arg)
 {
 	void __user *argp = compat_ptr(arg);
 
@@ -153,7 +154,7 @@  static long snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd, uns
 	case SNDRV_TIMER_IOCTL_PAUSE:
 	case SNDRV_TIMER_IOCTL_PAUSE_OLD:
 	case SNDRV_TIMER_IOCTL_NEXT_DEVICE:
-		return snd_timer_user_ioctl(file, cmd, (unsigned long)argp);
+		return __snd_timer_user_ioctl(file, cmd, (unsigned long)argp);
 	case SNDRV_TIMER_IOCTL_GPARAMS32:
 		return snd_timer_user_gparams_compat(file, argp);
 	case SNDRV_TIMER_IOCTL_INFO32:
@@ -167,3 +168,15 @@  static long snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd, uns
 	}
 	return -ENOIOCTLCMD;
 }
+
+static long snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd,
+					unsigned long arg)
+{
+	struct snd_timer_user *tu = file->private_data;
+	long ret;
+
+	mutex_lock(&tu->ioctl_lock);
+	ret = __snd_timer_user_ioctl_compat(file, cmd, arg);
+	mutex_unlock(&tu->ioctl_lock);
+	return ret;
+}