[PATCH] btrfs: Two data races in btrfs

[PATCH] btrfs: Two data races in btrfs

[Hao-ran Zheng]

Hello maintainers,

I would like to report two data race bugs we discovered in BTRFS
filesystem on Linux kernel v6.16-rc3. These issues were identified
using our data race detector.

These issues were deemed non-critical after evaluation, as they
do not impact core functionality or security. To minimize
performance overhead while ensuring clarity for future maintenance,
I have annotated them with data_race() macros.

Below is a summary of the findings:

---

============ DATARACE ============
 extent_write_cache_pages fs/btrfs/extent_io.c:2439 [inline]
 btrfs_writepages+0x34fc/0x3d20 fs/btrfs/extent_io.c:2376
 do_writepages+0x302/0x7c0 mm/page-writeback.c:2687
 filemap_fdatawrite_wbc mm/filemap.c:389 [inline]
  __filemap_fdatawrite_range mm/filemap.c:422 [inline]
 filemap_fdatawrite_range+0x145/0x1d0 mm/filemap.c:440
 btrfs_fdatawrite_range fs/btrfs/file.c:3701 [inline]
 start_ordered_ops fs/btrfs/file.c:1439 [inline]
 btrfs_sync_file+0x6e7/0x1d70 fs/btrfs/file.c:1550
 generic_write_sync include/linux/fs.h:2970 [inline]
 btrfs_do_write_iter+0xd0c/0x12f0 fs/btrfs/file.c:1391
 btrfs_file_write_iter+0x3d/0x60 fs/btrfs/file.c:1401
 new_sync_write fs/read_write.c:586 [inline]
 vfs_write+0x940/0xd10 fs/read_write.c:679
 ksys_write+0x116/0x200 fs/read_write.c:731
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xc9/0x1a0 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
 0x0
============OTHER_INFO============
 extent_write_cache_pages fs/btrfs/extent_io.c:2439 [inline]
 btrfs_writepages+0x34fc/0x3d20 fs/btrfs/extent_io.c:2376
 do_writepages+0x302/0x7c0 mm/page-writeback.c:2687
 filemap_fdatawrite_wbc mm/filemap.c:389 [inline]
 __filemap_fdatawrite_range mm/filemap.c:422 [inline]
 filemap_fdatawrite_range+0x145/0x1d0 mm/filemap.c:440
 btrfs_fdatawrite_range fs/btrfs/file.c:3701 [inline]
 start_ordered_ops fs/btrfs/file.c:1439 [inline]
 btrfs_sync_file+0x509/0x1d70 fs/btrfs/file.c:1521
 generic_write_sync include/linux/fs.h:2970 [inline]
 btrfs_do_write_iter+0xd0c/0x12f0 fs/btrfs/file.c:1391
 btrfs_file_write_iter+0x3d/0x60 fs/btrfs/file.c:1401
 new_sync_write fs/read_write.c:586 [inline]
 vfs_write+0x940/0xd10 fs/read_write.c:679
 ksys_write+0x116/0x200 fs/read_write.c:731
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xc9/0x1a0 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
=================END==============

===========================DATA_RACE===========================
 btrfs_inode_safe_disk_i_size_write+0x144/0x190 fs/btrfs/file-item.c:65
 btrfs_finish_one_ordered+0x999/0x1330 fs/btrfs/inode.c:3203
 btrfs_finish_ordered_io+0x33/0x50 fs/btrfs/inode.c:3308
 finish_ordered_fn+0x3a/0x50 fs/btrfs/ordered-data.c:331
 btrfs_work_helper+0x199/0x6c0 fs/btrfs/async-thread.c:314
 process_one_work kernel/workqueue.c:3238 [inline]
 process_scheduled_works+0x21f/0x520 kernel/workqueue.c:3319
 worker_thread+0x323/0x4a0 kernel/workqueue.c:3400
 kthread+0x2d5/0x300 kernel/kthread.c:464
 ret_from_fork+0x4d/0x60 arch/x86/kernel/process.c:148
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
============OTHER_INFO============
 fill_stack_inode_item fs/btrfs/delayed-inode.c:1809 [inline]
 btrfs_delayed_update_inode+0x1ab/0xa40 fs/btrfs/delayed-inode.c:1931
 btrfs_update_inode+0x128/0x270 fs/btrfs/inode.c:4156
 btrfs_setxattr_trans+0x143/0x280 fs/btrfs/xattr.c:266
 btrfs_xattr_handler_set+0xb7/0xf0 fs/btrfs/xattr.c:380
 __vfs_setxattr+0x21e/0x240 fs/xattr.c:200
 __vfs_setxattr_noperm+0xa5/0x2d0 fs/xattr.c:234
 vfs_setxattr+0xd5/0x1d0 fs/xattr.c:321
 do_setxattr fs/xattr.c:636 [inline]
 file_setxattr+0xb0/0x110 fs/xattr.c:646
 path_setxattrat+0x217/0x260 fs/xattr.c:711
 __do_sys_fsetxattr fs/xattr.c:761 [inline]
 __se_sys_fsetxattr fs/xattr.c:758 [inline]
 __x64_sys_fsetxattr+0x2c/0x40 fs/xattr.c:758
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xc9/0x1a0 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
=================================

