[PATCH 1/3] Btrfs-progs: subvol_uuid_search: Return error on memory allocation failure

[PATCH 1/3] Btrfs-progs: subvol_uuid_search: Return error on memory allocation failure

[Prasanth K S R]

From: Prasanth K S R <prasanth.ksr@dell.com>

This commit fixes coverity defect CID 1328695.

Signed-off-by: Prasanth K S R <prasanth.ksr@dell.com>
---
 send-utils.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/send-utils.c b/send-utils.c
index a85fa08..5026882 100644
--- a/send-utils.c
+++ b/send-utils.c
@@ -486,6 +486,10 @@ struct subvol_info *subvol_uuid_search(struct subvol_uuid_search *s,
 		info->path = strdup(path);
 	} else {
 		info->path = malloc(PATH_MAX);
+		if (!info->path) {
+			ret = -ENOMEM;
+			goto out;
+		}
 		ret = btrfs_subvolid_resolve(s->mnt_fd, info->path,
 					     PATH_MAX, root_id);
 	}
-- 
2.10.2

[PATCH 2/3] Btrfs-progs: subvol_uuid_search: Return error encoded pointer

[Prasanth K S R]

From: Prasanth K S R <prasanth.ksr@dell.com>

This commit changes subvol_uuid_search() to return an error encoded
pointer on failure.

Signed-off-by: Prasanth K S R <prasanth.ksr@dell.com>
---
 cmds-receive.c | 10 +++++-----
 cmds-send.c    | 19 +++++++++----------
 send-utils.c   |  2 +-
 3 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/cmds-receive.c b/cmds-receive.c
index 6f54c2e..cb42aa2 100644
--- a/cmds-receive.c
+++ b/cmds-receive.c
@@ -287,13 +287,13 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
 	parent_subvol = subvol_uuid_search(&rctx->sus, 0, parent_uuid,
 					   parent_ctransid, NULL,
 					   subvol_search_by_received_uuid);
