All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Cc: Ritesh Harjani <riteshh@linux.ibm.com>, Qu Wenruo <wqu@suse.com>,
	Filipe Manana <fdmanana@suse.com>
Subject: [PATCH v7 13/17] btrfs: fix a crash caused by race between prepare_pages() and btrfs_releasepage()
Date: Mon, 12 Jul 2021 16:30:23 +0800	[thread overview]
Message-ID: <20210712083027.212734-14-wqu@suse.com> (raw)
In-Reply-To: <20210712083027.212734-1-wqu@suse.com>

[BUG]
When running generic/095, there is a high chance to crash with subpage
data RW support:
 assertion failed: PagePrivate(page) && page->private
 ------------[ cut here ]------------
 kernel BUG at fs/btrfs/ctree.h:3403!
 Internal error: Oops - BUG: 0 [#1] SMP
 CPU: 1 PID: 3567 Comm: fio Tainted: 5.12.0-rc7-custom+ #17
 Hardware name: Khadas VIM3 (DT)
 Call trace:
  assertfail.constprop.0+0x28/0x2c [btrfs]
  btrfs_subpage_assert+0x80/0xa0 [btrfs]
  btrfs_subpage_set_uptodate+0x34/0xec [btrfs]
  btrfs_page_clamp_set_uptodate+0x74/0xa4 [btrfs]
  btrfs_dirty_pages+0x160/0x270 [btrfs]
  btrfs_buffered_write+0x444/0x630 [btrfs]
  btrfs_direct_write+0x1cc/0x2d0 [btrfs]
  btrfs_file_write_iter+0xc0/0x160 [btrfs]
  new_sync_write+0xe8/0x180
  vfs_write+0x1b4/0x210
  ksys_pwrite64+0x7c/0xc0
  __arm64_sys_pwrite64+0x24/0x30
  el0_svc_common.constprop.0+0x70/0x140
  do_el0_svc+0x28/0x90
  el0_svc+0x2c/0x54
  el0_sync_handler+0x1a8/0x1ac
  el0_sync+0x170/0x180
 Code: f0000160 913be042 913c4000 955444bc (d4210000)
 ---[ end trace 3fdd39f4cccedd68 ]---

[CAUSE]
Although prepare_pages() calls find_or_create_page(), which returns with
the page locked, but in later prepare_uptodate_page() calls, we may call
btrfs_readpage() which will unlock the page before it returns.

This leaves a window where btrfs_releasepage() can sneak in and release
the page, clearing page->private and causing above ASSERT().

[FIX]
In prepare_uptodate_page(), we should not only check page->mapping, but
also PagePrivate() to ensure we are still hold a correct page which has
proper fs context setup.

Reported-by: Ritesh Harjani <riteshh@linux.ibm.com>
Tested-by: Ritesh Harjani <riteshh@linux.ibm.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/file.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 28a05ba47060..0831ca08376f 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1341,7 +1341,18 @@ static int prepare_uptodate_page(struct inode *inode,
 			unlock_page(page);
 			return -EIO;
 		}
-		if (page->mapping != inode->i_mapping) {
+
+		/*
+		 * Since btrfs_readpage() will unlock the page before it
+		 * returns, there is a window where btrfs_releasepage() can
+		 * be called to release the page.
+		 * Here we check both inode mapping and PagePrivate() to
+		 * make sure the page was not released.
+		 *
+		 * The private flag check is essential for subpage as we need
+		 * to store extra bitmap using page->private.
+		 */
+		if (page->mapping != inode->i_mapping || !PagePrivate(page)) {
 			unlock_page(page);
 			return -EAGAIN;
 		}
-- 
2.32.0


  parent reply	other threads:[~2021-07-12  8:34 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-12  8:30 [PATCH v7 00/17] btrfs: add data write support for subpage Qu Wenruo
2021-07-12  8:30 ` [PATCH v7 01/17] btrfs: properly reset @this_bio_flag in btrfs_do_readpage() to avoid inheriting old bio flags to next extent Qu Wenruo
2021-07-16 13:59   ` Anand Jain
2021-07-12  8:30 ` [PATCH v7 02/17] btrfs: fix NULL pointer dereference when reading two compressed extent inside the same page Qu Wenruo
2021-07-12  8:30 ` [PATCH v7 03/17] btrfs: disable compressed readahead for subpage Qu Wenruo
2021-07-12  8:30 ` [PATCH v7 04/17] btrfs: grab correct extent map for subpage compressed extent read Qu Wenruo
2021-07-12  8:30 ` [PATCH v7 05/17] btrfs: rework btrfs_decompress_buf2page() Qu Wenruo
2021-07-12  8:30 ` [PATCH v7 06/17] btrfs: rework lzo_decompress_bio() to make it subpage compatible Qu Wenruo
2021-07-12  8:30 ` [PATCH v7 07/17] btrfs: extract relocation page read and dirty part into its own function Qu Wenruo
2021-07-12  8:30 ` [PATCH v7 08/17] btrfs: make relocate_one_page() to handle subpage case Qu Wenruo
2021-07-12  8:30 ` [PATCH v7 09/17] btrfs: fix wild subpage writeback which does not have ordered extent Qu Wenruo
2021-07-12  8:30 ` [PATCH v7 10/17] btrfs: disable inline extent creation for subpage Qu Wenruo
2021-07-12  8:30 ` [PATCH v7 11/17] btrfs: allow submit_extent_page() to do bio split " Qu Wenruo
2021-07-12  8:30 ` [PATCH v7 12/17] btrfs: reject raid5/6 fs " Qu Wenruo
2021-07-13  0:39   ` Anand Jain
2021-07-12  8:30 ` Qu Wenruo [this message]
2021-07-12  8:30 ` [PATCH v7 14/17] btrfs: fix a use-after-free bug in writeback subpage helper Qu Wenruo
2021-07-12  8:30 ` [PATCH v7 15/17] btrfs: fix a subpage false alert for relocating partial preallocated data extents Qu Wenruo
2021-07-12  8:30 ` [PATCH v7 16/17] btrfs: fix a subpage relocation data corruption Qu Wenruo
2021-07-12  8:30 ` [PATCH v7 17/17] btrfs: allow read-write for 4K sectorsize on 64K page size systems Qu Wenruo

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=20210712083027.212734-14-wqu@suse.com \
    --to=wqu@suse.com \
    --cc=fdmanana@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=riteshh@linux.ibm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.