[3/4] fs: btrfs: fix data races in start_transaction()

Commit Message

Jia-Ju Bai May 9, 2020, 5:29 a.m. UTC

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 <[e-mail]>
---
 fs/btrfs/transaction.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Patch

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