[PATCH 1/4] btrfs-progs: Fix memory leak in write_raid56_with_parity

[PATCH 1/4] btrfs-progs: Fix memory leak in write_raid56_with_parity

[Qu Wenruo]

Ebs and pointers are allocated, but if any of the allocation failed, we
should free the allocated memory.

Reported-by: David Sterba <dsterba@suse.cz>
Resolves-Coverity-CID: 1296749
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 volumes.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/volumes.c b/volumes.c
index a7abd92..f687b0d 100644
--- a/volumes.c
+++ b/volumes.c
@@ -2120,8 +2120,11 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
 
 	ebs = malloc(sizeof(*ebs) * multi->num_stripes);
 	pointers = malloc(sizeof(*pointers) * multi->num_stripes);
-	if (!ebs || !pointers)
+	if (!ebs || !pointers) {
+		free(ebs);
+		free(pointers);
 		return -ENOMEM;
+	}
 
 	if (stripe_len > alloc_size)
 		alloc_size = stripe_len;
-- 
2.10.1

[PATCH 2/4] btrfs-progs: fsck: Fix patch allocation check and leak in check_fs_first_inode

[Qu Wenruo]

Allocated 'path' in check_fs_first_inode() is not checked and for
btrfs_search_slot() error, it will leak 'path'.

Fix it.

Reported-by: David Sterba <dsterba@suse.cz>
Resolves-Coverity-CID: 1374098
Resolves-Coverity-CID: 1374099
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 cmds-check.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/cmds-check.c b/cmds-check.c
index a92901d..91ed8b4 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -4963,13 +4963,15 @@ static int check_fs_first_inode(struct btrfs_root *root, unsigned int ext_ref)
 	int ret;
 
 	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
 	key.objectid = 256;
 	key.type = BTRFS_INODE_ITEM_KEY;
 	key.offset = 0;
 
 	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
 	if (ret < 0)
-		return ret;
+		goto out;
 	if (ret > 0) {
 		ret = 0;
 		err |= INODE_ITEM_MISSING;
@@ -4979,6 +4981,7 @@ static int check_fs_first_inode(struct btrfs_root *root, unsigned int ext_ref)
 	err &= ~LAST_ITEM;
 	if (err && !ret)
 		ret = -EIO;
+out:
 	btrfs_free_path(path);
 	return ret;
 }
-- 
2.10.1

[PATCH 3/4] btrfs-progs: utils: Fix NULL pointer derefernces in string_is_numerical

[Qu Wenruo]

In get_running_kernel_version() function, we directly pass return
pointer from strtok_r() to string_is_numberical().

Return pointer from strok_r() can be NULL, but string_is_numberical()
can't handle it and will cause NULL pointer derefernces.

Fix it by check if it's a NULL pointer first.

