2020-05-09 05:31:48

by Jia-Ju Bai

[permalink] [raw]
Subject: [PATCH 3/4] fs: btrfs: fix data races in start_transaction()

The functions start_transaction() and btrfs_update_delayed_refs_rsv()
are concurrently executed at runtime in the following call contexts:

Thread 1:
btrfs_sync_file()
btrfs_start_transaction()
start_transaction()

Thread 2:
finish_ordered_fn()
btrfs_finish_ordered_io()
insert_reserved_file_extent()
__btrfs_drop_extents()
btrfs_free_extent()
btrfs_add_delayed_data_ref()
btrfs_update_delayed_refs_rsv()

In start_transaction():
if (delayed_refs_rsv->full == 0)
...
else if (... && !delayed_refs_rsv->full)

In btrfs_update_delayed_refs_rsv():
spin_lock(&delayed_rsv->lock);
delayed_rsv->size += num_bytes;
delayed_rsv->full = 0;
spin_unlock(&delayed_rsv->lock);

The values delayed_refs_rsv->full and delayed_rsv->full access the same
memory, and these data races can occur.
These data races were found and actually reproduced by our conccurency
fuzzer.

To fix these races, the spinlock delayed_refs_rsv->lock is used to
protect the access to delayed_refs_rsv->full in start_transaction().

Signed-off-by: Jia-Ju Bai <[email protected]>
---
fs/btrfs/transaction.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 8cede6eb9843..ca38d7cf665d 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -524,6 +524,7 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
u64 qgroup_reserved = 0;
bool reloc_reserved = false;
int ret;
+ unsigned short full = 0;

/* Send isn't supposed to start transactions. */
ASSERT(current->journal_info != BTRFS_SEND_TRANS_STUB);
@@ -541,6 +542,10 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
goto got_it;
}

+ spin_lock(&delayed_refs_rsv->lock);
+ full = delayed_refs_rsv->full;
+ spin_unlock(&delayed_refs_rsv->lock);
+
/*
* Do the reservation before we join the transaction so we can do all
* the appropriate flushing if need be.
@@ -563,7 +568,7 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
* refill that amount for whatever is missing in the reserve.
*/
num_bytes = btrfs_calc_insert_metadata_size(fs_info, num_items);
- if (delayed_refs_rsv->full == 0) {
+ if (full == 0) {
delayed_refs_bytes = num_bytes;
num_bytes <<= 1;
}
@@ -585,7 +590,7 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
num_bytes -= delayed_refs_bytes;
}
} else if (num_items == 0 && flush == BTRFS_RESERVE_FLUSH_ALL &&
- !delayed_refs_rsv->full) {
+ !full) {
/*
* Some people call with btrfs_start_transaction(root, 0)
* because they can be throttled, but have some other mechanism
--
2.17.1