From: fdmanana@kernel.org
To: linux-btrfs@vger.kernel.org
Subject: [PATCH] Btrfs: fix negative subv_writers counter and data space leak after buffered write
Date: Wed, 9 Oct 2019 17:44:22 +0100 [thread overview]
Message-ID: <20191009164422.7202-1-fdmanana@kernel.org> (raw)
From: Filipe Manana <fdmanana@suse.com>
When doing a buffered write it's possible to leave the subv_writers
counter of the root, used for synchronization between buffered nocow
writers and snapshotting. This happens in an exceptional case like the
following:
1) We fail to allocate data space for the write, since there's not
enough available data space nor enough unallocated space for allocating
a new data block group;
2) Because of that failure, we try to go to NOCOW mode, which succeeds
and therefore we set the local variable 'only_release_metadata' to true
and set the root's sub_writers counter to 1 through the call to
btrfs_start_write_no_snapshotting() made by check_can_nocow();
3) The call to btrfs_copy_from_user() returns zero, which is very unlikely
to happen but not impossible;
4) No pages are copied because btrfs_copy_from_user() returned zero;
5) We call btrfs_end_write_no_snapshotting() which decrements the root's
subv_writers counter to 0;
6) We don't set 'only_release_metadata' back to 'false' because we do
it only if 'copied', the value returned by btrfs_copy_from_user(), is
greater than zero;
7) On the next iteration of the while loop, which processes the same
page range, we are now able to allocate data space for the write (we
got enough data space released in the meanwhile);
8) After this if we fail at btrfs_delalloc_reserve_metadata(), because
now there isn't enough free metadata space, or in some other place
further below (prepare_pages(), lock_and_cleanup_extent_if_need(),
btrfs_dirty_pages()), we break out of the while loop with
'only_release_metadata' having a value of 'true';
9) Because 'only_release_metadata' is 'true' we end up decrementing the
root's subv_writers counter to -1, and we also end up not releasing the
data space previously reserved through btrfs_check_data_free_space().
As a consequence the mechanism for synchronizing NOCOW buffered writes
with snapshotting gets broken.
Fix this by always setting 'only_release_metadata' to false whenever it
currently has a true value, independently of having been able to copy any
data to the pages.
Fixes: 8257b2dc3c1a10 ("Btrfs: introduce btrfs_{start, end}_nocow_write() for each subvolume")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/file.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 27e5b269e729..c98c1d10fd3a 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1780,18 +1780,19 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
}
release_bytes = 0;
- if (only_release_metadata)
+ if (only_release_metadata) {
btrfs_end_write_no_snapshotting(root);
-
- if (only_release_metadata && copied > 0) {
- lockstart = round_down(pos,
- fs_info->sectorsize);
- lockend = round_up(pos + copied,
- fs_info->sectorsize) - 1;
-
- set_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
- lockend, EXTENT_NORESERVE, NULL,
- NULL, GFP_NOFS);
+ if (copied > 0) {
+ lockstart = round_down(pos,
+ fs_info->sectorsize);
+ lockend = round_up(pos + copied,
+ fs_info->sectorsize) - 1;
+
+ set_extent_bit(&BTRFS_I(inode)->io_tree,
+ lockstart, lockend,
+ EXTENT_NORESERVE, NULL, NULL,
+ GFP_NOFS);
+ }
only_release_metadata = false;
}
--
2.11.0
next reply other threads:[~2019-10-09 16:44 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-09 16:44 fdmanana [this message]
2019-10-11 13:27 ` [PATCH] Btrfs: fix negative subv_writers counter and data space leak after buffered write Josef Bacik
2019-10-11 15:40 ` Filipe Manana
2019-10-11 15:41 ` [PATCH v2] " fdmanana
2019-10-11 17:14 ` Josef Bacik
2019-10-11 18:23 ` 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=20191009164422.7202-1-fdmanana@kernel.org \
--to=fdmanana@kernel.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).