ACK: [CVE-2016-7911][Precise, Trusty] block: fix use-after-free in sys_ioprio_get()

[Colin Ian King]

On 23/01/17 10:34, Luis Henriques wrote:
> From: Omar Sandoval <osandov at fb.com>
> 
> get_task_ioprio() accesses the task->io_context without holding the task
> lock and thus can race with exit_io_context(), leading to a
> use-after-free. The reproducer below hits this within a few seconds on
> my 4-core QEMU VM:
> 
> int main(int argc, char **argv)
> {
> 	pid_t pid, child;
> 	long nproc, i;
> 
> 	/* ioprio_set(IOPRIO_WHO_PROCESS, 0, IOPRIO_PRIO_VALUE(IOPRIO_CLASS_IDLE, 0)); */
> 	syscall(SYS_ioprio_set, 1, 0, 0x6000);
> 
> 	nproc = sysconf(_SC_NPROCESSORS_ONLN);
> 
> 	for (i = 0; i < nproc; i++) {
> 		pid = fork();
> 		assert(pid != -1);
> 		if (pid == 0) {
> 			for (;;) {
> 				pid = fork();
> 				assert(pid != -1);
> 				if (pid == 0) {
> 					_exit(0);
> 				} else {
> 					child = wait(NULL);
> 					assert(child == pid);
> 				}
> 			}
> 		}
> 
> 		pid = fork();
> 		assert(pid != -1);
> 		if (pid == 0) {
> 			for (;;) {
> 				/* ioprio_get(IOPRIO_WHO_PGRP, 0); */
> 				syscall(SYS_ioprio_get, 2, 0);
> 			}
> 		}
> 	}
> 
> 	for (;;) {
> 		/* ioprio_get(IOPRIO_WHO_PGRP, 0); */
> 		syscall(SYS_ioprio_get, 2, 0);
> 	}
> 
> 	return 0;
> }
> 
> This gets us KASAN dumps like this:
> 
> [   35.526914] ==================================================================
> [   35.530009] BUG: KASAN: out-of-bounds in get_task_ioprio+0x7b/0x90 at addr ffff880066f34e6c
> [   35.530009] Read of size 2 by task ioprio-gpf/363
> [   35.530009] =============================================================================
> [   35.530009] BUG blkdev_ioc (Not tainted): kasan: bad access detected
> [   35.530009] -----------------------------------------------------------------------------
> 
> [   35.530009] Disabling lock debugging due to kernel taint
> [   35.530009] INFO: Allocated in create_task_io_context+0x2b/0x370 age=0 cpu=0 pid=360
> [   35.530009] 	___slab_alloc+0x55d/0x5a0
> [   35.530009] 	__slab_alloc.isra.20+0x2b/0x40
> [   35.530009] 	kmem_cache_alloc_node+0x84/0x200
> [   35.530009] 	create_task_io_context+0x2b/0x370
> [   35.530009] 	get_task_io_context+0x92/0xb0
> [   35.530009] 	copy_process.part.8+0x5029/0x5660
> [   35.530009] 	_do_fork+0x155/0x7e0
> [   35.530009] 	SyS_clone+0x19/0x20
> [   35.530009] 	do_syscall_64+0x195/0x3a0
> [   35.530009] 	return_from_SYSCALL_64+0x0/0x6a
> [   35.530009] INFO: Freed in put_io_context+0xe7/0x120 age=0 cpu=0 pid=1060
> [   35.530009] 	__slab_free+0x27b/0x3d0
> [   35.530009] 	kmem_cache_free+0x1fb/0x220
> [   35.530009] 	put_io_context+0xe7/0x120
> [   35.530009] 	put_io_context_active+0x238/0x380
> [   35.530009] 	exit_io_context+0x66/0x80
> [   35.530009] 	do_exit+0x158e/0x2b90
> [   35.530009] 	do_group_exit+0xe5/0x2b0
> [   35.530009] 	SyS_exit_group+0x1d/0x20
> [   35.530009] 	entry_SYSCALL_64_fastpath+0x1a/0xa4
> [   35.530009] INFO: Slab 0xffffea00019bcd00 objects=20 used=4 fp=0xffff880066f34ff0 flags=0x1fffe0000004080
> [   35.530009] INFO: Object 0xffff880066f34e58 @offset=3672 fp=0x0000000000000001
> [   35.530009] ==================================================================
> 
> Fix it by grabbing the task lock while we poke at the io_context.
> 
> Cc: stable at vger.kernel.org
> Reported-by: Dmitry Vyukov <dvyukov at google.com>
> Signed-off-by: Omar Sandoval <osandov at fb.com>
> Signed-off-by: Jens Axboe <axboe at fb.com>
> CVE-2016-7911
> (backported from commit 8ba8682107ee2ca3347354e018865d8e1967c5f4)
> [ luis: file renamed: block/ioprio.c -> fs/ioprio.c ]
> Signed-off-by: Luis Henriques <luis.henriques at canonical.com>
> ---
>  fs/ioprio.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/ioprio.c b/fs/ioprio.c
> index 5b55511945ef..e1ba1e607689 100644
> --- a/fs/ioprio.c
> +++ b/fs/ioprio.c
> @@ -161,8 +161,10 @@ static int get_task_ioprio(struct task_struct *p)
>  	if (ret)
>  		goto out;
>  	ret = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, IOPRIO_NORM);
> +	task_lock(p);
>  	if (p->io_context)
>  		ret = p->io_context->ioprio;
> +	task_unlock(p);
>  out:
>  	return ret;
>  }
> 
Acked-by: Colin Ian King <colin.king at canonical.com>