-	if (!parent_subvol) {
+	if (IS_ERR(parent_subvol)) {
 		parent_subvol = subvol_uuid_search(&rctx->sus, 0, parent_uuid,
 						   parent_ctransid, NULL,
 						   subvol_search_by_uuid);
 	}
-	if (!parent_subvol) {
-		ret = -ENOENT;
+	if (IS_ERR(parent_subvol)) {
+		ret = PTR_ERR(parent_subvol);
 		error("cannot find parent subvolume");
 		goto out;
 	}
@@ -750,13 +750,13 @@ static int process_clone(const char *path, u64 offset, u64 len,
 	si = subvol_uuid_search(&rctx->sus, 0, clone_uuid, clone_ctransid,
 				NULL,
 				subvol_search_by_received_uuid);
-	if (!si) {
+	if (IS_ERR(si)) {
 		if (memcmp(clone_uuid, rctx->cur_subvol.received_uuid,
 				BTRFS_UUID_SIZE) == 0) {
 			/* TODO check generation of extent */
 			subvol_path = strdup(rctx->cur_subvol_path);
 		} else {
-			ret = -ENOENT;
+			ret = PTR_ERR(si);
 			error("clone: did not find source subvol");
 			goto out;
 		}
diff --git a/cmds-send.c b/cmds-send.c
index a4fb126..5da64d8 100644
--- a/cmds-send.c
+++ b/cmds-send.c
@@ -70,8 +70,8 @@ static int get_root_id(struct btrfs_send *sctx, const char *path, u64 *root_id)
 
 	si = subvol_uuid_search(&sctx->sus, 0, NULL, 0, path,
 			subvol_search_by_path);
-	if (!si)
-		return -ENOENT;
+	if (IS_ERR(si))
+		return PTR_ERR(si);
 	*root_id = si->root_id;
 	free(si->path);
 	free(si);
@@ -85,8 +85,8 @@ static struct subvol_info *get_parent(struct btrfs_send *sctx, u64 root_id)
 
 	si_tmp = subvol_uuid_search(&sctx->sus, root_id, NULL, 0, NULL,
 			subvol_search_by_root_id);
-	if (!si_tmp)
-		return NULL;
+	if (IS_ERR(si_tmp))
+		return si_tmp;
 
 	si = subvol_uuid_search(&sctx->sus, 0, si_tmp->parent_uuid, 0, NULL,
 			subvol_search_by_uuid);
@@ -105,8 +105,8 @@ static int find_good_parent(struct btrfs_send *sctx, u64 root_id, u64 *found)
 	int i;
 
 	parent = get_parent(sctx, root_id);
-	if (!parent) {
-		ret = -ENOENT;
+	if (IS_ERR(parent)) {
+		ret = PTR_ERR(parent);
 		goto out;
 	}
 
@@ -122,7 +122,7 @@ static int find_good_parent(struct btrfs_send *sctx, u64 root_id, u64 *found)
 		s64 tmp;
 
 		parent2 = get_parent(sctx, sctx->clone_sources[i]);
-		if (!parent2)
+		if (IS_ERR(parent2))
 			continue;
 		if (parent2->root_id != parent->root_id) {
 			free(parent2->path);
@@ -136,9 +136,8 @@ static int find_good_parent(struct btrfs_send *sctx, u64 root_id, u64 *found)
 		parent2 = subvol_uuid_search(&sctx->sus,
 				sctx->clone_sources[i], NULL, 0, NULL,
 				subvol_search_by_root_id);
-
-		if (!parent2) {
-			ret = -ENOENT;
+		if (IS_ERR(parent2)) {
+			ret = PTR_ERR(parent2);
 			goto out;
 		}
 		tmp = parent2->ctransid - parent->ctransid;
diff --git a/send-utils.c b/send-utils.c
index 5026882..252ca6d 100644
--- a/send-utils.c
+++ b/send-utils.c
@@ -498,7 +498,7 @@ out:
 	if (ret && info) {
 		free(info->path);
 		free(info);
-		info = NULL;
+		return ERR_PTR(ret);
 	}
 
 	return info;
-- 
2.10.2

[PATCH 3/3] Btrfs-progs: subvol_uuid_search: Return error code on memory allocation failure

[Prasanth K S R]

From: Prasanth K S R <prasanth.ksr@dell.com>

On failure of memory allocation for a 'struct subvol_info', we would end
up dereferencing a NULL pointer. This commit fixes the issue by returning an
error encoded pointer.

Signed-off-by: Prasanth K S R <prasanth.ksr@dell.com>
---
 send-utils.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/send-utils.c b/send-utils.c
index 252ca6d..95445b5 100644
--- a/send-utils.c
+++ b/send-utils.c
@@ -474,6 +474,10 @@ struct subvol_info *subvol_uuid_search(struct subvol_uuid_search *s,
 		goto out;
 
 	info = calloc(1, sizeof(*info));
+	if (!info) {
+		ret = -ENOMEM;
+		goto out;
+	}
 	info->root_id = root_id;
 	memcpy(info->uuid, root_item.uuid, BTRFS_UUID_SIZE);
 	memcpy(info->received_uuid, root_item.received_uuid, BTRFS_UUID_SIZE);
@@ -495,9 +499,11 @@ struct subvol_info *subvol_uuid_search(struct subvol_uuid_search *s,
 	}
 
 out:
-	if (ret && info) {
-		free(info->path);
-		free(info);
+	if (ret) {
+		if (info) {
+			free(info->path);
+			free(info);
+		}
 		return ERR_PTR(ret);
 	}
 
-- 
2.10.2

Re: [PATCH 1/3] Btrfs-progs: subvol_uuid_search: Return error on memory allocation failure

[David Sterba]

On Sat, Dec 10, 2016 at 07:17:42PM +0530, Prasanth K S R wrote:
> From: Prasanth K S R <prasanth.ksr@dell.com>
> 
> This commit fixes coverity defect CID 1328695.
> 
> Signed-off-by: Prasanth K S R <prasanth.ksr@dell.com>

Thanks, 1-3 applied.

JFYI, I've realized that subvol_uuid_search is part of the public API so
changing the return value would break the existing users (I know that
snapper uses this function), as it now returns just a pointer or NULL.
I'll fix that, a new function and library version bump would be needed.