* [PATCH] btrfs: free checksum hash on in close_ctree
@ 2019-07-11 15:23 Johannes Thumshirn
2019-07-12 7:34 ` Qu Wenruo
2019-07-17 14:56 ` David Sterba
0 siblings, 2 replies; 5+ messages in thread
From: Johannes Thumshirn @ 2019-07-11 15:23 UTC (permalink / raw)
To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn
fs_info::csum_hash gets initialized in btrfs_init_csum_hash() which is
called by open_ctree().
But it only gets freed if open_ctree() fails, not on normal operation.
This leads to a memory leak like the following found by kmemleak:
unreferenced object 0xffff888132cb8720 (size 96):
comm "mount", pid 450, jiffies 4294912436 (age 17.584s)
hex dump (first 32 bytes):
04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<000000000c9643d4>] crypto_create_tfm+0x2d/0xd0
[<00000000ae577f68>] crypto_alloc_tfm+0x4b/0xb0
[<000000002b5cdf30>] open_ctree+0xb84/0x2060 [btrfs]
[<0000000043204297>] btrfs_mount_root+0x552/0x640 [btrfs]
[<00000000c99b10ea>] legacy_get_tree+0x22/0x40
[<0000000071a6495f>] vfs_get_tree+0x1f/0xc0
[<00000000f180080e>] fc_mount+0x9/0x30
[<000000009e36cebd>] vfs_kern_mount.part.11+0x6a/0x80
[<0000000004594c05>] btrfs_mount+0x174/0x910 [btrfs]
[<00000000c99b10ea>] legacy_get_tree+0x22/0x40
[<0000000071a6495f>] vfs_get_tree+0x1f/0xc0
[<00000000b86e92c5>] do_mount+0x6b0/0x940
[<0000000097464494>] ksys_mount+0x7b/0xd0
[<0000000057213c80>] __x64_sys_mount+0x1c/0x20
[<00000000cb689b5e>] do_syscall_64+0x43/0x130
[<000000002194e289>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
Free fs_info::csum_hash in close_ctree() to avoid the memory leak.
Fixes: 6d97c6e31b55 ("btrfs: add boilerplate code for directly including the crypto framework")
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
fs/btrfs/disk-io.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 41a2bd2e0c56..5f7ee70b3d1a 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -4106,6 +4106,7 @@ void close_ctree(struct btrfs_fs_info *fs_info)
percpu_counter_destroy(&fs_info->dev_replace.bio_counter);
cleanup_srcu_struct(&fs_info->subvol_srcu);
+ btrfs_free_csum_hash(fs_info);
btrfs_free_stripe_hash_table(fs_info);
btrfs_free_ref_cache(fs_info);
}
--
2.16.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: free checksum hash on in close_ctree
2019-07-11 15:23 [PATCH] btrfs: free checksum hash on in close_ctree Johannes Thumshirn
@ 2019-07-12 7:34 ` Qu Wenruo
2019-07-12 9:21 ` Johannes Thumshirn
2019-07-17 14:56 ` David Sterba
1 sibling, 1 reply; 5+ messages in thread
From: Qu Wenruo @ 2019-07-12 7:34 UTC (permalink / raw)
To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist
[-- Attachment #1.1: Type: text/plain, Size: 2519 bytes --]
On 2019/7/11 下午11:23, Johannes Thumshirn wrote:
> fs_info::csum_hash gets initialized in btrfs_init_csum_hash() which is
> called by open_ctree().
>
> But it only gets freed if open_ctree() fails, not on normal operation.
>
> This leads to a memory leak like the following found by kmemleak:
> unreferenced object 0xffff888132cb8720 (size 96):
> comm "mount", pid 450, jiffies 4294912436 (age 17.584s)
> hex dump (first 32 bytes):
> 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<000000000c9643d4>] crypto_create_tfm+0x2d/0xd0
> [<00000000ae577f68>] crypto_alloc_tfm+0x4b/0xb0
> [<000000002b5cdf30>] open_ctree+0xb84/0x2060 [btrfs]
> [<0000000043204297>] btrfs_mount_root+0x552/0x640 [btrfs]
> [<00000000c99b10ea>] legacy_get_tree+0x22/0x40
> [<0000000071a6495f>] vfs_get_tree+0x1f/0xc0
> [<00000000f180080e>] fc_mount+0x9/0x30
> [<000000009e36cebd>] vfs_kern_mount.part.11+0x6a/0x80
> [<0000000004594c05>] btrfs_mount+0x174/0x910 [btrfs]
> [<00000000c99b10ea>] legacy_get_tree+0x22/0x40
> [<0000000071a6495f>] vfs_get_tree+0x1f/0xc0
> [<00000000b86e92c5>] do_mount+0x6b0/0x940
> [<0000000097464494>] ksys_mount+0x7b/0xd0
> [<0000000057213c80>] __x64_sys_mount+0x1c/0x20
> [<00000000cb689b5e>] do_syscall_64+0x43/0x130
> [<000000002194e289>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> Free fs_info::csum_hash in close_ctree() to avoid the memory leak.
>
> Fixes: 6d97c6e31b55 ("btrfs: add boilerplate code for directly including the crypto framework")
Not yet in upstream, thus I believe David could just fold this fix into
the original commit.
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Although for the folding case, that reviewed-by won't make much sense.
Thanks,
Qu
> ---
> fs/btrfs/disk-io.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 41a2bd2e0c56..5f7ee70b3d1a 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -4106,6 +4106,7 @@ void close_ctree(struct btrfs_fs_info *fs_info)
> percpu_counter_destroy(&fs_info->dev_replace.bio_counter);
> cleanup_srcu_struct(&fs_info->subvol_srcu);
>
> + btrfs_free_csum_hash(fs_info);
> btrfs_free_stripe_hash_table(fs_info);
> btrfs_free_ref_cache(fs_info);
> }
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: free checksum hash on in close_ctree
2019-07-12 7:34 ` Qu Wenruo
@ 2019-07-12 9:21 ` Johannes Thumshirn
2019-07-17 14:02 ` David Sterba
0 siblings, 1 reply; 5+ messages in thread
From: Johannes Thumshirn @ 2019-07-12 9:21 UTC (permalink / raw)
To: Qu Wenruo; +Cc: David Sterba, Linux BTRFS Mailinglist
On Fri, Jul 12, 2019 at 03:34:45PM +0800, Qu Wenruo wrote:
> Not yet in upstream, thus I believe David could just fold this fix into
> the original commit.
Right, I just didn't know if David's gonna rebase his branch before the pull
request. AFAIK Linus doesn't really like recently rebased branches.
>
> Reviewed-by: Qu Wenruo <wqu@suse.com>
>
> Although for the folding case, that reviewed-by won't make much sense.
Thanks anyways,
Johannes
--
Johannes Thumshirn SUSE Labs Filesystems
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: free checksum hash on in close_ctree
2019-07-12 9:21 ` Johannes Thumshirn
@ 2019-07-17 14:02 ` David Sterba
0 siblings, 0 replies; 5+ messages in thread
From: David Sterba @ 2019-07-17 14:02 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: Qu Wenruo, David Sterba, Linux BTRFS Mailinglist
On Fri, Jul 12, 2019 at 11:21:38AM +0200, Johannes Thumshirn wrote:
> On Fri, Jul 12, 2019 at 03:34:45PM +0800, Qu Wenruo wrote:
> > Not yet in upstream, thus I believe David could just fold this fix into
> > the original commit.
>
> Right, I just didn't know if David's gonna rebase his branch before the pull
> request. AFAIK Linus doesn't really like recently rebased branches.
The branch for pull request is frozen at least a week before the merge
window starts and in exceptional and justified cases some changes are
possible, but certainly not rebaasing half of the branch.
As we found out, there are several problems with leaks, some of them
known before the merge window, but we have the rc milestones to fix
that.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: free checksum hash on in close_ctree
2019-07-11 15:23 [PATCH] btrfs: free checksum hash on in close_ctree Johannes Thumshirn
2019-07-12 7:34 ` Qu Wenruo
@ 2019-07-17 14:56 ` David Sterba
1 sibling, 0 replies; 5+ messages in thread
From: David Sterba @ 2019-07-17 14:56 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: David Sterba, Linux BTRFS Mailinglist
On Thu, Jul 11, 2019 at 05:23:04PM +0200, Johannes Thumshirn wrote:
> fs_info::csum_hash gets initialized in btrfs_init_csum_hash() which is
> called by open_ctree().
>
> But it only gets freed if open_ctree() fails, not on normal operation.
>
> This leads to a memory leak like the following found by kmemleak:
> unreferenced object 0xffff888132cb8720 (size 96):
> comm "mount", pid 450, jiffies 4294912436 (age 17.584s)
> hex dump (first 32 bytes):
> 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<000000000c9643d4>] crypto_create_tfm+0x2d/0xd0
> [<00000000ae577f68>] crypto_alloc_tfm+0x4b/0xb0
> [<000000002b5cdf30>] open_ctree+0xb84/0x2060 [btrfs]
> [<0000000043204297>] btrfs_mount_root+0x552/0x640 [btrfs]
> [<00000000c99b10ea>] legacy_get_tree+0x22/0x40
> [<0000000071a6495f>] vfs_get_tree+0x1f/0xc0
> [<00000000f180080e>] fc_mount+0x9/0x30
> [<000000009e36cebd>] vfs_kern_mount.part.11+0x6a/0x80
> [<0000000004594c05>] btrfs_mount+0x174/0x910 [btrfs]
> [<00000000c99b10ea>] legacy_get_tree+0x22/0x40
> [<0000000071a6495f>] vfs_get_tree+0x1f/0xc0
> [<00000000b86e92c5>] do_mount+0x6b0/0x940
> [<0000000097464494>] ksys_mount+0x7b/0xd0
> [<0000000057213c80>] __x64_sys_mount+0x1c/0x20
> [<00000000cb689b5e>] do_syscall_64+0x43/0x130
> [<000000002194e289>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> Free fs_info::csum_hash in close_ctree() to avoid the memory leak.
>
> Fixes: 6d97c6e31b55 ("btrfs: add boilerplate code for directly including the crypto framework")
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Added to 5.3 queue, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-17 14:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-11 15:23 [PATCH] btrfs: free checksum hash on in close_ctree Johannes Thumshirn
2019-07-12 7:34 ` Qu Wenruo
2019-07-12 9:21 ` Johannes Thumshirn
2019-07-17 14:02 ` David Sterba
2019-07-17 14:56 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).