2020-02-01 20:40:52

by Wenwen Wang

[permalink] [raw]
Subject: [PATCH] btrfs: ref-verify: fix memory leaks

In btrfs_ref_tree_mod(), 'ref' and 'ra' are allocated through kzalloc() and
kmalloc(), respectively. In the following code, if an error occurs, the
execution will be redirected to 'out' or 'out_unlock' and the function will
be exited. However, on some of the paths, 'ref' and 'ra' are not
deallocated, leading to memory leaks. For example, if 'action' is
BTRFS_ADD_DELAYED_EXTENT, add_block_entry() will be invoked. If the return
value indicates an error, the execution will be redirected to 'out'. But,
'ref' is not deallocated on this path, causing a memory leak.

To fix the above issues, deallocate both 'ref' and 'ra' before exiting from
the function when an error is encountered.

Signed-off-by: Wenwen Wang <[email protected]>
---
fs/btrfs/ref-verify.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/fs/btrfs/ref-verify.c b/fs/btrfs/ref-verify.c
index b57f3618e58e..454a1015d026 100644
--- a/fs/btrfs/ref-verify.c
+++ b/fs/btrfs/ref-verify.c
@@ -744,6 +744,7 @@ int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
*/
be = add_block_entry(fs_info, bytenr, num_bytes, ref_root);
if (IS_ERR(be)) {
+ kfree(ref);
kfree(ra);
ret = PTR_ERR(be);
goto out;
@@ -757,6 +758,8 @@ int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
"re-allocated a block that still has references to it!");
dump_block_entry(fs_info, be);
dump_ref_action(fs_info, ra);
+ kfree(ref);
+ kfree(ra);
goto out_unlock;
}

@@ -819,6 +822,7 @@ int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
"dropping a ref for a existing root that doesn't have a ref on the block");
dump_block_entry(fs_info, be);
dump_ref_action(fs_info, ra);
+ kfree(ref);
kfree(ra);
goto out_unlock;
}
@@ -834,6 +838,7 @@ int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
"attempting to add another ref for an existing ref on a tree block");
dump_block_entry(fs_info, be);
dump_ref_action(fs_info, ra);
+ kfree(ref);
kfree(ra);
goto out_unlock;
}
--
2.17.1


2020-02-04 16:16:20

by David Sterba

[permalink] [raw]
Subject: Re: [PATCH] btrfs: ref-verify: fix memory leaks

On Sat, Feb 01, 2020 at 08:38:38PM +0000, Wenwen Wang wrote:
> In btrfs_ref_tree_mod(), 'ref' and 'ra' are allocated through kzalloc() and
> kmalloc(), respectively. In the following code, if an error occurs, the
> execution will be redirected to 'out' or 'out_unlock' and the function will
> be exited. However, on some of the paths, 'ref' and 'ra' are not
> deallocated, leading to memory leaks. For example, if 'action' is
> BTRFS_ADD_DELAYED_EXTENT, add_block_entry() will be invoked. If the return
> value indicates an error, the execution will be redirected to 'out'. But,
> 'ref' is not deallocated on this path, causing a memory leak.
>
> To fix the above issues, deallocate both 'ref' and 'ra' before exiting from
> the function when an error is encountered.

Right, the kfrees are missing. The whole function needs to be
restructured otherwise it's too easy to get lost in the conditions and
what needs to be cleaned up after errors but that's for a separate
patch. Added to devel queue, thanks.