Re: [PATCH] btrfs: fix bio counter and bioc leak when async checksumming fails

[Qu Wenruo <wqu@suse.com>]

在 2026/11/2 07:08, Qu Wenruo 写道:
> 
> 
> 在 2026/11/2 05:09, Mani Tofigh via B4 Relay 写道:
>> From: Mani Tofigh <mt4022@columbia.edu>
>>
>> A single failed checksum allocation can make a later device replace
>> hang forever.  btrfs_submit_chunk() takes a dev-replace bio counter
>> reference and a bioc before it hands a write to the checksum worker.
>> If the checksum fails there (eg its allocation fails),
>> run_one_async_done() ends the bio but drops neither.  The counter then
>> never gets back to zero and the next replace waits for it:
>>
>>    [    8.369171] BTRFS info (device ram0): dev_replace from /dev/ram0 
>> (devid 1) to /dev/ram1 finished
>>    [   41.534247] INFO: task btrfscsum:80 blocked for more than 20 
>> seconds.
>>    [   41.534391]  btrfs_rm_dev_replace_blocked+0x135/0x180
>>    [   41.534463]  btrfs_dev_replace_finishing+0xefe/0x13b0
>>
>> Put the bioc and drop the counter in that error branch as the
>> synchronous error path does.  Also put the bioc on the synchronous
>> path, which only drops the counter.  With the fix, the replace
>> finishes.
>>
>> This bug was identified and confirmed with the help of fault-injection
>> testing on a deterministic hypervisor currently under development.  The
>> C reproducer below however hits it within about 35s.
>>
>> Fixes: f8a53bb58ec7 ("btrfs: handle checksum generation in the storage 
>> layer")
>> Cc: stable@vger.kernel.org # 6.6+
>> Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
> 
> Reviewed-by: Qu Wenruo <wqu@suse.com>
> 
> Although it's not that common to hit this path.
> 
> On EXPERIMENTAL builds you are already running with async csum 
> generation, other than the offload path.
> 
> Otherwise on most x86_64, the default CRC32 is considered fast, so is 
> xxhash on all archs, and they will not go through the offload mode.
> 
> So I'm wondering what's your setup? Are you running SHA256 algo for 
> checksum? I can add it to the commit message during merge.

And Sashiko exposed an existing problem, even with this patch applied, 
the error handling of btrfs_split_bio() is not going to release the bioc.

https://sashiko.dev/#/map/223655

Although it's an existing bug, since you're touching all the bioc 
releasing, it would be better to handle them all in one patch.

Thanks,
Qu

> 
>> ---
>> Reproducer: https://github.com/manitofigh/files/blob/main/linux- 
>> repros/btrfscsum.c
>> ---
>>   fs/btrfs/bio.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
>> index 2c6234f6182f..a508f4abd3a3 100644
>> --- a/fs/btrfs/bio.c
>> +++ b/fs/btrfs/bio.c
>> @@ -624,6 +624,8 @@ static void run_one_async_done(struct btrfs_work 
>> *work, bool do_free)
>>       /* If an error occurred we just want to clean up the bio and 
>> move on. */
>>       if (bio->bi_status) {
>> +        btrfs_put_bioc(async->bioc);
>> +        btrfs_bio_counter_dec(async->bbio->inode->root->fs_info);
>>           btrfs_bio_end_io(async->bbio, bio->bi_status);
>>           return;
>>       }
>> @@ -822,6 +824,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio 
>> *bbio, int mirror_num)
>>       return map_length == length;
>>   fail:
>> +    btrfs_put_bioc(bioc);
>>       btrfs_bio_counter_dec(fs_info);
>>       /*
>>        * We have split the original bbio, now we have to end both the 
>> current
>>
>> ---
>> base-commit: 5e6c45aaaf93343c7f8a1cde9797080238af5c1b
>> change-id: 20261030-btrfs-async-csum-leak-1772df5f0c32
>>
>> Best regards,
>> -- 
>> Mani Tofigh <mt4022@columbia.edu>
>