Skip to content
  • Vegard Nossum's avatar
    fs/seq_file: fix out-of-bounds read · 088bf2ff
    Vegard Nossum authored
    seq_read() is a nasty piece of work, not to mention buggy.
    
    It has (I think) an old bug which allows unprivileged userspace to read
    beyond the end of m->buf.
    
    I was getting these:
    
        BUG: KASAN: slab-out-of-bounds in seq_read+0xcd2/0x1480 at addr ffff880116889880
        Read of size 2713 by task trinity-c2/1329
        CPU: 2 PID: 1329 Comm: trinity-c2 Not tainted 4.8.0-rc1+ #96
        Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
        Call Trace:
          kasan_object_err+0x1c/0x80
          kasan_report_error+0x2cb/0x7e0
          kasan_report+0x4e/0x80
          check_memory_region+0x13e/0x1a0
          kasan_check_read+0x11/0x20
          seq_read+0xcd2/0x1480
          proc_reg_read+0x10b/0x260
          do_loop_readv_writev.part.5+0x140/0x2c0
          do_readv_writev+0x589/0x860
          vfs_readv+0x7b/0xd0
          do_readv+0xd8/0x2c0
          SyS_readv+0xb/0x10
          do_syscall_64+0x1b3/0x4b0
          entry_SYSCALL64_slow_path+0x25/0x25
        Object at ffff880116889100, in cache kmalloc-4096 size: 4096
        Allocated:
        PID = 1329
          save_stack_trace+0x26/0x80
          save_stack+0x46/0xd0
          kasan_kmalloc+0xad/0xe0
          __kmalloc+0x1aa/0x4a0
          seq_buf_alloc+0x35/0x40
          seq_read+0x7d8/0x1480
          proc_reg_read+0x10b/0x260
          do_loop_readv_writev.part.5+0x140/0x2c0
          do_readv_writev+0x589/0x860
          vfs_readv+0x7b/0xd0
          do_readv+0xd8/0x2c0
          SyS_readv+0xb/0x10
          do_syscall_64+0x1b3/0x4b0
          return_from_SYSCALL_64+0x0/0x6a
        Freed:
        PID = 0
        (stack is not available)
        Memory state around the buggy address:
         ffff88011688a000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
         ffff88011688a080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
        >ffff88011688a100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
    		       ^
         ffff88011688a180: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
         ffff88011688a200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
        ==================================================================
        Disabling lock debugging due to kernel taint
    
    This seems to be the same thing that Dave Jones was seeing here:
    
      https://lkml.org/lkml/2016/8/12/334
    
    There are multiple issues here:
    
      1) If we enter the function with a non-empty buffer, there is an attempt
         to flush it. But it was not clearing m->from after doing so, which
         means that if we try to do this flush twice in a row without any call
         to traverse() in between, we are going to be reading from the wrong
         place -- the splat above, fixed by this patch.
    
      2) If there's a short write to userspace because of page faults, the
         buffer may already contain multiple lines (i.e. pos has advanced by
         more than 1), but we don't save the progress that was made so the
         next call will output what we've already returned previously. Since
         that is a much less serious issue (and I have a headache after
         staring at seq_read() for the past 8 hours), I'll leave that for now.
    
    Link: http://lkml.kernel.org/r/1471447270-32093-1-git-send-email-vegard.nossum@oracle.com
    
    
    Signed-off-by: default avatarVegard Nossum <vegard.nossum@oracle.com>
    Reported-by: default avatarDave Jones <davej@codemonkey.org.uk>
    Cc: Al Viro <viro@zeniv.linux.org.uk>
    Cc: <stable@vger.kernel.org>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    088bf2ff