Signed-off-by: Hao-ran Zheng <zhenghaoran154@gmail.com>
---
 fs/btrfs/extent_io.c | 2 +-
 fs/btrfs/file-item.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 849199768664..0c03fafc3ae0 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2436,7 +2436,7 @@ static int extent_write_cache_pages(struct address_space *mapping,
 	}
 
 	if (wbc->range_cyclic || (wbc->nr_to_write > 0 && range_whole))
-		mapping->writeback_index = done_index;
+		data_race(mapping->writeback_index = done_index);
 
 	btrfs_add_delayed_iput(BTRFS_I(inode));
 	return ret;
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 54d523d4f421..15572e79b6de 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -61,7 +61,7 @@ void btrfs_inode_safe_disk_i_size_write(struct btrfs_inode *inode, u64 new_i_siz
 		i_size = min(i_size, end + 1);
 	else
 		i_size = 0;
-	inode->disk_i_size = i_size;
+	data_race(inode->disk_i_size = i_size);
 out_unlock:
 	spin_unlock(&inode->lock);
 }
-- 
2.34.1

Re: [PATCH] btrfs: Two data races in btrfs

[Filipe Manana]

On Thu, Jun 26, 2025 at 4:08 AM Hao-ran Zheng <zhenghaoran154@gmail.com> wrote:
>
> Hello maintainers,
>
> I would like to report two data race bugs we discovered in BTRFS
> filesystem on Linux kernel v6.16-rc3. These issues were identified
> using our data race detector.

This sort of text is not proper for a patch... Saying hello and saying
that you are reporting is totally irrelevant and odd.

And what do you mean by "our data race detector"? Is it something else
other than KCSAN, something not in the linux kernel tree?

>
> These issues were deemed non-critical after evaluation, as they
> do not impact core functionality or security. To minimize
> performance overhead while ensuring clarity for future maintenance,
> I have annotated them with data_race() macros.

You have to explain things in far more detail.
To me it seems you are sprinkling data_race() randomly just because
it's fine in some other places.
You don't explain why "these issues were deemed non-critical after evaluation".

