Re: [PATCH] btrfs-progs: super-recover: fix double free fs_devices memory

2018-10-24 Thread David Sterba 
On Fri, Oct 12, 2018 at 04:25:23PM +0800, robbieko wrote:
> From: Robbie Ko 
> 
> 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 

The fix for the double free has been in the devel branch, but thanks
anyway.

[PATCH] btrfs-progs: super-recover: fix double free fs_devices memory

2018-10-12 Thread robbieko 
From: Robbie Ko 

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 
---
 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

Re: [PATCH] Btrfs-progs: super-recover: fix double free fs_devices memory

2014-09-22 Thread Eric Sandeen 
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 
> Cc: Chris Murphy 
> Signed-off-by: Wang Shilong 

That does seem to fix the testcase.  Thanks!

Acked-by: Eric Sandeen 

> ---
>  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;
>  }
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Btrfs-progs: super-recover: fix double free fs_devices memory

2014-09-18 Thread Gui Hecheng 
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 
> Cc: Chris Murphy 
> Signed-off-by: Wang Shilong 
> ---
>  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". ^_^

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Btrfs-progs: super-recover: fix double free fs_devices memory

2014-09-18 Thread Wang Shilong 
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 
Cc: Chris Murphy 
Signed-off-by: Wang Shilong 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html