• Qu Wenruo's avatar
    btrfs: inode: fix NULL pointer dereference if inode doesn't need compression · 1e6e238c
    Qu Wenruo authored
    [BUG]
    There is a bug report of NULL pointer dereference caused in
    compress_file_extent():
    
      Oops: Kernel access of bad area, sig: 11 [#1]
      LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
      Workqueue: btrfs-delalloc btrfs_delalloc_helper [btrfs]
      NIP [c008000006dd4d34] compress_file_range.constprop.41+0x75c/0x8a0 [btrfs]
      LR [c008000006dd4d1c] compress_file_range.constprop.41+0x744/0x8a0 [btrfs]
      Call Trace:
      [c000000c69093b00] [c008000006dd4d1c] compress_file_range.constprop.41+0x744/0x8a0 [btrfs] (unreliable)
      [c000000c69093bd0] [c008000006dd4ebc] async_cow_start+0x44/0xa0 [btrfs]
      [c000000c69093c10] [c008000006e14824] normal_work_helper+0xdc/0x598 [btrfs]
      [c000000c69093c80] [c0000000001608c0] process_one_work+0x2c0/0x5b0
      [c000000c69093d10] [c000000000160c38] worker_thread+0x88/0x660
      [c000000c69093db0] [c00000000016b55c] kthread+0x1ac/0x1c0
      [c000000c69093e20] [c00000000000b660] ret_from_kernel_thread+0x5c/0x7c
      ---[ end trace f16954aa20d822f6 ]---
    
    [CAUSE]
    For the following execution route of compress_file_range(), it's
    possible to hit NULL pointer dereference:
    
     compress_file_extent()
     |- pages = NULL;
     |- start = async_chunk->start = 0;
     |- end = async_chunk = 4095;
     |- nr_pages = 1;
     |- inode_need_compress() == false; <<< Possible, see later explanation
     |  Now, we have nr_pages = 1, pages = NULL
     |- cont:
     |- 		ret = cow_file_range_inline();
     |- 		if (ret <= 0) {
     |-		for (i = 0; i < nr_pages; i++) {
     |-			WARN_ON(pages[i]->mapping);	<<< Crash
    
    To enter above call execution branch, we need the following race:
    
        Thread 1 (chattr)     |            Thread 2 (writeback)
    --------------------------+------------------------------
                              | btrfs_run_delalloc_range
                              | |- inode_need_compress = true
                              | |- cow_file_range_async()
    btrfs_ioctl_set_flag()    |
    |- binode_flags |=        |
       BTRFS_INODE_NOCOMPRESS |
                              | compress_file_range()
                              | |- inode_need_compress = false
                              | |- nr_page = 1 while pages = NULL
                              | |  Then hit the crash
    
    [FIX]
    This patch will fix it by checking @pages before doing accessing it.
    This patch is only designed as a hot fix and easy to backport.
    
    More elegant fix may make btrfs only check inode_need_compress() once to
    avoid such race, but that would be another story.
    Reported-by: default avatarLuciano Chavez <chavez@us.ibm.com>
    Fixes: 4d3a800e ("btrfs: merge nr_pages input and output parameter in compress_pages")
    CC: stable@vger.kernel.org # 4.14.x: cecc8d90: btrfs: Move free_pages_out label in inline extent handling branch in compress_file_range
    CC: stable@vger.kernel.org # 4.14+
    Signed-off-by: default avatarQu Wenruo <wqu@suse.com>
    Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
    1e6e238c
inode.c 287 KB