>
> Below is a summary of the findings:
>
> ---
>
> ============ DATARACE ============
>  extent_write_cache_pages fs/btrfs/extent_io.c:2439 [inline]
>  btrfs_writepages+0x34fc/0x3d20 fs/btrfs/extent_io.c:2376
>  do_writepages+0x302/0x7c0 mm/page-writeback.c:2687
>  filemap_fdatawrite_wbc mm/filemap.c:389 [inline]
>   __filemap_fdatawrite_range mm/filemap.c:422 [inline]
>  filemap_fdatawrite_range+0x145/0x1d0 mm/filemap.c:440
>  btrfs_fdatawrite_range fs/btrfs/file.c:3701 [inline]
>  start_ordered_ops fs/btrfs/file.c:1439 [inline]
>  btrfs_sync_file+0x6e7/0x1d70 fs/btrfs/file.c:1550
>  generic_write_sync include/linux/fs.h:2970 [inline]
>  btrfs_do_write_iter+0xd0c/0x12f0 fs/btrfs/file.c:1391
>  btrfs_file_write_iter+0x3d/0x60 fs/btrfs/file.c:1401
>  new_sync_write fs/read_write.c:586 [inline]
>  vfs_write+0x940/0xd10 fs/read_write.c:679
>  ksys_write+0x116/0x200 fs/read_write.c:731
>  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>  do_syscall_64+0xc9/0x1a0 arch/x86/entry/common.c:83
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
>  0x0
> ============OTHER_INFO============
>  extent_write_cache_pages fs/btrfs/extent_io.c:2439 [inline]
>  btrfs_writepages+0x34fc/0x3d20 fs/btrfs/extent_io.c:2376
>  do_writepages+0x302/0x7c0 mm/page-writeback.c:2687
>  filemap_fdatawrite_wbc mm/filemap.c:389 [inline]
>  __filemap_fdatawrite_range mm/filemap.c:422 [inline]
>  filemap_fdatawrite_range+0x145/0x1d0 mm/filemap.c:440
>  btrfs_fdatawrite_range fs/btrfs/file.c:3701 [inline]
>  start_ordered_ops fs/btrfs/file.c:1439 [inline]
>  btrfs_sync_file+0x509/0x1d70 fs/btrfs/file.c:1521
>  generic_write_sync include/linux/fs.h:2970 [inline]
>  btrfs_do_write_iter+0xd0c/0x12f0 fs/btrfs/file.c:1391
>  btrfs_file_write_iter+0x3d/0x60 fs/btrfs/file.c:1401
>  new_sync_write fs/read_write.c:586 [inline]
>  vfs_write+0x940/0xd10 fs/read_write.c:679
>  ksys_write+0x116/0x200 fs/read_write.c:731
>  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>  do_syscall_64+0xc9/0x1a0 arch/x86/entry/common.c:83
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> =================END==============
>
> ===========================DATA_RACE===========================
>  btrfs_inode_safe_disk_i_size_write+0x144/0x190 fs/btrfs/file-item.c:65
>  btrfs_finish_one_ordered+0x999/0x1330 fs/btrfs/inode.c:3203
>  btrfs_finish_ordered_io+0x33/0x50 fs/btrfs/inode.c:3308
>  finish_ordered_fn+0x3a/0x50 fs/btrfs/ordered-data.c:331
>  btrfs_work_helper+0x199/0x6c0 fs/btrfs/async-thread.c:314
>  process_one_work kernel/workqueue.c:3238 [inline]
>  process_scheduled_works+0x21f/0x520 kernel/workqueue.c:3319
>  worker_thread+0x323/0x4a0 kernel/workqueue.c:3400
>  kthread+0x2d5/0x300 kernel/kthread.c:464
>  ret_from_fork+0x4d/0x60 arch/x86/kernel/process.c:148
>  ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
> ============OTHER_INFO============
>  fill_stack_inode_item fs/btrfs/delayed-inode.c:1809 [inline]
>  btrfs_delayed_update_inode+0x1ab/0xa40 fs/btrfs/delayed-inode.c:1931
>  btrfs_update_inode+0x128/0x270 fs/btrfs/inode.c:4156
>  btrfs_setxattr_trans+0x143/0x280 fs/btrfs/xattr.c:266
>  btrfs_xattr_handler_set+0xb7/0xf0 fs/btrfs/xattr.c:380
>  __vfs_setxattr+0x21e/0x240 fs/xattr.c:200
>  __vfs_setxattr_noperm+0xa5/0x2d0 fs/xattr.c:234
>  vfs_setxattr+0xd5/0x1d0 fs/xattr.c:321
>  do_setxattr fs/xattr.c:636 [inline]
>  file_setxattr+0xb0/0x110 fs/xattr.c:646
>  path_setxattrat+0x217/0x260 fs/xattr.c:711
>  __do_sys_fsetxattr fs/xattr.c:761 [inline]
>  __se_sys_fsetxattr fs/xattr.c:758 [inline]
>  __x64_sys_fsetxattr+0x2c/0x40 fs/xattr.c:758
>  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>  do_syscall_64+0xc9/0x1a0 arch/x86/entry/common.c:83
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> =================================
>
> Signed-off-by: Hao-ran Zheng <zhenghaoran154@gmail.com>
> ---
>  fs/btrfs/extent_io.c | 2 +-
>  fs/btrfs/file-item.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 849199768664..0c03fafc3ae0 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2436,7 +2436,7 @@ static int extent_write_cache_pages(struct address_space *mapping,
>         }
>
>         if (wbc->range_cyclic || (wbc->nr_to_write > 0 && range_whole))
> -               mapping->writeback_index = done_index;
> +               data_race(mapping->writeback_index = done_index);
>
>         btrfs_add_delayed_iput(BTRFS_I(inode));
>         return ret;
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 54d523d4f421..15572e79b6de 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -61,7 +61,7 @@ void btrfs_inode_safe_disk_i_size_write(struct btrfs_inode *inode, u64 new_i_siz
>                 i_size = min(i_size, end + 1);
>         else
>                 i_size = 0;
> -       inode->disk_i_size = i_size;
> +       data_race(inode->disk_i_size = i_size);


These are two completely different cases, each should be in a
dedicated patch with a proper analysis.

At least this one for disk_i_size, I don't think data_race() is a good solution.
It doesn't prevent store and load tearing, which would result in an
inode item with a bogus size.

Please provide a rationale for the proposed solution for each case.
We have gone through this in a patch you sent in the past (commit
5324c4e10e9c2ce307a037e904c0d9671d7137d9), and there data_race() was
ok because getting a stale value or some weird value due to the result
of a load/store tearing would only makes us take unnecessary locks,
therefore not affecting correctness - it's this type of analysis that
you should place in a change log.

Thanks.

>  out_unlock:
>         spin_unlock(&inode->lock);
>  }
> --
> 2.34.1
>
>