| From af368027a49a751d6ff4ee9e3f9961f35bb4fede Mon Sep 17 00:00:00 2001 |
| From: Takashi Iwai <tiwai@suse.de> |
| Date: Wed, 13 Jan 2016 17:48:01 +0100 |
| Subject: ALSA: timer: Fix race among timer ioctls |
| |
| From: Takashi Iwai <tiwai@suse.de> |
| |
| commit af368027a49a751d6ff4ee9e3f9961f35bb4fede upstream. |
| |
| ALSA timer ioctls have an open race and this may lead to a |
| use-after-free of timer instance object. A simplistic fix is to make |
| each ioctl exclusive. We have already tread_sem for controlling the |
| tread, and extend this as a global mutex to be applied to each ioctl. |
| |
| The downside is, of course, the worse concurrency. But these ioctls |
| aren't to be parallel accessible, in anyway, so it should be fine to |
| serialize there. |
| |
| Reported-by: Dmitry Vyukov <dvyukov@google.com> |
| Tested-by: Dmitry Vyukov <dvyukov@google.com> |
| Signed-off-by: Takashi Iwai <tiwai@suse.de> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| sound/core/timer.c | 32 +++++++++++++++++++------------- |
| 1 file changed, 19 insertions(+), 13 deletions(-) |
| |
| --- a/sound/core/timer.c |
| +++ b/sound/core/timer.c |
| @@ -73,7 +73,7 @@ struct snd_timer_user { |
| struct timespec tstamp; /* trigger tstamp */ |
| wait_queue_head_t qchange_sleep; |
| struct fasync_struct *fasync; |
| - struct mutex tread_sem; |
| + struct mutex ioctl_lock; |
| }; |
| |
| /* list of timers */ |
| @@ -1263,7 +1263,7 @@ static int snd_timer_user_open(struct in |
| return -ENOMEM; |
| spin_lock_init(&tu->qlock); |
| init_waitqueue_head(&tu->qchange_sleep); |
| - mutex_init(&tu->tread_sem); |
| + mutex_init(&tu->ioctl_lock); |
| tu->ticks = 1; |
| tu->queue_size = 128; |
| tu->queue = kmalloc(tu->queue_size * sizeof(struct snd_timer_read), |
| @@ -1283,8 +1283,10 @@ static int snd_timer_user_release(struct |
| if (file->private_data) { |
| tu = file->private_data; |
| file->private_data = NULL; |
| + mutex_lock(&tu->ioctl_lock); |
| if (tu->timeri) |
| snd_timer_close(tu->timeri); |
| + mutex_unlock(&tu->ioctl_lock); |
| kfree(tu->queue); |
| kfree(tu->tqueue); |
| kfree(tu); |
| @@ -1522,7 +1524,6 @@ static int snd_timer_user_tselect(struct |
| int err = 0; |
| |
| tu = file->private_data; |
| - mutex_lock(&tu->tread_sem); |
| if (tu->timeri) { |
| snd_timer_close(tu->timeri); |
| tu->timeri = NULL; |
| @@ -1566,7 +1567,6 @@ static int snd_timer_user_tselect(struct |
| } |
| |
| __err: |
| - mutex_unlock(&tu->tread_sem); |
| return err; |
| } |
| |
| @@ -1779,7 +1779,7 @@ enum { |
| SNDRV_TIMER_IOCTL_PAUSE_OLD = _IO('T', 0x23), |
| }; |
| |
| -static long snd_timer_user_ioctl(struct file *file, unsigned int cmd, |
| +static long __snd_timer_user_ioctl(struct file *file, unsigned int cmd, |
| unsigned long arg) |
| { |
| struct snd_timer_user *tu; |
| @@ -1796,17 +1796,11 @@ static long snd_timer_user_ioctl(struct |
| { |
| int xarg; |
| |
| - mutex_lock(&tu->tread_sem); |
| - if (tu->timeri) { /* too late */ |
| - mutex_unlock(&tu->tread_sem); |
| + if (tu->timeri) /* too late */ |
| return -EBUSY; |
| - } |
| - if (get_user(xarg, p)) { |
| - mutex_unlock(&tu->tread_sem); |
| + if (get_user(xarg, p)) |
| return -EFAULT; |
| - } |
| tu->tread = xarg ? 1 : 0; |
| - mutex_unlock(&tu->tread_sem); |
| return 0; |
| } |
| case SNDRV_TIMER_IOCTL_GINFO: |
| @@ -1839,6 +1833,18 @@ static long snd_timer_user_ioctl(struct |
| return -ENOTTY; |
| } |
| |
| +static long snd_timer_user_ioctl(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(file, cmd, arg); |
| + mutex_unlock(&tu->ioctl_lock); |
| + return ret; |
| +} |
| + |
| static int snd_timer_user_fasync(int fd, struct file * file, int on) |
| { |
| struct snd_timer_user *tu; |