Reported-by: David Sterba <dsterba@suse.cz>
Resolves-Coverity-CID: 1374097
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 utils.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/utils.c b/utils.c
index 3f54245..c135ac9 100644
--- a/utils.c
+++ b/utils.c
@@ -4015,6 +4015,8 @@ unsigned int get_unit_mode_from_arg(int *argc, char *argv[], int df_mode)
 
 int string_is_numerical(const char *str)
 {
+	if (!str)
+		return 0;
 	if (!(*str >= '0' && *str <= '9'))
 		return 0;
 	while (*str >= '0' && *str <= '9')
-- 
2.10.1

[PATCH 4/4] btrfs-progs: fsck: Fix NULL pointer dereference for possible memory allocation failure

[Qu Wenruo]

We didn't check 'path' allocated in check_root_ref(), which can cause
NULL pointer dereference if the memory allocation failed.

Fix it by using stack memory, since the function should return error
bitmap not minus error code, we don't want memory allocation to be an
exception.

Reported-by: David Sterba <dsterba@suse.cz>
Resolves-Coverity-CID: 1372510
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 cmds-check.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index 91ed8b4..563cd55 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -5067,7 +5067,7 @@ out:
 static int check_root_ref(struct btrfs_root *root, struct btrfs_key *ref_key,
 			  struct extent_buffer *node, int slot)
 {
-	struct btrfs_path *path;
+	struct btrfs_path path;
 	struct btrfs_key key;
 	struct btrfs_root_ref *ref;
 	struct btrfs_root_ref *backref;
@@ -5104,8 +5104,8 @@ static int check_root_ref(struct btrfs_root *root, struct btrfs_key *ref_key,
 	key.type = BTRFS_ROOT_BACKREF_KEY + BTRFS_ROOT_REF_KEY - ref_key->type;
 	key.offset = ref_key->objectid;
 
-	path = btrfs_alloc_path();
-	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+	btrfs_init_path(&path);
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
 	if (ret) {
 		err |= ROOT_REF_MISSING;
 		error("%s[%llu %llu] couldn't find relative ref",
@@ -5115,11 +5115,11 @@ static int check_root_ref(struct btrfs_root *root, struct btrfs_key *ref_key,
 		goto out;
 	}
 
-	backref = btrfs_item_ptr(path->nodes[0], path->slots[0],
+	backref = btrfs_item_ptr(path.nodes[0], path.slots[0],
 				 struct btrfs_root_ref);
-	backref_dirid = btrfs_root_ref_dirid(path->nodes[0], backref);
-	backref_seq = btrfs_root_ref_sequence(path->nodes[0], backref);
-	backref_namelen = btrfs_root_ref_name_len(path->nodes[0], backref);
+	backref_dirid = btrfs_root_ref_dirid(path.nodes[0], backref);
+	backref_seq = btrfs_root_ref_sequence(path.nodes[0], backref);
+	backref_namelen = btrfs_root_ref_name_len(path.nodes[0], backref);
 
 	if (backref_namelen <= BTRFS_NAME_LEN) {
 		len = backref_namelen;
@@ -5130,7 +5130,7 @@ static int check_root_ref(struct btrfs_root *root, struct btrfs_key *ref_key,
 			"ROOT_REF" : "ROOT_BACKREF",
 			key.objectid, key.offset);
 	}
-	read_extent_buffer(path->nodes[0], backref_name,
+	read_extent_buffer(path.nodes[0], backref_name,
 			   (unsigned long)(backref + 1), len);
 
 	if (ref_dirid != backref_dirid || ref_seq != backref_seq ||
@@ -5143,7 +5143,7 @@ static int check_root_ref(struct btrfs_root *root, struct btrfs_key *ref_key,
 		      ref_key->objectid, ref_key->offset);
 	}
 out:
-	btrfs_free_path(path);
+	btrfs_release_path(&path);
 	return err;
 }
 
-- 
2.10.1

Re: [PATCH 1/4] btrfs-progs: Fix memory leak in write_raid56_with_parity

[Qu Wenruo]

At 10/24/2016 10:43 AM, Qu Wenruo wrote:
> Ebs and pointers are allocated, but if any of the allocation failed, we
> should free the allocated memory.
>
> Reported-by: David Sterba <dsterba@suse.cz>
> Resolves-Coverity-CID: 1296749

Sorry, wrong CID here,
Correct ones are:
Resolves-Coverity-CID: 1374101
Resolves-Coverity-CID: 1374100


> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
>  volumes.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/volumes.c b/volumes.c
> index a7abd92..f687b0d 100644
> --- a/volumes.c
> +++ b/volumes.c
> @@ -2120,8 +2120,11 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
>
>  	ebs = malloc(sizeof(*ebs) * multi->num_stripes);
>  	pointers = malloc(sizeof(*pointers) * multi->num_stripes);
> -	if (!ebs || !pointers)
> +	if (!ebs || !pointers) {
> +		free(ebs);
> +		free(pointers);
>  		return -ENOMEM;
> +	}
>
>  	if (stripe_len > alloc_size)
>  		alloc_size = stripe_len;
>

Re: [PATCH 1/4] btrfs-progs: Fix memory leak in write_raid56_with_parity

[David Sterba]

On Mon, Oct 24, 2016 at 10:43:32AM +0800, Qu Wenruo wrote:
> Ebs and pointers are allocated, but if any of the allocation failed, we
> should free the allocated memory.
> 
> Reported-by: David Sterba <dsterba@suse.cz>
> Resolves-Coverity-CID: 1296749
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>

1-4 applied, thanks. Please don't put my reported-by there.