* [PATCH] Btrfs-progs: super-recover: fix double free fs_devices memory
2014-09-17 17:00 "btrfs rescue super-recover" memory corruption Eric Sandeen
@ 2014-09-18 9:01 ` Wang Shilong
2014-09-18 9:23 ` Gui Hecheng
2014-09-22 20:39 ` Eric Sandeen
0 siblings, 2 replies; 5+ messages in thread
From: Wang Shilong @ 2014-09-18 9:01 UTC (permalink / raw)
To: linux-btrfs; +Cc: Wang Shilong, Eric Sandeen, Chris Murphy
super-recover collects btrfs devices infomation using existed
functions scan_one_devices().
Problem is fs_devices is freed twice in close_ctree() and
free_recover_superblock() for super correction path.
Fix this problem by checking whether fs_devices memory
have been freed before we free it.
Cc: Eric Sandeen <sandeen@redhat.com>
Cc: Chris Murphy <lists@colorremedies.com>
Signed-off-by: Wang Shilong <wangshilong1991@gmail.com>
---
super-recover.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/super-recover.c b/super-recover.c
index 767de4b..419b86a 100644
--- a/super-recover.c
+++ b/super-recover.c
@@ -69,21 +69,11 @@ void init_recover_superblock(struct btrfs_recover_superblock *recover)
static
void free_recover_superblock(struct btrfs_recover_superblock *recover)
{
- struct btrfs_device *device;
struct super_block_record *record;
if (!recover->fs_devices)
return;
- while (!list_empty(&recover->fs_devices->devices)) {
- device = list_entry(recover->fs_devices->devices.next,
- struct btrfs_device, dev_list);
- list_del_init(&device->dev_list);
- free(device->name);
- free(device);
- }
- free(recover->fs_devices);
-
while (!list_empty(&recover->good_supers)) {
record = list_entry(recover->good_supers.next,
struct super_block_record, list);
@@ -341,6 +331,9 @@ int btrfs_recover_superblocks(const char *dname,
no_recover:
recover_err_str(ret);
free_recover_superblock(&recover);
+ /* check if we have freed fs_deivces in close_ctree() */
+ if (!root)
+ btrfs_close_devices(recover.fs_devices);
return ret;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Btrfs-progs: super-recover: fix double free fs_devices memory
2014-09-18 9:01 ` [PATCH] Btrfs-progs: super-recover: fix double free fs_devices memory Wang Shilong
@ 2014-09-18 9:23 ` Gui Hecheng
2014-09-22 20:39 ` Eric Sandeen
1 sibling, 0 replies; 5+ messages in thread
From: Gui Hecheng @ 2014-09-18 9:23 UTC (permalink / raw)
To: Wang Shilong; +Cc: linux-btrfs, Eric Sandeen, Chris Murphy
On Thu, 2014-09-18 at 05:01 -0400, Wang Shilong wrote:
> super-recover collects btrfs devices infomation using existed
> functions scan_one_devices().
>
> Problem is fs_devices is freed twice in close_ctree() and
> free_recover_superblock() for super correction path.
>
> Fix this problem by checking whether fs_devices memory
> have been freed before we free it.
>
> Cc: Eric Sandeen <sandeen@redhat.com>
> Cc: Chris Murphy <lists@colorremedies.com>
> Signed-off-by: Wang Shilong <wangshilong1991@gmail.com>
> ---
> super-recover.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/super-recover.c b/super-recover.c
> index 767de4b..419b86a 100644
> --- a/super-recover.c
> +++ b/super-recover.c
> @@ -69,21 +69,11 @@ void init_recover_superblock(struct btrfs_recover_superblock *recover)
> static
> void free_recover_superblock(struct btrfs_recover_superblock *recover)
> {
> - struct btrfs_device *device;
> struct super_block_record *record;
>
> if (!recover->fs_devices)
> return;
>
> - while (!list_empty(&recover->fs_devices->devices)) {
> - device = list_entry(recover->fs_devices->devices.next,
> - struct btrfs_device, dev_list);
> - list_del_init(&device->dev_list);
> - free(device->name);
> - free(device);
> - }
> - free(recover->fs_devices);
> -
> while (!list_empty(&recover->good_supers)) {
> record = list_entry(recover->good_supers.next,
> struct super_block_record, list);
> @@ -341,6 +331,9 @@ int btrfs_recover_superblocks(const char *dname,
> no_recover:
> recover_err_str(ret);
> free_recover_superblock(&recover);
> + /* check if we have freed fs_deivces in close_ctree() */
> + if (!root)
> + btrfs_close_devices(recover.fs_devices);
> return ret;
> }
>
nice catch! "+20, recorded". ^_^
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Btrfs-progs: super-recover: fix double free fs_devices memory
2014-09-18 9:01 ` [PATCH] Btrfs-progs: super-recover: fix double free fs_devices memory Wang Shilong
2014-09-18 9:23 ` Gui Hecheng
@ 2014-09-22 20:39 ` Eric Sandeen
1 sibling, 0 replies; 5+ messages in thread
From: Eric Sandeen @ 2014-09-22 20:39 UTC (permalink / raw)
To: Wang Shilong, linux-btrfs; +Cc: Chris Murphy
On 9/18/14 4:01 AM, Wang Shilong wrote:
> super-recover collects btrfs devices infomation using existed
> functions scan_one_devices().
>
> Problem is fs_devices is freed twice in close_ctree() and
> free_recover_superblock() for super correction path.
>
> Fix this problem by checking whether fs_devices memory
> have been freed before we free it.
>
> Cc: Eric Sandeen <sandeen@redhat.com>
> Cc: Chris Murphy <lists@colorremedies.com>
> Signed-off-by: Wang Shilong <wangshilong1991@gmail.com>
That does seem to fix the testcase. Thanks!
Acked-by: Eric Sandeen <sandeen@redhat.com>
> ---
> super-recover.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/super-recover.c b/super-recover.c
> index 767de4b..419b86a 100644
> --- a/super-recover.c
> +++ b/super-recover.c
> @@ -69,21 +69,11 @@ void init_recover_superblock(struct btrfs_recover_superblock *recover)
> static
> void free_recover_superblock(struct btrfs_recover_superblock *recover)
> {
> - struct btrfs_device *device;
> struct super_block_record *record;
>
> if (!recover->fs_devices)
> return;
>
> - while (!list_empty(&recover->fs_devices->devices)) {
> - device = list_entry(recover->fs_devices->devices.next,
> - struct btrfs_device, dev_list);
> - list_del_init(&device->dev_list);
> - free(device->name);
> - free(device);
> - }
> - free(recover->fs_devices);
> -
> while (!list_empty(&recover->good_supers)) {
> record = list_entry(recover->good_supers.next,
> struct super_block_record, list);
> @@ -341,6 +331,9 @@ int btrfs_recover_superblocks(const char *dname,
> no_recover:
> recover_err_str(ret);
> free_recover_superblock(&recover);
> + /* check if we have freed fs_deivces in close_ctree() */
> + if (!root)
> + btrfs_close_devices(recover.fs_devices);
> return ret;
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] btrfs-progs: super-recover: fix double free fs_devices memory
@ 2018-10-12 8:25 robbieko
2018-10-24 18:44 ` David Sterba
0 siblings, 1 reply; 5+ messages in thread
From: robbieko @ 2018-10-12 8:25 UTC (permalink / raw)
To: linux-btrfs; +Cc: Robbie Ko
From: Robbie Ko <robbieko@synology.com>
super-recover collects btrfs devices information using existed
functions scan_one_devices().
Problem is fs_devices is freed twice. One in __open_ctree_fd() when
error happens and the other in btrfs_close_devices(recover.fs_devices)
when root is NULL.
Commit "30fd6f2e92695c355c8f76b8887cd4fade60cdac" add force-close
all opened device before program exit, to avoid memory leak in all
btrfs sub-command.
Therefore, there is an unnecessary freed of fs_devices in
btrfs_recover_superblocks.
Fix this problem by remove unnecessary freed of fs_devices.
Signed-off-by: Robbie Ko <robbieko@synology.com>
---
super-recover.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/super-recover.c b/super-recover.c
index 880fd77..86b3df9 100644
--- a/super-recover.c
+++ b/super-recover.c
@@ -292,9 +292,6 @@ int btrfs_recover_superblocks(const char *dname,
no_recover:
recover_err_str(ret);
free_recover_superblock(&recover);
- /* check if we have freed fs_devices in close_ctree() */
- if (!root)
- btrfs_close_devices(recover.fs_devices);
return ret;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs-progs: super-recover: fix double free fs_devices memory
2018-10-12 8:25 [PATCH] btrfs-progs: super-recover: fix double free fs_devices memory robbieko
@ 2018-10-24 18:44 ` David Sterba
0 siblings, 0 replies; 5+ messages in thread
From: David Sterba @ 2018-10-24 18:44 UTC (permalink / raw)
To: robbieko; +Cc: linux-btrfs
On Fri, Oct 12, 2018 at 04:25:23PM +0800, robbieko wrote:
> From: Robbie Ko <robbieko@synology.com>
>
> super-recover collects btrfs devices information using existed
> functions scan_one_devices().
>
> Problem is fs_devices is freed twice. One in __open_ctree_fd() when
> error happens and the other in btrfs_close_devices(recover.fs_devices)
> when root is NULL.
>
> Commit "30fd6f2e92695c355c8f76b8887cd4fade60cdac" add force-close
> all opened device before program exit, to avoid memory leak in all
> btrfs sub-command.
> Therefore, there is an unnecessary freed of fs_devices in
> btrfs_recover_superblocks.
>
> Fix this problem by remove unnecessary freed of fs_devices.
>
> Signed-off-by: Robbie Ko <robbieko@synology.com>
The fix for the double free has been in the devel branch, but thanks
anyway.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-10-24 18:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-12 8:25 [PATCH] btrfs-progs: super-recover: fix double free fs_devices memory robbieko
2018-10-24 18:44 ` David Sterba
-- strict thread matches above, loose matches on Subject: below --
2014-09-17 17:00 "btrfs rescue super-recover" memory corruption Eric Sandeen
2014-09-18 9:01 ` [PATCH] Btrfs-progs: super-recover: fix double free fs_devices memory Wang Shilong
2014-09-18 9:23 ` Gui Hecheng
2014-09-22 20:39 ` Eric Sandeen
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).