[PATCH] btrfs: scrub: fix memory leak in scrub_raid56_parity_stripe()

Zilin Guan posted 1 patch 1 month, 2 weeks ago
fs/btrfs/scrub.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] btrfs: scrub: fix memory leak in scrub_raid56_parity_stripe()
Posted by Zilin Guan 1 month, 2 weeks ago
scrub_raid56_parity_stripe() allocates a bio with bio_alloc(), but
fails to release it on some error paths, leading to a potential
memory leak.

Add the missing bio_put() calls to properly drop the bio reference
in those error cases.

Fixes: 1009254bf22a3 ("btrfs: scrub: use scrub_stripe to implement RAID56 P/Q scrub")
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
---
 fs/btrfs/scrub.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 651b11884f82..ba20d9286a34 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -2203,6 +2203,7 @@ static int scrub_raid56_parity_stripe(struct scrub_ctx *sctx,
 	ret = btrfs_map_block(fs_info, BTRFS_MAP_WRITE, full_stripe_start,
 			      &length, &bioc, NULL, NULL);
 	if (ret < 0) {
+		bio_put(bio);
 		btrfs_put_bioc(bioc);
 		btrfs_bio_counter_dec(fs_info);
 		goto out;
@@ -2212,6 +2213,7 @@ static int scrub_raid56_parity_stripe(struct scrub_ctx *sctx,
 	btrfs_put_bioc(bioc);
 	if (!rbio) {
 		ret = -ENOMEM;
+		bio_put(bio);
 		btrfs_bio_counter_dec(fs_info);
 		goto out;
 	}
-- 
2.34.1
Re: [PATCH] btrfs: scrub: fix memory leak in scrub_raid56_parity_stripe()
Posted by Markus Elfring 1 month, 1 week ago
…
> Add the missing bio_put() calls to properly drop the bio reference
> in those error cases.

How do you think about to use additional labels?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v6.18-rc4#n526

Regards,
Markus
Re: [PATCH] btrfs: scrub: fix memory leak in scrub_raid56_parity_stripe()
Posted by Qu Wenruo 1 month, 1 week ago

在 2025/11/5 20:56, Markus Elfring 写道:
> …
>> Add the missing bio_put() calls to properly drop the bio reference
>> in those error cases.
> 
> How do you think about to use additional labels?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v6.18-rc4#n526

I believe the current hot fix is fine.

As in the long run we're going to use on-stack bio for this particular 
call site.

Thanks,
Qu

> 
> Regards,
> Markus
> 
Re: btrfs: scrub: fix memory leak in scrub_raid56_parity_stripe()
Posted by Markus Elfring 1 month, 1 week ago
>> …
>>> Add the missing bio_put() calls to properly drop the bio reference
>>> in those error cases.
>>
>> How do you think about to use additional labels?
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v6.18-rc4#n526
> 
> I believe the current hot fix is fine.
…

Would you like to care a bit more for avoidance of duplicate source code
(also according to better exception handling)?

Regards,
Markus
Re: btrfs: scrub: fix memory leak in scrub_raid56_parity_stripe()
Posted by Qu Wenruo 1 month, 1 week ago

在 2025/11/6 17:25, Markus Elfring 写道:
>>> …
>>>> Add the missing bio_put() calls to properly drop the bio reference
>>>> in those error cases.
>>>
>>> How do you think about to use additional labels?
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v6.18-rc4#n526
>>
>> I believe the current hot fix is fine.
> …
> 
> Would you like to care a bit more for avoidance of duplicate source code
> (also according to better exception handling)?

That is handled in this patch, a dedicated cleanup/refactor:

https://lore.kernel.org/linux-btrfs/2d2cfb7729a65d88ea8b9d6408611d0cc76e1ab7.1762398098.git.wqu@suse.com/T/#u

To be honest, doing that extra tag inside the original 
scrub_raid56_parity_stripe() is not that helpful, that function is doing 
a lot of different works, and @bio is only allocated for the last two 
stages (using cached stripes and to wait for the scrub of parity stripe).

Doing a dedicated tag for less than half of the code is not going to 
improve readability.

It's better to extract the unrelated behavior into a dedicated helper so 
that it's way straight forward.

Thanks,
Qu

> 
> Regards,
> Markus
> 
Re: btrfs: scrub: fix memory leak in scrub_raid56_parity_stripe()
Posted by Markus Elfring 1 month, 1 week ago
> That is handled in this patch, a dedicated cleanup/refactor:
> 
> https://lore.kernel.org/linux-btrfs/2d2cfb7729a65d88ea8b9d6408611d0cc76e1ab7.1762398098.git.wqu@suse.com/T/#u

Thanks for another bit of collateral evolution.
[PATCH] btrfs: extract the parity scrub code into a helper

Regards,
Markus
Re: [PATCH] btrfs: scrub: fix memory leak in scrub_raid56_parity_stripe()
Posted by Qu Wenruo 1 month, 2 weeks ago

在 2025/11/5 14:23, Zilin Guan 写道:
> scrub_raid56_parity_stripe() allocates a bio with bio_alloc(), but
> fails to release it on some error paths, leading to a potential
> memory leak.
> 
> Add the missing bio_put() calls to properly drop the bio reference
> in those error cases.
> 
> Fixes: 1009254bf22a3 ("btrfs: scrub: use scrub_stripe to implement RAID56 P/Q scrub")
> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Merged into for-next branch.

Thanks,
Qu
> ---
>   fs/btrfs/scrub.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 651b11884f82..ba20d9286a34 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -2203,6 +2203,7 @@ static int scrub_raid56_parity_stripe(struct scrub_ctx *sctx,
>   	ret = btrfs_map_block(fs_info, BTRFS_MAP_WRITE, full_stripe_start,
>   			      &length, &bioc, NULL, NULL);
>   	if (ret < 0) {
> +		bio_put(bio);
>   		btrfs_put_bioc(bioc);
>   		btrfs_bio_counter_dec(fs_info);
>   		goto out;
> @@ -2212,6 +2213,7 @@ static int scrub_raid56_parity_stripe(struct scrub_ctx *sctx,
>   	btrfs_put_bioc(bioc);
>   	if (!rbio) {
>   		ret = -ENOMEM;
> +		bio_put(bio);
>   		btrfs_bio_counter_dec(fs_info);
>   		goto out;
>   	}