Linux Btrfs filesystem development
 help / color / mirror / Atom feed
From: Omar Sandoval <osandov@osandov.com>
To: Qu Wenruo <wqu@suse.com>
Cc: linux-btrfs@vger.kernel.org, Omar Sandoval <osandov@fb.com>
Subject: Re: [PATCH] btrfs: fix a out-of-boundary access for copy_compressed_data_to_page()
Date: Thu, 11 Nov 2021 20:26:51 -0800	[thread overview]
Message-ID: <YY3tC8HBOApvh4mK@relinquished.localdomain> (raw)
In-Reply-To: <20211112022253.20576-1-wqu@suse.com>

On Fri, Nov 12, 2021 at 10:22:53AM +0800, Qu Wenruo wrote:
> [BUG]
> The following script can cause btrfs to crash:
> 
>  mount -o compress-force=lzo $DEV /mnt
>  dd if=/dev/urandom of=/mnt/foo bs=4k count=1
>  sync
> 
> The calltrace looks like this:
> 
>  general protection fault, probably for non-canonical address 0xe04b37fccce3b000: 0000 [#1] PREEMPT SMP NOPTI
>  CPU: 5 PID: 164 Comm: kworker/u20:3 Not tainted 5.15.0-rc7-custom+ #4
>  Workqueue: btrfs-delalloc btrfs_work_helper [btrfs]
>  RIP: 0010:__memcpy+0x12/0x20
>  Call Trace:
>   lzo_compress_pages+0x236/0x540 [btrfs]
>   btrfs_compress_pages+0xaa/0xf0 [btrfs]
>   compress_file_range+0x431/0x8e0 [btrfs]
>   async_cow_start+0x12/0x30 [btrfs]
>   btrfs_work_helper+0xf6/0x3e0 [btrfs]
>   process_one_work+0x294/0x5d0
>   worker_thread+0x55/0x3c0
>   kthread+0x140/0x170
>   ret_from_fork+0x22/0x30
>  ---[ end trace 63c3c0f131e61982 ]---
> 
> [CAUSE]
> In lzo_compress_pages(), parameter @out_pages is not only an output
> parameter (for the compressed pages), but also an input parameter, for
> the maximum amount of pages we can utilize.
> 
> In commit d4088803f511 ("btrfs: subpage: make lzo_compress_pages()
> compatible"), the refactor doesn't take @out_pages as an input, thus
> completely ignoring the limit.
> 
> And for compress-force case, we could hit incompressible data that
> compressed size would go beyond the page limit, and cause above crash.
> 
> [FIX]
> Save @out_pages as @max_nr_page, and pass it to lzo_compress_pages(),
> and check if we're beyond the limit before accessing the pages.
> 
> Reported-by: Omar Sandoval <osandov@fb.com>
> Fixes: d4088803f511 ("btrfs: subpage: make lzo_compress_pages() compatible")
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/lzo.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)

This fixed the issue for me, and it looks correct.

Reviewed-by: Omar Sandoval <osandov@fb.com>

      parent reply	other threads:[~2021-11-12  4:26 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-12  2:22 [PATCH] btrfs: fix a out-of-boundary access for copy_compressed_data_to_page() Qu Wenruo
2021-11-12  4:17 ` Josef Bacik
2021-11-12  4:41   ` Qu Wenruo
2021-11-12 14:35     ` David Sterba
2021-11-12  4:26 ` Omar Sandoval [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YY3tC8HBOApvh4mK@relinquished.localdomain \
    --to=osandov@osandov.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=osandov@fb.com \
    --cc=wqu@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox