linux-kernel - [PATCH] btrfs: fix data race when accessing the block

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: <20250208073829.1176287-1-zhenghaoran154@gmail.com>
Date: Sat,  8 Feb 2025 15:38:29 +0800
From: Hao-ran Zheng <zhenghaoran154@...il.com>
To: clm@...com,
	josef@...icpanda.com,
	dsterba@...e.com,
	linux-btrfs@...r.kernel.org,
	linux-kernel@...r.kernel.org
Cc: baijiaju1990@...il.com,
	zhenghaoran154@...il.com,
	21371365@...a.edu.cn
Subject: [PATCH] btrfs: fix data race when accessing the block_group's used field

A data race may occur when the function `btrfs_discard_queue_work()`
and the function `btrfs_update_block_group()` is executed concurrently.
Specifically, when the `btrfs_update_block_group()` function is executed
to lines `cache->used = old_val;`, and `btrfs_discard_queue_work()`
is accessing `if(block_group->used == 0)` will appear with data race,
which may cause block_group to be placed unexpectedly in discard_list or
discard_unused_list. The specific function call stack is as follows:

============DATA_RACE============
 btrfs_discard_queue_work+0x245/0x500 [btrfs]
 __btrfs_add_free_space+0x3066/0x32f0 [btrfs]
 btrfs_add_free_space+0x19a/0x200 [btrfs]
 unpin_extent_range+0x847/0x2120 [btrfs]
 btrfs_finish_extent_commit+0x9a3/0x1840 [btrfs]
 btrfs_commit_transaction+0x5f65/0xc0f0 [btrfs]
 transaction_kthread+0x764/0xc20 [btrfs]
 kthread+0x292/0x330
 ret_from_fork+0x4d/0x80
 ret_from_fork_asm+0x1a/0x30
============OTHER_INFO============
 btrfs_update_block_group+0xa9d/0x2430 [btrfs]
 __btrfs_free_extent+0x4f69/0x9920 [btrfs]
 __btrfs_run_delayed_refs+0x290e/0xd7d0 [btrfs]
 btrfs_run_delayed_refs+0x317/0x770 [btrfs]
 flush_space+0x388/0x1440 [btrfs]
 btrfs_preempt_reclaim_metadata_space+0xd65/0x14c0 [btrfs]
 process_scheduled_works+0x716/0xf10
 worker_thread+0xb6a/0x1190
 kthread+0x292/0x330
 ret_from_fork+0x4d/0x80
 ret_from_fork_asm+0x1a/0x30
=================================

Although the `block_group->used` was checked again in the use of the
`peek_discard_list` function, considering that `block_group->used` is
a 64-bit variable, we still think that the data race here is an
unexpected behavior. It is recommended to add `READ_ONCE` and
`WRITE_ONCE` to read and write.

Signed-off-by: Hao-ran Zheng <zhenghaoran154@...il.com>
---
 fs/btrfs/block-group.c | 4 ++--
 fs/btrfs/discard.c     | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index c0a8f7d92acc..c681b97f6835 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -3678,7 +3678,7 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans,
 	old_val = cache->used;
 	if (alloc) {
 		old_val += num_bytes;
-		cache->used = old_val;
+		WRITE_ONCE(cache->used, old_val);
 		cache->reserved -= num_bytes;
 		cache->reclaim_mark = 0;
 		space_info->bytes_reserved -= num_bytes;
@@ -3690,7 +3690,7 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans,
 		spin_unlock(&space_info->lock);
 	} else {
 		old_val -= num_bytes;
-		cache->used = old_val;
+		WRITE_ONCE(cache->used, old_val);
 		cache->pinned += num_bytes;
 		btrfs_space_info_update_bytes_pinned(space_info, num_bytes);
 		space_info->bytes_used -= num_bytes;
diff --git a/fs/btrfs/discard.c b/fs/btrfs/discard.c
index e815d165cccc..71c57b571d50 100644
--- a/fs/btrfs/discard.c
+++ b/fs/btrfs/discard.c
@@ -363,7 +363,7 @@ void btrfs_discard_queue_work(struct btrfs_discard_ctl *discard_ctl,
 	if (!block_group || !btrfs_test_opt(block_group->fs_info, DISCARD_ASYNC))
 		return;
 
-	if (block_group->used == 0)
+	if (READ_ONCE(block_group->used) == 0)
 		add_to_discard_unused_list(discard_ctl, block_group);
 	else
 		add_to_discard_list(discard_ctl, block_group);
-- 
2.34.1