ACK: [CVE-2016-7910][Precise, Trusty, Vivid] block: fix use-after-free in seq file

[Colin Ian King]

On 23/01/17 10:33, Luis Henriques wrote:
> From: Vegard Nossum <vegard.nossum at oracle.com>
> 
> I got a KASAN report of use-after-free:
> 
>     ==================================================================
>     BUG: KASAN: use-after-free in klist_iter_exit+0x61/0x70 at addr ffff8800b6581508
>     Read of size 8 by task trinity-c1/315
>     =============================================================================
>     BUG kmalloc-32 (Not tainted): kasan: bad access detected
>     -----------------------------------------------------------------------------
> 
>     Disabling lock debugging due to kernel taint
>     INFO: Allocated in disk_seqf_start+0x66/0x110 age=144 cpu=1 pid=315
>             ___slab_alloc+0x4f1/0x520
>             __slab_alloc.isra.58+0x56/0x80
>             kmem_cache_alloc_trace+0x260/0x2a0
>             disk_seqf_start+0x66/0x110
>             traverse+0x176/0x860
>             seq_read+0x7e3/0x11a0
>             proc_reg_read+0xbc/0x180
>             do_loop_readv_writev+0x134/0x210
>             do_readv_writev+0x565/0x660
>             vfs_readv+0x67/0xa0
>             do_preadv+0x126/0x170
>             SyS_preadv+0xc/0x10
>             do_syscall_64+0x1a1/0x460
>             return_from_SYSCALL_64+0x0/0x6a
>     INFO: Freed in disk_seqf_stop+0x42/0x50 age=160 cpu=1 pid=315
>             __slab_free+0x17a/0x2c0
>             kfree+0x20a/0x220
>             disk_seqf_stop+0x42/0x50
>             traverse+0x3b5/0x860
>             seq_read+0x7e3/0x11a0
>             proc_reg_read+0xbc/0x180
>             do_loop_readv_writev+0x134/0x210
>             do_readv_writev+0x565/0x660
>             vfs_readv+0x67/0xa0
>             do_preadv+0x126/0x170
>             SyS_preadv+0xc/0x10
>             do_syscall_64+0x1a1/0x460
>             return_from_SYSCALL_64+0x0/0x6a
> 
>     CPU: 1 PID: 315 Comm: trinity-c1 Tainted: G    B           4.7.0+ #62
>     Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
>      ffffea0002d96000 ffff880119b9f918 ffffffff81d6ce81 ffff88011a804480
>      ffff8800b6581500 ffff880119b9f948 ffffffff8146c7bd ffff88011a804480
>      ffffea0002d96000 ffff8800b6581500 fffffffffffffff4 ffff880119b9f970
>     Call Trace:
>      [<ffffffff81d6ce81>] dump_stack+0x65/0x84
>      [<ffffffff8146c7bd>] print_trailer+0x10d/0x1a0
>      [<ffffffff814704ff>] object_err+0x2f/0x40
>      [<ffffffff814754d1>] kasan_report_error+0x221/0x520
>      [<ffffffff8147590e>] __asan_report_load8_noabort+0x3e/0x40
>      [<ffffffff83888161>] klist_iter_exit+0x61/0x70
>      [<ffffffff82404389>] class_dev_iter_exit+0x9/0x10
>      [<ffffffff81d2e8ea>] disk_seqf_stop+0x3a/0x50
>      [<ffffffff8151f812>] seq_read+0x4b2/0x11a0
>      [<ffffffff815f8fdc>] proc_reg_read+0xbc/0x180
>      [<ffffffff814b24e4>] do_loop_readv_writev+0x134/0x210
>      [<ffffffff814b4c45>] do_readv_writev+0x565/0x660
>      [<ffffffff814b8a17>] vfs_readv+0x67/0xa0
>      [<ffffffff814b8de6>] do_preadv+0x126/0x170
>      [<ffffffff814b92ec>] SyS_preadv+0xc/0x10
> 
> This problem can occur in the following situation:
> 
> open()
>  - pread()
>     - .seq_start()
>        - iter = kmalloc() // succeeds
>        - seqf->private = iter
>     - .seq_stop()
>        - kfree(seqf->private)
>  - pread()
>     - .seq_start()
>        - iter = kmalloc() // fails
>     - .seq_stop()
>        - class_dev_iter_exit(seqf->private) // boom! old pointer
> 
> As the comment in disk_seqf_stop() says, stop is called even if start
> failed, so we need to reinitialise the private pointer to NULL when seq
> iteration stops.
> 
> An alternative would be to set the private pointer to NULL when the
> kmalloc() in disk_seqf_start() fails.
> 
> Cc: stable at vger.kernel.org
> Signed-off-by: Vegard Nossum <vegard.nossum at oracle.com>
> Acked-by: Tejun Heo <tj at kernel.org>
> Signed-off-by: Jens Axboe <axboe at fb.com>
> CVE-2016-7910
> (cherry picked from commit 77da160530dd1dc94f6ae15a981f24e5f0021e84)
> Signed-off-by: Luis Henriques <luis.henriques at canonical.com>
> ---
>  block/genhd.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/block/genhd.c b/block/genhd.c
> index 424d1fa1c039..4c3f6db08626 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -829,6 +829,7 @@ static void disk_seqf_stop(struct seq_file *seqf, void *v)
>  	if (iter) {
>  		class_dev_iter_exit(iter);
>  		kfree(iter);
> +		seqf->private = NULL;
>  	}
>  }
>  
> 
Seems sane to me.

Acked-by: Colin Ian King <colin.king at canonical.com>