Permalink
Browse files

ALSA: timer: Fix race among timer ioctls

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.

Change-Id: Iaa21b00f62e02cc58e346a29846e0fce6536e860
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Tested-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
  • Loading branch information...
tiwai authored and andi34 committed Jan 13, 2016
1 parent 254a325 commit 7313ea3bb6726ddd07793be7b64706272169e139
Showing with 19 additions and 13 deletions.
  1. +19 −13 sound/core/timer.c
View
@@ -72,7 +72,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 */
@@ -1252,7 +1252,7 @@ static int snd_timer_user_open(struct inode *inode, struct file *file)
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),
@@ -1272,8 +1272,10 @@ static int snd_timer_user_release(struct inode *inode, struct file *file)
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);
@@ -1511,7 +1513,6 @@ static int snd_timer_user_tselect(struct file *file,
int err = 0;
tu = file->private_data;
mutex_lock(&tu->tread_sem);
if (tu->timeri) {
snd_timer_close(tu->timeri);
tu->timeri = NULL;
@@ -1555,7 +1556,6 @@ static int snd_timer_user_tselect(struct file *file,
}
__err:
mutex_unlock(&tu->tread_sem);
return err;
}
@@ -1768,7 +1768,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;
@@ -1785,17 +1785,11 @@ static long snd_timer_user_ioctl(struct file *file, unsigned int cmd,
{
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:
@@ -1828,6 +1822,18 @@ static long snd_timer_user_ioctl(struct file *file, unsigned int cmd,
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;

0 comments on commit 7313ea3

Please sign in to comment.