All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Subject: [PATCH] btrfs: fix a bug that btrfs_invalidapge() can double account ordered extent for subpage
Date: Wed, 27 Jan 2021 14:38:48 +0800	[thread overview]
Message-ID: <20210127063848.72528-1-wqu@suse.com> (raw)

Commit dbfdb6d1b369 ("Btrfs: Search for all ordered extents that could
span across a page") make btrfs_invalidapage() to search all ordered
extents.

The offending code looks like this:

again:
	start = page_start;
	ordered = btrfs_lookup_ordered_range(inode, start, page_end - start + 1);
	if (ordred) {
		end = min(page_end,
			  ordered->file_offset + ordered->num_bytes - 1);

		/* Do the cleanup */

		start = end + 1;
		if (start < page_end)
			goto again;
	}

The behavior is indeed necessary for the incoming subpage support, but
when it iterate through all the ordered extents, it also resets the
search range @start.

This means, for the following cases, we can double account the ordered
extents, causing its bytes_left underflow:

	Page offset
	0		16K		32K
	|<--- OE 1  --->|<--- OE 2 ---->|

As the first iteration will find OE 1, which doesn't cover the full
page, thus after cleanup code, we need to retry again.
But again label will reset start to page_start, and we got OE 1 again,
which causes double account on OE1, and cause OE1's byte_left to
underflow.

The only good news is, this problem can only happen for subpage case, as
for regular sectorsize == PAGE_SIZE case, we will always find a OE ends
at or after page end, thus no way to trigger the problem.

This patch will move the again label after start = page_start, to fix
the bug.
This is just a quick fix, which is easy to backport.

There will be more comprehensive rework to convert the open coded loop to
a proper while loop.

Fixes: dbfdb6d1b369 ("Btrfs: Search for all ordered extents that could span across a page")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/inode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index ef6cb7b620d0..2eea7d22405a 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8184,8 +8184,9 @@ static void btrfs_invalidatepage(struct page *page, unsigned int offset,
 
 	if (!inode_evicting)
 		lock_extent_bits(tree, page_start, page_end, &cached_state);
-again:
+
 	start = page_start;
+again:
 	ordered = btrfs_lookup_ordered_range(inode, start, page_end - start + 1);
 	if (ordered) {
 		found_ordered = true;
-- 
2.30.0


             reply	other threads:[~2021-01-27  6:44 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-27  6:38 Qu Wenruo [this message]
2021-01-27 10:44 ` [PATCH] btrfs: fix a bug that btrfs_invalidapge() can double account ordered extent for subpage Filipe Manana
2021-01-27 10:52   ` Qu Wenruo
2021-01-28 10:50     ` David Sterba

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=20210127063848.72528-1-wqu@suse.com \
    --to=wqu@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    /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.