Skip to content

Commit

Permalink
sound: use-after-free in snd_timer_interrupt
Browse files Browse the repository at this point in the history 
On Wed, 13 Jan 2016 16:00:09 +0100,
Dmitry Vyukov wrote:
>
> Hello,
>
> The following program triggers use-after-free in snd_timer_interrupt:
>
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include <unistd.h>
> #include <sys/syscall.h>
> #include <string.h>
> #include <stdint.h>
> #include <pthread.h>
>
> long r[84];
>
> void *thr(void *arg)
> {
>         switch ((long)arg) {
>         case 0:
>                 r[0] = syscall(SYS_mmap, 0x20000000ul, 0xe000ul,
> 0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul);
>                 break;
>         case 1:
>                 memcpy((void*)0x20000990,
> "\x2f\x64\x65\x76\x2f\x73\x6e\x64\x2f\x74\x69\x6d\x65\x72", 14);
>                 r[2] = syscall(SYS_open, 0x20000990ul, 0x1ul, 0x0ul, 0, 0, 0);
>                 break;
>         case 2:
>                 *(uint32_t*)0x20000000 = (uint32_t)0x1;
>                 *(uint32_t*)0x20000004 = (uint32_t)0x7;
>                 *(uint32_t*)0x20000008 = (uint32_t)0x3;
>                 *(uint32_t*)0x2000000c = (uint32_t)0x0;
>                 *(uint32_t*)0x20000010 = (uint32_t)0x0;
>                 *(uint8_t*)0x20000014 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000015 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000016 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000017 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000018 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000019 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000001a = (uint8_t)0x0;
>                 *(uint8_t*)0x2000001b = (uint8_t)0x0;
>                 *(uint8_t*)0x2000001c = (uint8_t)0x0;
>                 *(uint8_t*)0x2000001d = (uint8_t)0x0;
>                 *(uint8_t*)0x2000001e = (uint8_t)0x0;
>                 *(uint8_t*)0x2000001f = (uint8_t)0x0;
>                 *(uint8_t*)0x20000020 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000021 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000022 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000023 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000024 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000025 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000026 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000027 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000028 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000029 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000002a = (uint8_t)0x0;
>                 *(uint8_t*)0x2000002b = (uint8_t)0x0;
>                 *(uint8_t*)0x2000002c = (uint8_t)0x0;
>                 *(uint8_t*)0x2000002d = (uint8_t)0x0;
>                 *(uint8_t*)0x2000002e = (uint8_t)0x0;
>                 *(uint8_t*)0x2000002f = (uint8_t)0x0;
>                 *(uint8_t*)0x20000030 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000031 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000032 = (uint8_t)0x0;
>                 *(uint8_t*)0x20000033 = (uint8_t)0x0;
>                 r[40] = syscall(SYS_ioctl, r[2], 0x40345410ul,
> 0x20000000ul, 0, 0, 0);
>                 break;
>         case 3:
>                 r[41] = syscall(SYS_ioctl, r[2], 0x54a2ul, 0, 0, 0, 0);
>                 break;
>         case 4:
>                 r[42] = syscall(SYS_mmap, 0x2000e000ul, 0x1000ul,
> 0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul);
>                 break;
>         case 5:
>                 *(uint32_t*)0x2000efcc = (uint32_t)0x7;
>                 *(uint32_t*)0x2000efd0 = (uint32_t)0x9;
>                 *(uint32_t*)0x2000efd4 = (uint32_t)0x4513;
>                 *(uint32_t*)0x2000efd8 = (uint32_t)0x9;
>                 *(uint32_t*)0x2000efdc = (uint32_t)0x5;
>                 *(uint8_t*)0x2000efe0 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efe1 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efe2 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efe3 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efe4 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efe5 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efe6 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efe7 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efe8 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efe9 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efea = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efeb = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efec = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efed = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efee = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efef = (uint8_t)0x0;
>                 *(uint8_t*)0x2000eff0 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000eff1 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000eff2 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000eff3 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000eff4 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000eff5 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000eff6 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000eff7 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000eff8 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000eff9 = (uint8_t)0x0;
>                 *(uint8_t*)0x2000effa = (uint8_t)0x0;
>                 *(uint8_t*)0x2000effb = (uint8_t)0x0;
>                 *(uint8_t*)0x2000effc = (uint8_t)0x0;
>                 *(uint8_t*)0x2000effd = (uint8_t)0x0;
>                 *(uint8_t*)0x2000effe = (uint8_t)0x0;
>                 *(uint8_t*)0x2000efff = (uint8_t)0x0;
>                 r[80] = syscall(SYS_ioctl, r[2], 0x40345410ul,
> 0x2000efccul, 0, 0, 0);
>                 break;
>         case 6:
>                 r[81] = syscall(SYS_ioctl, r[2], 0x54a0ul, 0, 0, 0, 0);
>                 break;
>         case 7:
>                 r[82] = syscall(SYS_mmap, 0x2000f000ul, 0x1000ul,
> 0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul);
>                 break;
>         case 8:
>                 r[83] = syscall(SYS_ioctl, r[2], 0x80e85411ul,
> 0x2000ffd4ul, 0, 0, 0);
>                 break;
>         }
>         return 0;
> }
>
> int main()
> {
>         long i;
>         pthread_t th[9];
>
>         memset(r, -1, sizeof(r));
>         for (i = 0; i < 9; i++) {
>                 pthread_create(&th[i], 0, thr, (void*)i);
>                 usleep(10000);
>         }
>         for (i = 0; i < 9; i++) {
>                 pthread_create(&th[i], 0, thr, (void*)i);
>                 if (i%2==0)
>                         usleep(10000);
>         }
>         usleep(100000);
>         return 0;
> }
>
>
> ==================================================================
> BUG: KASAN: use-after-free in snd_timer_interrupt+0xb11/0xbf0 at addr
> ffff88002fd230d8
> Read of size 8 by task syz-executor/8301
> =============================================================================
> BUG kmalloc-256 (Not tainted): kasan: bad access detected
> -----------------------------------------------------------------------------
>
> INFO: Allocated in snd_timer_instance_new+0x52/0x3a0 age=2 cpu=1 pid=8283
> [<      none      >] ___slab_alloc+0x486/0x4e0 mm/slub.c:2468
> [<      none      >] __slab_alloc+0x66/0xc0 mm/slub.c:2497
> [<     inline     >] slab_alloc_node mm/slub.c:2560
> [<     inline     >] slab_alloc mm/slub.c:2602
> [<      none      >] kmem_cache_alloc_trace+0x284/0x310 mm/slub.c:2619
> [<     inline     >] kmalloc include/linux/slab.h:458
> [<     inline     >] kzalloc include/linux/slab.h:602
> [<      none      >] snd_timer_instance_new+0x52/0x3a0 sound/core/timer.c:105
> [<      none      >] snd_timer_open+0x522/0xc90 sound/core/timer.c:286
> [<     inline     >] snd_timer_user_tselect sound/core/timer.c:1527
> [<      none      >] snd_timer_user_ioctl+0x89f/0x2540 sound/core/timer.c:1809
> [<     inline     >] vfs_ioctl fs/ioctl.c:43
> [<      none      >] do_vfs_ioctl+0x18c/0xfa0 fs/ioctl.c:674
> [<     inline     >] SYSC_ioctl fs/ioctl.c:689
> [<      none      >] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:680
> [<      none      >] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185
>
> INFO: Freed in snd_timer_close+0x354/0x5f0 age=1 cpu=1 pid=8283
> [<      none      >] __slab_free+0x1fc/0x320 mm/slub.c:2678
> [<     inline     >] slab_free mm/slub.c:2833
> [<      none      >] kfree+0x2a8/0x2d0 mm/slub.c:3662
> [<      none      >] snd_timer_close+0x354/0x5f0 sound/core/timer.c:364
> [<     inline     >] snd_timer_user_tselect sound/core/timer.c:1517
> [<      none      >] snd_timer_user_ioctl+0x784/0x2540 sound/core/timer.c:1809
> [<     inline     >] vfs_ioctl fs/ioctl.c:43
> [<      none      >] do_vfs_ioctl+0x18c/0xfa0 fs/ioctl.c:674
> [<     inline     >] SYSC_ioctl fs/ioctl.c:689
> [<      none      >] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:680
> [<      none      >] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185
>
> INFO: Slab 0xffffea0000bf4800 objects=22 used=2 fp=0xffff88002fd23058
> flags=0x1fffc0000004080
> INFO: Object 0xffff88002fd23058 @offset=12376 fp=0xffff88002fd227d0
> CPU: 2 PID: 8301 Comm: syz-executor Tainted: G    B           4.4.0+ torvalds#240
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>  00000000ffffffff ffff88006d607b08 ffffffff82926eed ffff88003e807000
>  ffff88002fd23058 ffff88002fd20000 ffff88006d607b38 ffffffff81740ca4
>  ffff88003e807000 ffffea0000bf4800 ffff88002fd23058 ffff88002fd230d8
>
> Call Trace:
>  [<ffffffff8174a1fe>] __asan_report_load8_noabort+0x3e/0x40
> mm/kasan/report.c:295
>  [<ffffffff84ebe841>] snd_timer_interrupt+0xb11/0xbf0 sound/core/timer.c:680
>  [<ffffffff84ebe9dd>] snd_timer_s_function+0xbd/0x130 sound/core/timer.c:963
>  [<ffffffff814b8c56>] call_timer_fn+0x176/0x550 kernel/time/timer.c:1178
>  [<     inline     >] __run_timers kernel/time/timer.c:1254
>  [<ffffffff814ba175>] run_timer_softirq+0x5c5/0x9f0 kernel/time/timer.c:1437
>  [<ffffffff813606a8>] __do_softirq+0x268/0x920 kernel/softirq.c:273
>  [<     inline     >] invoke_softirq kernel/softirq.c:350
>  [<ffffffff813610ef>] irq_exit+0x18f/0x1d0 kernel/softirq.c:391
>  [<     inline     >] exiting_irq ./arch/x86/include/asm/apic.h:659
>  [<ffffffff8125157b>] smp_apic_timer_interrupt+0x7b/0xa0
> arch/x86/kernel/apic/apic.c:932
>  [<ffffffff86273dec>] apic_timer_interrupt+0x8c/0xa0
> arch/x86/entry/entry_64.S:520
>  <EOI>  [<ffffffff813ac59d>] ? alloc_pid+0x5d/0xc90 kernel/pid.c:306
>  [<     inline     >] slab_alloc_node mm/slub.c:2560
>  [<     inline     >] slab_alloc mm/slub.c:2602
>  [<ffffffff817446e1>] kmem_cache_alloc+0x261/0x2e0 mm/slub.c:2607
>  [<ffffffff813ac59d>] alloc_pid+0x5d/0xc90 kernel/pid.c:306
>  [<ffffffff8134ca2e>] copy_process.part.35+0x374e/0x5770 kernel/fork.c:1463
>  [<     inline     >] copy_process kernel/fork.c:1275
>  [<ffffffff8134ed7c>] _do_fork+0x1bc/0xcb0 kernel/fork.c:1724
>  [<     inline     >] SYSC_clone kernel/fork.c:1833
>  [<ffffffff8134f947>] SyS_clone+0x37/0x50 kernel/fork.c:1827
>  [<ffffffff86272ff6>] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185
> ==================================================================
>
> On commit 6799060 (Jan 12).

