linux-kernel - [PATCH 3/4] fs: btrfs: fix data races in start

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <20200509052907.3324-1-baijiaju1990@gmail.com>
Date:   Sat,  9 May 2020 13:29:07 +0800
From:   Jia-Ju Bai <baijiaju1990@...il.com>
To:     clm@...com, josef@...icpanda.com, dsterba@...e.com
Cc:     linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jia-Ju Bai <baijiaju1990@...il.com>
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 <baijiaju1990@...il.com>
---
 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