[PATCH 0/4] some btrfs-progs coverity fixes

[PATCH 0/4] some btrfs-progs coverity fixes

[Zach Brown]

Hi gang,

Here's another set of coverity fixes for btrfs-progs against David's 
integration-20141007 branch.

I got tired of adding error checking after a few so I moved on to the
other warnings.  Maybe we should subscribe linux-btrfs to the reports
that coverity can send out?

- z

[PATCH 1/4] btrfs-progs: check sscanf return code

[Zach Brown]

coverity warned that the return code from sscanf() assigned to 'i'
wasn't checked before being assigned again.  Check it.

Signed-off-by: Zach Brown <zab@zabbo.net>
---
 utils.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/utils.c b/utils.c
index c2f30d4..51e55be 100644
--- a/utils.c
+++ b/utils.c
@@ -1574,7 +1574,11 @@ scan_again:
 
 	strcpy(fullpath,"/dev/");
 	while(fgets(buf, 1023, proc_partitions)) {
-		i = sscanf(buf," %*d %*d %*d %99s", fullpath+5);
+		ret = sscanf(buf," %*d %*d %*d %99s", fullpath+5);
+		if (ret != 1) {
+			fprintf(stderr, "failed to scan device name from /proc/partitions\n");
+			break;
+		}
 
 		/*
 		 * multipath and MD devices may register as a btrfs filesystem
-- 
1.9.3

[PATCH 2/4] btrfs-progs: check read extent errors when mapping

[Zach Brown]

coverity barked out a warning that btrfs-map-logical was storing but
ignoring errors from read_extent_from_disk().   So don't ignore 'em.  I
made extent reading errors fatal to match the fatal errors from mapping
mirrors above.

And while we're at it have read_extent_from_disk() return -errno pread
errors instead of -EIO or -1 (-EPERM).  The only other caller who tests
errors clobbers them with -EIO.

Signed-off-by: Zach Brown <zab@zabbo.net>
---
 btrfs-map-logical.c | 12 +++++++++++-
 extent_io.c         |  4 +++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/btrfs-map-logical.c b/btrfs-map-logical.c
index 6b475fc..47d1104 100644
--- a/btrfs-map-logical.c
+++ b/btrfs-map-logical.c
@@ -76,8 +76,18 @@ static struct extent_buffer * debug_read_block(struct btrfs_root *root,
 			(unsigned long long)eb->dev_bytenr, device->name);
 		kfree(multi);
 
-		if (!copy || mirror_num == copy)
+		if (!copy || mirror_num == copy) {
 			ret = read_extent_from_disk(eb, 0, eb->len);
+			if (ret) {
+				fprintf(info_file,
+					"Error: failed to read extent: mirror %d logical %llu: %s\n",
+					mirror_num, (unsigned long long)eb->start,
+					strerror(-ret));
+				free_extent_buffer(eb);
+				eb = NULL;
+				break;
+			}
+		}
 
 		num_copies = btrfs_num_copies(&root->fs_info->mapping_tree,
 					      eb->start, eb->len);
diff --git a/extent_io.c b/extent_io.c
index 425af8a..5186e89 100644
--- a/extent_io.c
+++ b/extent_io.c
@@ -647,8 +647,10 @@ int read_extent_from_disk(struct extent_buffer *eb,
 {
 	int ret;
 	ret = pread(eb->fd, eb->data + offset, len, eb->dev_bytenr);
-	if (ret < 0)
+	if (ret < 0) {
+		ret = -errno;
 		goto out;
+	}
 	if (ret != len) {
 		ret = -EIO;
 		goto out;
-- 
1.9.3

[PATCH 3/4] btrfs-progs: fix show super unknown flag output

[Zach Brown]

coverity pointed out that unknown flag printing in show super had some
dead code.  It turns out that first was reset when the first flag was
tested, not when it was output.  We only want to clear it if the first
matching bit is output.  If there are no matching bits then we'll want
to output the unknown flag first.

Signed-off-by: Zach Brown <zab@zabbo.net>
---
 btrfs-show-super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/btrfs-show-super.c b/btrfs-show-super.c
index 456dbd8..2b48f44 100644
--- a/btrfs-show-super.c
+++ b/btrfs-show-super.c
@@ -324,8 +324,8 @@ static void print_readable_incompat_flag(u64 flag)
 				printf("%s ", entry->output);
 			else
 				printf("|\n\t\t\t  %s ", entry->output);
+			first = 0;
 		}
-		first = 0;
 	}
 	flag &= ~BTRFS_FEATURE_INCOMPAT_SUPP;
 	if (flag) {
-- 
1.9.3

[PATCH 4/4] btrfs-progs: fix csum root copy-n-paste error

[Zach Brown]

btrfs_setup_all_roots() had some copy and pasted code for trying to
setup a root and then creating a blank node if that failed.  The copy
for the csum_root created the blank node in the extent_root.

So we create a function to use a consistent root.

Signed-off-by: Zach Brown <zab@zabbo.net>
---
 disk-io.c | 66 ++++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 36 insertions(+), 30 deletions(-)

diff --git a/disk-io.c b/disk-io.c
index f9dbc85..b89bb29 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -831,6 +831,34 @@ static int find_best_backup_root(struct btrfs_super_block *super)
 	return best_index;
 }
 
+static int setup_root_or_create_block(struct btrfs_fs_info *fs_info,
+				      enum btrfs_open_ctree_flags flags,
+				      struct btrfs_root *info_root,
+				      u64 objectid, char *str)
+{
+	struct btrfs_super_block *sb = fs_info->super_copy;
+	struct btrfs_root *root = fs_info->tree_root;
+	u32 leafsize = btrfs_super_leafsize(sb);
+	int ret;
+
+	ret = find_and_setup_root(root, fs_info, objectid, info_root);
+	if (ret) {
+		printk("Couldn't setup %s tree\n", str);
+		if (!(flags & OPEN_CTREE_PARTIAL))
+			return -EIO;
+		/* Need a blank node here just so we don't screw up in the
+		 * million of places that assume a root has a valid ->node
+		 */
+		info_root->node =
+			btrfs_find_create_tree_block(info_root, 0, leafsize);
+		if (!info_root->node)
+			return -ENOMEM;
+		clear_extent_buffer_uptodate(NULL, info_root->node);
+	}
+
+	return 0;
+}
+
 int btrfs_setup_all_roots(struct btrfs_fs_info *fs_info, u64 root_tree_bytenr,
 			  enum btrfs_open_ctree_flags flags)
 {
@@ -877,22 +905,10 @@ int btrfs_setup_all_roots(struct btrfs_fs_info *fs_info, u64 root_tree_bytenr,
 		return -EIO;
 	}
 
-	ret = find_and_setup_root(root, fs_info, BTRFS_EXTENT_TREE_OBJECTID,
-				  fs_info->extent_root);
-	if (ret) {
-		printk("Couldn't setup extent tree\n");
-		if (!(flags & OPEN_CTREE_PARTIAL))
-			return -EIO;
-		/* Need a blank node here just so we don't screw up in the
-		 * million of places that assume a root has a valid ->node
-		 */
-		fs_info->extent_root->node =
-			btrfs_find_create_tree_block(fs_info->extent_root, 0,
-						     leafsize);
-		if (!fs_info->extent_root->node)
-			return -ENOMEM;
-		clear_extent_buffer_uptodate(NULL, fs_info->extent_root->node);
-	}
+	ret = setup_root_or_create_block(fs_info, flags, fs_info->extent_root,
+					 BTRFS_EXTENT_TREE_OBJECTID, "extent");
+	if (ret)
+		return ret;
 	fs_info->extent_root->track_dirty = 1;
 
 	ret = find_and_setup_root(root, fs_info, BTRFS_DEV_TREE_OBJECTID,
@@ -903,20 +919,10 @@ int btrfs_setup_all_roots(struct btrfs_fs_info *fs_info, u64 root_tree_bytenr,
 	}
 	fs_info->dev_root->track_dirty = 1;
 
-	ret = find_and_setup_root(root, fs_info, BTRFS_CSUM_TREE_OBJECTID,
-				  fs_info->csum_root);
-	if (ret) {
-		printk("Couldn't setup csum tree\n");
-		if (!(flags & OPEN_CTREE_PARTIAL))
-			return -EIO;
-		/* do the same thing as extent tree rebuilding */
-		fs_info->csum_root->node =
-			btrfs_find_create_tree_block(fs_info->extent_root, 0,
-						     leafsize);
-		if (!fs_info->csum_root->node)
-			return -ENOMEM;
-		clear_extent_buffer_uptodate(NULL, fs_info->csum_root->node);
-	}
+	ret = setup_root_or_create_block(fs_info, flags, fs_info->csum_root,
+					 BTRFS_CSUM_TREE_OBJECTID, "csum");
+	if (ret)
+		return ret;
 	fs_info->csum_root->track_dirty = 1;
 
 	ret = find_and_setup_root(root, fs_info, BTRFS_QUOTA_TREE_OBJECTID,
-- 
1.9.3

Re: [PATCH 4/4] btrfs-progs: fix csum root copy-n-paste error

[David Sterba]

On Wed, Oct 15, 2014 at 04:14:21PM -0700, Zach Brown wrote:
> btrfs_setup_all_roots() had some copy and pasted code for trying to
> setup a root and then creating a blank node if that failed.  The copy
> for the csum_root created the blank node in the extent_root.

The cleanup is good, though I don't see the bug there. Although the passed root
is extent_root in both cases, it's only used to reach the
fs_info->extent_cache:

 131 struct extent_buffer *btrfs_find_create_tree_block(struct btrfs_root *root,
 132                                                  u64 bytenr, u32 blocksize)
 133 {
 134         return alloc_extent_buffer(&root->fs_info->extent_cache, bytenr,
 135                                    blocksize);
 136 }

Re: [PATCH 0/4] some btrfs-progs coverity fixes

[David Sterba]

On Wed, Oct 15, 2014 at 04:14:17PM -0700, Zach Brown wrote:
> Here's another set of coverity fixes for btrfs-progs against David's 
> integration-20141007 branch.

Thanks, I've fished 2 patches for 3.17, the rest is queued.

> I got tired of adding error checking after a few so I moved on to the
> other warnings.  Maybe we should subscribe linux-btrfs to the reports
> that coverity can send out?

I'm not sure if this is allowed by the coverity service and I was not
able to any info about that.

Re: [PATCH 0/4] some btrfs-progs coverity fixes

[Eric Sandeen]

On 10/16/14 6:46 AM, David Sterba wrote:
> On Wed, Oct 15, 2014 at 04:14:17PM -0700, Zach Brown wrote:
>> Here's another set of coverity fixes for btrfs-progs against David's 
>> integration-20141007 branch.
> 
> Thanks, I've fished 2 patches for 3.17, the rest is queued.
> 
>> I got tired of adding error checking after a few so I moved on to the
>> other warnings.  Maybe we should subscribe linux-btrfs to the reports
>> that coverity can send out?
> 
> I'm not sure if this is allowed by the coverity service and I was not
> able to any info about that.

We could, in theory, "invite" the list, and then I suppose we'd get
a confirmation email that 100 people would click on.  ;)

Could maybe just email the scan folks and ask.

I'm a little on the fence about immediately broadcasting all defects, though.
I doubt there should be security implications, but ...

I wish scan were better integrated with git, and then something like
"email the author of the commit that introduced a new defect" would
be pretty cool.

-Eric