This and your other relevant reports seem pointing the race of timer
ioctls.  Although snd_timer_close() itself calls snd_timer_stop(),
there is no other protection against the concurrent execution.

If my guess is correct, a simplistic fix like below should work.  It
basically serializes the timer ioctl by using a new mutex (and
replacing the old tread_sem mutex).  They are no longtime blocking
calls, so this shouldn't be a big problem.  But certainly there can be
a less intrusive way to paper over this if this really matters.

In this case for timer.c, I'd leave the final decision rather to
Jaroslav.  Jaroslav, what do you think?

thanks,

Takashi
  • Loading branch information
@tiwai
tiwai authored and 0day robot committed Jan 13, 2016
1 parent c4a359a commit ecc8983
Showing 1 changed file with 19 additions and 13 deletions.
32 changes: 19 additions & 13 deletions sound/core/timer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -1253,7 +1253,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),
Expand All @@ -1273,8 +1273,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);
Expand Down Expand Up @@ -1512,7 +1514,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;
Expand Down Expand Up @@ -1556,7 +1557,6 @@ static int snd_timer_user_tselect(struct file *file,
}

__err:
mutex_unlock(&tu->tread_sem);
return err;
}

Expand Down Expand Up @@ -1769,7 +1769,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;
Expand All @@ -1786,17 +1786,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:
Expand Down Expand Up @@ -1829,6 +1823,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;
Expand Down

0 comments on commit ecc8983

Please sign in to comment.