[PATCH] btrfs: fix use-after-free in mark_block_group_to_copy

[PATCH] btrfs: fix use-after-free in mark_block_group_to_copy

[Hongling Zeng]

mark_block_group_to_copy() iterates over the commit root with
skip_locking=true to avoid lock contention. Without holding
commit_root_sem,a concurrent transaction commit can swap and free
the commit root during iteration, leading to use-after-free when
accessing extent buffers.

The fix adds commit_root_sem locking, but CRITICALLY must only cover the
actual search period (btrfs_for_each_slot), NOT the preceding while loop
which calls btrfs_commit_transaction(). If held during that loop, we get
self-deadlock: down_read(commit_root_sem) → btrfs_commit_transaction()
→ switch_commit_roots() → down_write(commit_root_sem), blocking forever
on our own read lock.

Lock scope:
- down_read() after path setup, before btrfs_for_each_slot
- up_read() immediately after btrfs_for_each_slot completes
- NOT held during while loop (btrfs_commit_transaction) path
- NOT held on the !path error path (goto unlock before lock
  acquisition)

This matches the established btrfs pattern used in send.c, backref.c, etc.:
hold commit_root_sem read lock ONLY while searching commit root, never
across transaction commits.

Fixes: 78ce9fc269af ("btrfs: zoned: mark block groups to copy for device-replace")
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/btrfs/dev-replace.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index dc0834f920c3..734807bf48b3 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -499,6 +499,7 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
 	key.type = BTRFS_DEV_EXTENT_KEY;
 	key.offset = 0;
 
+	down_read(&fs_info->commit_root_sem);
 	btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
 		struct extent_buffer *leaf = path->nodes[0];
 
@@ -525,6 +526,8 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
 	if (iter_ret < 0)
 		ret = iter_ret;
 
+	up_read(&fs_info->commit_root_sem);
+
 	btrfs_free_path(path);
 unlock:
 	mutex_unlock(&fs_info->chunk_mutex);
-- 
2.25.1

[PATCH] btrfs: fix use-after-free in mark_block_group_to_copy

[Hongling Zeng]

mark_block_group_to_copy() iterates over the commit root with
skip_locking=true to avoid lock contention. Without holding
commit_root_sem,a concurrent transaction commit can swap and free
the commit root during iteration, leading to use-after-free when
accessing extent buffers.

The fix adds commit_root_sem locking, but CRITICALLY must only cover the
actual search period (btrfs_for_each_slot), NOT the preceding while loop
which calls btrfs_commit_transaction(). If held during that loop, we get
self-deadlock: down_read(commit_root_sem) → btrfs_commit_transaction()
→ switch_commit_roots() → down_write(commit_root_sem), blocking forever
on our own read lock.

Lock scope:
- down_read() after path setup, before btrfs_for_each_slot
- up_read() immediately after btrfs_for_each_slot completes
- NOT held during while loop (btrfs_commit_transaction) path
- NOT held on the !path error path (goto unlock before lock
  acquisition)

This matches the established btrfs pattern used in send.c, backref.c, etc.:
hold commit_root_sem read lock ONLY while searching commit root, never
across transaction commits.

Fixes: 78ce9fc269af ("btrfs: zoned: mark block groups to copy for device-replace")
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/btrfs/dev-replace.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index dc0834f920c3..734807bf48b3 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -499,6 +499,7 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
 	key.type = BTRFS_DEV_EXTENT_KEY;
 	key.offset = 0;
 
+	down_read(&fs_info->commit_root_sem);
 	btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
 		struct extent_buffer *leaf = path->nodes[0];
 
@@ -525,6 +526,8 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
 	if (iter_ret < 0)
 		ret = iter_ret;
 
+	up_read(&fs_info->commit_root_sem);
+
 	btrfs_free_path(path);
 unlock:
 	mutex_unlock(&fs_info->chunk_mutex);
-- 
2.25.1

Re: [PATCH] btrfs: fix use-after-free in mark_block_group_to_copy

[Qu Wenruo]

