Btrfs devel - Apr 2013 - [PATCH] Btrfs: deal with bad mappings in btrfs_map_block

Martin Steigerwald reported a BUG_ON() in btrfs_map_block where we
didn''t find
a chunk for a particular block we were trying to map.  This happened because the
block was bogus.  We shouldn''t be BUG_ON()''ing in this case,
just print a
message and return an error.  This came from reada_add_block and it appears to
deal with an error fine so we should be good there.  Thanks,

Reported-by: Martin Steigerwald <Martin@lichtvoll.de>
Signed-off-by: Josef Bacik <jbacik@fusionio.com>
---
 fs/btrfs/volumes.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 76ded9e..c8a315d 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4406,10 +4406,16 @@ static int __btrfs_map_block(struct btrfs_fs_info
*fs_info, int rw,
 		btrfs_crit(fs_info, "unable to find logical %llu len %llu",
 			(unsigned long long)logical,
 			(unsigned long long)*length);
-		BUG();
+		return -EINVAL;
+	}
+
+	if (em->start > logical || em->start + em->len < logical) {
+		btrfs_crit(fs_info, "found a bad mapping, wanted %Lu, "
+			   "found %Lu-%Lu\n", logical, em->start,
+			   em->start + em->len);
+		return -EINVAL;
 	}
 
-	BUG_ON(em->start > logical || em->start + em->len < logical);
 	map = (struct map_lookup *)em->bdev;
 	offset = logical - em->start;
 
-- 
1.7.7.6

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

On Mon, Apr 22, 2013 at 09:14:11AM -0400, Josef Bacik
wrote:
> Martin Steigerwald reported a BUG_ON() in btrfs_map_block where we
didn''t find
> a chunk for a particular block we were trying to map.  This happened
because the
> block was bogus.  We shouldn''t be BUG_ON()''ing in this
case, just print a
> message and return an error.  This came from reada_add_block and it appears
to
> deal with an error fine so we should be good there.  Thanks,
There are various call paths that lead to __btrfs_map_block, via
btrfs_map_block (and btrfs_map_bio).

Lots of the retval checks are still marked with /* ENOMEM */ so it does
not seem that all the paths were verified to be safe wrt EIO. After a
quick skim I think that most of the callsites are doing proper error
handling (and some of them continue to BUG_ON(ret < 0)) namely after the
btrfs_map_bio handling that Stefan did. Remains to make sure that
btrfs_map_block are also ok, sofar I don''t see major problems right
now.

As you''re extending the set of return values please remove all the
ENOMEM comments like

btrfs_map_bio():

5278         ret = __btrfs_map_block(root->fs_info, rw, logical,
&map_length, &bbio,
5279                               mirror_num, &raid_map);
5280         if (ret) /* -ENOMEM */

so comments and code match.


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