* [PATCH] btrfs-progs: delayed-ref: Fix memory leak and use-after-free caused by wrong condition to free delayed ref/head.
@ 2019-06-13 7:33 Qu Wenruo
2019-06-19 6:16 ` Nikolay Borisov
0 siblings, 1 reply; 4+ messages in thread
From: Qu Wenruo @ 2019-06-13 7:33 UTC (permalink / raw)
To: linux-btrfs
[BUG]
When btrfs-progs is compiled with D=asan, it can't pass even the very
basic fsck tests due to btrfs-image has memory leak:
=== START TEST /home/adam/btrfs/btrfs-progs/tests//fsck-tests/001-bad-file-extent-bytenr
restoring image default_case.img
=================================================================
==7790==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 104 byte(s) in 1 object(s) allocated from:
#0 0x7f1d3b738389 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:86
#1 0x560ca6b7f4ff in btrfs_add_delayed_tree_ref /home/adam/btrfs/btrfs-progs/delayed-ref.c:569
#2 0x560ca6af2d0b in btrfs_free_extent /home/adam/btrfs/btrfs-progs/extent-tree.c:2155
#3 0x560ca6ac16ca in __btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:319
#4 0x560ca6ac1d8c in btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:383
#5 0x560ca6ac6c8e in btrfs_search_slot /home/adam/btrfs/btrfs-progs/ctree.c:1153
#6 0x560ca6ab7e83 in fixup_device_size image/main.c:2113
#7 0x560ca6ab9279 in fixup_chunks_and_devices image/main.c:2333
#8 0x560ca6ab9ada in restore_metadump image/main.c:2455
#9 0x560ca6abaeba in main image/main.c:2723
#10 0x7f1d3b148ce2 in __libc_start_main (/usr/lib/libc.so.6+0x23ce2)
... tons of similar leakage for delayed_tree_ref ...
Direct leak of 96 byte(s) in 1 object(s) allocated from:
#0 0x7f1d3b738389 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:86
#1 0x560ca6b7f5fb in btrfs_add_delayed_tree_ref /home/adam/btrfs/btrfs-progs/delayed-ref.c:583
#2 0x560ca6af5679 in alloc_tree_block /home/adam/btrfs/btrfs-progs/extent-tree.c:2503
#3 0x560ca6af57ac in btrfs_alloc_free_block /home/adam/btrfs/btrfs-progs/extent-tree.c:2524
#4 0x560ca6ac115b in __btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:290
#5 0x560ca6ac1d8c in btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:383
#6 0x560ca6b7bb15 in commit_tree_roots /home/adam/btrfs/btrfs-progs/transaction.c:98
#7 0x560ca6b7c525 in btrfs_commit_transaction /home/adam/btrfs/btrfs-progs/transaction.c:192
#8 0x560ca6ab92be in fixup_chunks_and_devices image/main.c:2337
#9 0x560ca6ab9ada in restore_metadump image/main.c:2455
#10 0x560ca6abaeba in main image/main.c:2723
#11 0x7f1d3b148ce2 in __libc_start_main (/usr/lib/libc.so.6+0x23ce2)
... tons of similar leakage for delayed_ref_head ...
SUMMARY: AddressSanitizer: 1600 byte(s) leaked in 16 allocation(s).
failed to restore image ./default_case.img
[CAUSE]
Commit c6039704c580 ("btrfs-progs: Add delayed refs infrastructure")
introduces delayed ref infrastructure for free space tree, however the
refcount_dec_and_test() from kernel code is wrongly backported.
refcount_dec_and_test() will return true if the refcount reaches 0.
So kernel code will free the allocated space as expected:
if (refcount_dec_and_test(&ref->refs)) {
kmem_cache_free();
}
However btrfs-progs backport is using the opposite condition:
if (--ref->refs) {
kfree();
}
This will not free the memory for the last user, but for refs >= 2.
Causing both use-after-free and memory leak for any offline write
operation.
[FIX]
Fix the (--ref->refs) condition to (--ref->refs == 0) to fix the
backport error.
Fixes: c6039704c580 ("btrfs-progs: Add delayed refs infrastructure")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
delayed-ref.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/delayed-ref.h b/delayed-ref.h
index efc855eff621..2ccfd27c2b95 100644
--- a/delayed-ref.h
+++ b/delayed-ref.h
@@ -161,7 +161,7 @@ btrfs_free_delayed_extent_op(struct btrfs_delayed_extent_op *op)
static inline void btrfs_put_delayed_ref(struct btrfs_delayed_ref_node *ref)
{
WARN_ON(ref->refs == 0);
- if (--ref->refs) {
+ if (--ref->refs == 0) {
WARN_ON(ref->in_tree);
switch (ref->type) {
case BTRFS_TREE_BLOCK_REF_KEY:
@@ -180,7 +180,7 @@ static inline void btrfs_put_delayed_ref(struct btrfs_delayed_ref_node *ref)
static inline void btrfs_put_delayed_ref_head(struct btrfs_delayed_ref_head *head)
{
- if (--head->refs)
+ if (--head->refs == 0)
kfree(head);
}
--
2.22.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs-progs: delayed-ref: Fix memory leak and use-after-free caused by wrong condition to free delayed ref/head.
2019-06-13 7:33 [PATCH] btrfs-progs: delayed-ref: Fix memory leak and use-after-free caused by wrong condition to free delayed ref/head Qu Wenruo
@ 2019-06-19 6:16 ` Nikolay Borisov
0 siblings, 0 replies; 4+ messages in thread
From: Nikolay Borisov @ 2019-06-19 6:16 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 13.06.19 г. 10:33 ч., Qu Wenruo wrote:
> [BUG]
> When btrfs-progs is compiled with D=asan, it can't pass even the very
> basic fsck tests due to btrfs-image has memory leak:
> === START TEST /home/adam/btrfs/btrfs-progs/tests//fsck-tests/001-bad-file-extent-bytenr
> restoring image default_case.img
>
> =================================================================
> ==7790==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 104 byte(s) in 1 object(s) allocated from:
> #0 0x7f1d3b738389 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:86
> #1 0x560ca6b7f4ff in btrfs_add_delayed_tree_ref /home/adam/btrfs/btrfs-progs/delayed-ref.c:569
> #2 0x560ca6af2d0b in btrfs_free_extent /home/adam/btrfs/btrfs-progs/extent-tree.c:2155
> #3 0x560ca6ac16ca in __btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:319
> #4 0x560ca6ac1d8c in btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:383
> #5 0x560ca6ac6c8e in btrfs_search_slot /home/adam/btrfs/btrfs-progs/ctree.c:1153
> #6 0x560ca6ab7e83 in fixup_device_size image/main.c:2113
> #7 0x560ca6ab9279 in fixup_chunks_and_devices image/main.c:2333
> #8 0x560ca6ab9ada in restore_metadump image/main.c:2455
> #9 0x560ca6abaeba in main image/main.c:2723
> #10 0x7f1d3b148ce2 in __libc_start_main (/usr/lib/libc.so.6+0x23ce2)
>
> ... tons of similar leakage for delayed_tree_ref ...
>
> Direct leak of 96 byte(s) in 1 object(s) allocated from:
> #0 0x7f1d3b738389 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:86
> #1 0x560ca6b7f5fb in btrfs_add_delayed_tree_ref /home/adam/btrfs/btrfs-progs/delayed-ref.c:583
> #2 0x560ca6af5679 in alloc_tree_block /home/adam/btrfs/btrfs-progs/extent-tree.c:2503
> #3 0x560ca6af57ac in btrfs_alloc_free_block /home/adam/btrfs/btrfs-progs/extent-tree.c:2524
> #4 0x560ca6ac115b in __btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:290
> #5 0x560ca6ac1d8c in btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:383
> #6 0x560ca6b7bb15 in commit_tree_roots /home/adam/btrfs/btrfs-progs/transaction.c:98
> #7 0x560ca6b7c525 in btrfs_commit_transaction /home/adam/btrfs/btrfs-progs/transaction.c:192
> #8 0x560ca6ab92be in fixup_chunks_and_devices image/main.c:2337
> #9 0x560ca6ab9ada in restore_metadump image/main.c:2455
> #10 0x560ca6abaeba in main image/main.c:2723
> #11 0x7f1d3b148ce2 in __libc_start_main (/usr/lib/libc.so.6+0x23ce2)
>
> ... tons of similar leakage for delayed_ref_head ...
>
> SUMMARY: AddressSanitizer: 1600 byte(s) leaked in 16 allocation(s).
> failed to restore image ./default_case.img
>
> [CAUSE]
> Commit c6039704c580 ("btrfs-progs: Add delayed refs infrastructure")
> introduces delayed ref infrastructure for free space tree, however the
> refcount_dec_and_test() from kernel code is wrongly backported.
>
> refcount_dec_and_test() will return true if the refcount reaches 0.
> So kernel code will free the allocated space as expected:
> if (refcount_dec_and_test(&ref->refs)) {
> kmem_cache_free();
> }
>
> However btrfs-progs backport is using the opposite condition:
> if (--ref->refs) {
> kfree();
> }
>
> This will not free the memory for the last user, but for refs >= 2.
> Causing both use-after-free and memory leak for any offline write
> operation.
>
> [FIX]
> Fix the (--ref->refs) condition to (--ref->refs == 0) to fix the
> backport error.
>
> Fixes: c6039704c580 ("btrfs-progs: Add delayed refs infrastructure")
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> delayed-ref.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/delayed-ref.h b/delayed-ref.h
> index efc855eff621..2ccfd27c2b95 100644
> --- a/delayed-ref.h
> +++ b/delayed-ref.h
> @@ -161,7 +161,7 @@ btrfs_free_delayed_extent_op(struct btrfs_delayed_extent_op *op)
> static inline void btrfs_put_delayed_ref(struct btrfs_delayed_ref_node *ref)
> {
> WARN_ON(ref->refs == 0);
> - if (--ref->refs) {
> + if (--ref->refs == 0) {
> WARN_ON(ref->in_tree);
> switch (ref->type) {
> case BTRFS_TREE_BLOCK_REF_KEY:
> @@ -180,7 +180,7 @@ static inline void btrfs_put_delayed_ref(struct btrfs_delayed_ref_node *ref)
>
> static inline void btrfs_put_delayed_ref_head(struct btrfs_delayed_ref_head *head)
> {
> - if (--head->refs)
> + if (--head->refs == 0)
> kfree(head);
> }
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs-progs: delayed-ref: Fix memory leak and use-after-free caused by wrong condition to free delayed ref/head.
2019-06-13 7:49 Qu Wenruo
@ 2019-06-14 16:03 ` David Sterba
0 siblings, 0 replies; 4+ messages in thread
From: David Sterba @ 2019-06-14 16:03 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Thu, Jun 13, 2019 at 03:49:59PM +0800, Qu Wenruo wrote:
> [BUG]
> When btrfs-progs is compiled with D=asan, it can't pass even the very
> basic fsck tests due to btrfs-image has memory leak:
> === START TEST /home/adam/btrfs/btrfs-progs/tests//fsck-tests/001-bad-file-extent-bytenr
> restoring image default_case.img
>
> =================================================================
> ==7790==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 104 byte(s) in 1 object(s) allocated from:
> #0 0x7f1d3b738389 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:86
> #1 0x560ca6b7f4ff in btrfs_add_delayed_tree_ref /home/adam/btrfs/btrfs-progs/delayed-ref.c:569
> #2 0x560ca6af2d0b in btrfs_free_extent /home/adam/btrfs/btrfs-progs/extent-tree.c:2155
> #3 0x560ca6ac16ca in __btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:319
> #4 0x560ca6ac1d8c in btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:383
> #5 0x560ca6ac6c8e in btrfs_search_slot /home/adam/btrfs/btrfs-progs/ctree.c:1153
> #6 0x560ca6ab7e83 in fixup_device_size image/main.c:2113
> #7 0x560ca6ab9279 in fixup_chunks_and_devices image/main.c:2333
> #8 0x560ca6ab9ada in restore_metadump image/main.c:2455
> #9 0x560ca6abaeba in main image/main.c:2723
> #10 0x7f1d3b148ce2 in __libc_start_main (/usr/lib/libc.so.6+0x23ce2)
>
> ... tons of similar leakage for delayed_tree_ref ...
>
> Direct leak of 96 byte(s) in 1 object(s) allocated from:
> #0 0x7f1d3b738389 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:86
> #1 0x560ca6b7f5fb in btrfs_add_delayed_tree_ref /home/adam/btrfs/btrfs-progs/delayed-ref.c:583
> #2 0x560ca6af5679 in alloc_tree_block /home/adam/btrfs/btrfs-progs/extent-tree.c:2503
> #3 0x560ca6af57ac in btrfs_alloc_free_block /home/adam/btrfs/btrfs-progs/extent-tree.c:2524
> #4 0x560ca6ac115b in __btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:290
> #5 0x560ca6ac1d8c in btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:383
> #6 0x560ca6b7bb15 in commit_tree_roots /home/adam/btrfs/btrfs-progs/transaction.c:98
> #7 0x560ca6b7c525 in btrfs_commit_transaction /home/adam/btrfs/btrfs-progs/transaction.c:192
> #8 0x560ca6ab92be in fixup_chunks_and_devices image/main.c:2337
> #9 0x560ca6ab9ada in restore_metadump image/main.c:2455
> #10 0x560ca6abaeba in main image/main.c:2723
> #11 0x7f1d3b148ce2 in __libc_start_main (/usr/lib/libc.so.6+0x23ce2)
>
> ... tons of similar leakage for delayed_ref_head ...
>
> SUMMARY: AddressSanitizer: 1600 byte(s) leaked in 16 allocation(s).
> failed to restore image ./default_case.img
>
> [CAUSE]
> Commit c6039704c580 ("btrfs-progs: Add delayed refs infrastructure")
> introduces delayed ref infrastructure for free space tree, however the
> refcount_dec_and_test() from kernel code is wrongly backported.
>
> refcount_dec_and_test() will return true if the refcount reaches 0.
> So kernel code will free the allocated space as expected:
> if (refcount_dec_and_test(&ref->refs)) {
> kmem_cache_free();
> }
>
> However btrfs-progs backport is using the opposite condition:
> if (--ref->refs) {
> kfree();
> }
>
> This will not free the memory for the last user, but for refs >= 2.
> Causing both use-after-free and memory leak for any offline write
> operation.
>
> [FIX]
> Fix the (--ref->refs) condition to (--ref->refs == 0) to fix the
> backport error.
>
> Fixes: c6039704c580 ("btrfs-progs: Add delayed refs infrastructure")
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Thanks, added to devel.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] btrfs-progs: delayed-ref: Fix memory leak and use-after-free caused by wrong condition to free delayed ref/head.
@ 2019-06-13 7:49 Qu Wenruo
2019-06-14 16:03 ` David Sterba
0 siblings, 1 reply; 4+ messages in thread
From: Qu Wenruo @ 2019-06-13 7:49 UTC (permalink / raw)
To: linux-btrfs
[BUG]
When btrfs-progs is compiled with D=asan, it can't pass even the very
basic fsck tests due to btrfs-image has memory leak:
=== START TEST /home/adam/btrfs/btrfs-progs/tests//fsck-tests/001-bad-file-extent-bytenr
restoring image default_case.img
=================================================================
==7790==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 104 byte(s) in 1 object(s) allocated from:
#0 0x7f1d3b738389 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:86
#1 0x560ca6b7f4ff in btrfs_add_delayed_tree_ref /home/adam/btrfs/btrfs-progs/delayed-ref.c:569
#2 0x560ca6af2d0b in btrfs_free_extent /home/adam/btrfs/btrfs-progs/extent-tree.c:2155
#3 0x560ca6ac16ca in __btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:319
#4 0x560ca6ac1d8c in btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:383
#5 0x560ca6ac6c8e in btrfs_search_slot /home/adam/btrfs/btrfs-progs/ctree.c:1153
#6 0x560ca6ab7e83 in fixup_device_size image/main.c:2113
#7 0x560ca6ab9279 in fixup_chunks_and_devices image/main.c:2333
#8 0x560ca6ab9ada in restore_metadump image/main.c:2455
#9 0x560ca6abaeba in main image/main.c:2723
#10 0x7f1d3b148ce2 in __libc_start_main (/usr/lib/libc.so.6+0x23ce2)
... tons of similar leakage for delayed_tree_ref ...
Direct leak of 96 byte(s) in 1 object(s) allocated from:
#0 0x7f1d3b738389 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:86
#1 0x560ca6b7f5fb in btrfs_add_delayed_tree_ref /home/adam/btrfs/btrfs-progs/delayed-ref.c:583
#2 0x560ca6af5679 in alloc_tree_block /home/adam/btrfs/btrfs-progs/extent-tree.c:2503
#3 0x560ca6af57ac in btrfs_alloc_free_block /home/adam/btrfs/btrfs-progs/extent-tree.c:2524
#4 0x560ca6ac115b in __btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:290
#5 0x560ca6ac1d8c in btrfs_cow_block /home/adam/btrfs/btrfs-progs/ctree.c:383
#6 0x560ca6b7bb15 in commit_tree_roots /home/adam/btrfs/btrfs-progs/transaction.c:98
#7 0x560ca6b7c525 in btrfs_commit_transaction /home/adam/btrfs/btrfs-progs/transaction.c:192
#8 0x560ca6ab92be in fixup_chunks_and_devices image/main.c:2337
#9 0x560ca6ab9ada in restore_metadump image/main.c:2455
#10 0x560ca6abaeba in main image/main.c:2723
#11 0x7f1d3b148ce2 in __libc_start_main (/usr/lib/libc.so.6+0x23ce2)
... tons of similar leakage for delayed_ref_head ...
SUMMARY: AddressSanitizer: 1600 byte(s) leaked in 16 allocation(s).
failed to restore image ./default_case.img
[CAUSE]
Commit c6039704c580 ("btrfs-progs: Add delayed refs infrastructure")
introduces delayed ref infrastructure for free space tree, however the
refcount_dec_and_test() from kernel code is wrongly backported.
refcount_dec_and_test() will return true if the refcount reaches 0.
So kernel code will free the allocated space as expected:
if (refcount_dec_and_test(&ref->refs)) {
kmem_cache_free();
}
However btrfs-progs backport is using the opposite condition:
if (--ref->refs) {
kfree();
}
This will not free the memory for the last user, but for refs >= 2.
Causing both use-after-free and memory leak for any offline write
operation.
[FIX]
Fix the (--ref->refs) condition to (--ref->refs == 0) to fix the
backport error.
Fixes: c6039704c580 ("btrfs-progs: Add delayed refs infrastructure")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
delayed-ref.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/delayed-ref.h b/delayed-ref.h
index efc855eff621..2ccfd27c2b95 100644
--- a/delayed-ref.h
+++ b/delayed-ref.h
@@ -161,7 +161,7 @@ btrfs_free_delayed_extent_op(struct btrfs_delayed_extent_op *op)
static inline void btrfs_put_delayed_ref(struct btrfs_delayed_ref_node *ref)
{
WARN_ON(ref->refs == 0);
- if (--ref->refs) {
+ if (--ref->refs == 0) {
WARN_ON(ref->in_tree);
switch (ref->type) {
case BTRFS_TREE_BLOCK_REF_KEY:
@@ -180,7 +180,7 @@ static inline void btrfs_put_delayed_ref(struct btrfs_delayed_ref_node *ref)
static inline void btrfs_put_delayed_ref_head(struct btrfs_delayed_ref_head *head)
{
- if (--head->refs)
+ if (--head->refs == 0)
kfree(head);
}
--
2.22.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-06-19 6:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-13 7:33 [PATCH] btrfs-progs: delayed-ref: Fix memory leak and use-after-free caused by wrong condition to free delayed ref/head Qu Wenruo
2019-06-19 6:16 ` Nikolay Borisov
2019-06-13 7:49 Qu Wenruo
2019-06-14 16:03 ` 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).