在 2026/8/17 11:15, Hongling Zeng 写道:
> mark_block_group_to_copy() iterates over the commit root with
> skip_locking=true to avoid lock contention. Without holding
> commit_root_sem,a concurrent transaction commit can swap and free
> the commit root during iteration, leading to use-after-free when
> accessing extent buffers.
> 
> The fix adds commit_root_sem locking, but CRITICALLY must only cover the
> actual search period (btrfs_for_each_slot), NOT the preceding while loop
> which calls btrfs_commit_transaction(). If held during that loop, we get
> self-deadlock: down_read(commit_root_sem) → btrfs_commit_transaction()
> → switch_commit_roots() → down_write(commit_root_sem), blocking forever
> on our own read lock.
> 
> Lock scope:
> - down_read() after path setup, before btrfs_for_each_slot
> - up_read() immediately after btrfs_for_each_slot completes
> - NOT held during while loop (btrfs_commit_transaction) path
> - NOT held on the !path error path (goto unlock before lock
>    acquisition)
> 
> This matches the established btrfs pattern used in send.c, backref.c, etc.:
> hold commit_root_sem read lock ONLY while searching commit root, never
> across transaction commits.
> 
> Fixes: 78ce9fc269af ("btrfs: zoned: mark block groups to copy for device-replace")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

Please disclose LLM usage.

> ---
>   fs/btrfs/dev-replace.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> index dc0834f920c3..734807bf48b3 100644
> --- a/fs/btrfs/dev-replace.c
> +++ b/fs/btrfs/dev-replace.c
> @@ -499,6 +499,7 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
>   	key.type = BTRFS_DEV_EXTENT_KEY;
>   	key.offset = 0;
>   
> +	down_read(&fs_info->commit_root_sem);
>   	btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
>   		struct extent_buffer *leaf = path->nodes[0];
>   
> @@ -525,6 +526,8 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
>   	if (iter_ret < 0)
>   		ret = iter_ret;
>   
> +	up_read(&fs_info->commit_root_sem);
> +
>   	btrfs_free_path(path);
>   unlock:
>   	mutex_unlock(&fs_info->chunk_mutex);

Re: [PATCH] btrfs: fix use-after-free in mark_block_group_to_copy

[Qu Wenruo]

在 2026/8/17 12:15, Hongling Zeng 写道:
>    Thanks for the review.
> 
>    This patch was developed with assistance from Claude AI.
> 
>    The AI helped with:
>    - Analyzing the code paths to understand the use-after-free scenario
>    - Identifying potential deadlock risks with different locking approaches
>    - Cross-referencing similar locking patterns in send.c and backref.c
>    - Drafting the commit message
> 
>    I modified the code to add the commit_root_sem locking myself, reviewed and
>    tested the changes before submission.
> 
>    Apologies for not disclosing AI assistance earlier. I will ensure to do so in
>    future submissions.
> 
>    Best regards,
>    Hongling Zeng
> 
> 在 2026年08月17日 09:46, Hongling Zeng 写道:
>> mark_block_group_to_copy() iterates over the commit root with
>> skip_locking=true to avoid lock contention. Without holding
>> commit_root_sem,a concurrent transaction commit can swap and free
>> the commit root during iteration, leading to use-after-free when
>> accessing extent buffers.
>>
>> The fix adds commit_root_sem locking, but CRITICALLY must only cover the
>> actual search period (btrfs_for_each_slot), NOT the preceding while loop
>> which calls btrfs_commit_transaction(). If held during that loop, we get
>> self-deadlock: down_read(commit_root_sem) → btrfs_commit_transaction()
>> → switch_commit_roots() → down_write(commit_root_sem), blocking forever
>> on our own read lock.
>>
>> Lock scope:
>> - down_read() after path setup, before btrfs_for_each_slot
>> - up_read() immediately after btrfs_for_each_slot completes
>> - NOT held during while loop (btrfs_commit_transaction) path
>> - NOT held on the !path error path (goto unlock before lock
>>    acquisition)
>>
>> This matches the established btrfs pattern used in send.c, backref.c, etc.:
>> hold commit_root_sem read lock ONLY while searching commit root, never
>> across transaction commits.
>>
>> Fixes: 78ce9fc269af ("btrfs: zoned: mark block groups to copy for device-replace")
>> Cc:stable@vger.kernel.org
>> Signed-off-by: Hongling Zeng<zenghongling@kylinos.cn>
>> ---
>>   fs/btrfs/dev-replace.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
>> index dc0834f920c3..734807bf48b3 100644
>> --- a/fs/btrfs/dev-replace.c
>> +++ b/fs/btrfs/dev-replace.c
>> @@ -499,6 +499,7 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
>>   	key.type = BTRFS_DEV_EXTENT_KEY;
>>   	key.offset = 0;
>>   
>> +	down_read(&fs_info->commit_root_sem);

And there is already path::need_commit_sem, which does a smaller 
critical section than holding it manually.

>>   	btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
>>   		struct extent_buffer *leaf = path->nodes[0];
>>   
>> @@ -525,6 +526,8 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
>>   	if (iter_ret < 0)
>>   		ret = iter_ret;
>>   
>> +	up_read(&fs_info->commit_root_sem);
>> +
>>   	btrfs_free_path(path);
>>   unlock:
>>   	mutex_unlock(&fs_info->chunk_mutex);
> 

Re: [PATCH] btrfs: fix use-after-free in mark_block_group_to_copy

[Hongling Zeng]

在 2026年08月17日 11:09, Qu Wenruo 写道:
>
>
> 在 2026/8/17 12:15, Hongling Zeng 写道:
>>    Thanks for the review.
>>
>>    This patch was developed with assistance from Claude AI.
>>
>>    The AI helped with:
>>    - Analyzing the code paths to understand the use-after-free scenario
>>    - Identifying potential deadlock risks with different locking 
>> approaches
>>    - Cross-referencing similar locking patterns in send.c and backref.c
>>    - Drafting the commit message
>>
>>    I modified the code to add the commit_root_sem locking myself, 
>> reviewed and
>>    tested the changes before submission.
>>
>>    Apologies for not disclosing AI assistance earlier. I will ensure 
>> to do so in
>>    future submissions.
>>
>>    Best regards,
>>    Hongling Zeng
>>
>> 在 2026年08月17日 09:46, Hongling Zeng 写道:
>>> mark_block_group_to_copy() iterates over the commit root with
>>> skip_locking=true to avoid lock contention. Without holding
>>> commit_root_sem,a concurrent transaction commit can swap and free
>>> the commit root during iteration, leading to use-after-free when
>>> accessing extent buffers.
>>>
>>> The fix adds commit_root_sem locking, but CRITICALLY must only cover 
>>> the
>>> actual search period (btrfs_for_each_slot), NOT the preceding while 
>>> loop
>>> which calls btrfs_commit_transaction(). If held during that loop, we 
>>> get
>>> self-deadlock: down_read(commit_root_sem) → btrfs_commit_transaction()
>>> → switch_commit_roots() → down_write(commit_root_sem), blocking forever
>>> on our own read lock.
>>>
>>> Lock scope:
>>> - down_read() after path setup, before btrfs_for_each_slot
>>> - up_read() immediately after btrfs_for_each_slot completes
>>> - NOT held during while loop (btrfs_commit_transaction) path
>>> - NOT held on the !path error path (goto unlock before lock
>>>    acquisition)
>>>
>>> This matches the established btrfs pattern used in send.c, 
>>> backref.c, etc.:
>>> hold commit_root_sem read lock ONLY while searching commit root, never
>>> across transaction commits.
>>>
>>> Fixes: 78ce9fc269af ("btrfs: zoned: mark block groups to copy for 
>>> device-replace")
>>> Cc:stable@vger.kernel.org
>>> Signed-off-by: Hongling Zeng<zenghongling@kylinos.cn>
>>> ---
>>>   fs/btrfs/dev-replace.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
>>> index dc0834f920c3..734807bf48b3 100644
>>> --- a/fs/btrfs/dev-replace.c
>>> +++ b/fs/btrfs/dev-replace.c
>>> @@ -499,6 +499,7 @@ static int mark_block_group_to_copy(struct 
>>> btrfs_fs_info *fs_info,
>>>       key.type = BTRFS_DEV_EXTENT_KEY;
>>>       key.offset = 0;
>>>   +    down_read(&fs_info->commit_root_sem);
>
> And there is already path::need_commit_sem, which does a smaller 
> critical section than holding it manually.
   Thanks for the review.

   I've updated the patch to use path->need_commit_sem as suggested. This is
   indeed a better approach than manual commit_root_sem locking.

   V2 patch is attached.

   Best regards,
   Hongling Zeng

>
>>>       btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
>>>           struct extent_buffer *leaf = path->nodes[0];
>>>   @@ -525,6 +526,8 @@ static int mark_block_group_to_copy(struct 
>>> btrfs_fs_info *fs_info,
>>>       if (iter_ret < 0)
>>>           ret = iter_ret;
>>>   +    up_read(&fs_info->commit_root_sem);
>>> +
>>>       btrfs_free_path(path);
>>>   unlock:
>>>       mutex_unlock(&fs_info->chunk_mutex);
>>