From 8b5273d5d10d180d981381353d2c88fee5744a2d Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Fri, 22 Jun 2018 10:46:52 -0400 Subject: [PATCH] libext2fs: don't corrupt an blkmap64_rb when marking a range of size zero Calling ext2fs_mark_block_bitmap_range2() with a count of zero can end up corrupting the red-black block bitmap structure, since a an entry in the rbtree with zero-length extent can end up causing the find_first_{zero,set} operations to return incorrect results. This was found by Adam Buchbinder, who created a fuzzed file system using which AFL that caused e2fsck to hang in an infinite loop in in e2fsck's readahead code. Added a regression test to detect this failure. Signed-off-by: Theodore Ts'o --- lib/ext2fs/blkmap64_rb.c | 41 +++++++++++++++++++++++++---------------- lib/ext2fs/tst_bitmaps_cmds | 2 ++ lib/ext2fs/tst_bitmaps_exp | 4 ++++ 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/lib/ext2fs/blkmap64_rb.c b/lib/ext2fs/blkmap64_rb.c index 5570477..4cbfb1e 100644 --- a/lib/ext2fs/blkmap64_rb.c +++ b/lib/ext2fs/blkmap64_rb.c @@ -74,15 +74,15 @@ static void print_tree(struct rb_root *root) struct rb_node *node = NULL; struct bmap_rb_extent *ext; - printf("\t\t\t=================================\n"); + fprintf(stderr, "\t\t\t=================================\n"); node = ext2fs_rb_first(root); for (node = ext2fs_rb_first(root); node != NULL; node = ext2fs_rb_next(node)) { ext = node_to_extent(node); - printf("\t\t\t--> (%llu -> %llu)\n", + fprintf(stderr, "\t\t\t--> (%llu -> %llu)\n", ext->start, ext->start + ext->count); } - printf("\t\t\t=================================\n"); + fprintf(stderr, "\t\t\t=================================\n"); } static void check_tree(struct rb_root *root, const char *msg) @@ -94,35 +94,41 @@ static void check_tree(struct rb_root *root, const char *msg) node = ext2fs_rb_next(node)) { ext = node_to_extent(node); if (ext->count == 0) { - printf("Tree Error: count is zero\n"); - printf("extent: %llu -> %llu (%llu)\n", ext->start, - ext->start + ext->count, ext->count); + fprintf(stderr, "Tree Error: count is zero\n"); + fprintf(stderr, "extent: %llu -> %llu (%llu)\n", + ext->start, ext->start + ext->count, + ext->count); goto err_out; } if (ext->start + ext->count < ext->start) { - printf("Tree Error: start or count is crazy\n"); - printf("extent: %llu -> %llu (%llu)\n", ext->start, - ext->start + ext->count, ext->count); + fprintf(stderr, + "Tree Error: start or count is crazy\n"); + fprintf(stderr, "extent: %llu -> %llu (%llu)\n", + ext->start, ext->start + ext->count, + ext->count); goto err_out; } if (old) { if (old->start > ext->start) { - printf("Tree Error: start is crazy\n"); - printf("extent: %llu -> %llu (%llu)\n", + fprintf(stderr, "Tree Error: start is crazy\n"); + fprintf(stderr, "extent: %llu -> %llu (%llu)\n", old->start, old->start + old->count, old->count); - printf("extent next: %llu -> %llu (%llu)\n", + fprintf(stderr, + "extent next: %llu -> %llu (%llu)\n", ext->start, ext->start + ext->count, ext->count); goto err_out; } if ((old->start + old->count) >= ext->start) { - printf("Tree Error: extent is crazy\n"); - printf("extent: %llu -> %llu (%llu)\n", + fprintf(stderr, + "Tree Error: extent is crazy\n"); + fprintf(stderr, "extent: %llu -> %llu (%llu)\n", old->start, old->start + old->count, old->count); - printf("extent next: %llu -> %llu (%llu)\n", + fprintf(stderr, + "extent next: %llu -> %llu (%llu)\n", ext->start, ext->start + ext->count, ext->count); goto err_out; @@ -133,7 +139,7 @@ static void check_tree(struct rb_root *root, const char *msg) return; err_out: - printf("%s\n", msg); + fprintf(stderr, "%s\n", msg); print_tree(root); exit(1); } @@ -391,6 +397,9 @@ static int rb_insert_extent(__u64 start, __u64 count, struct bmap_rb_extent *ext; int retval = 0; + if (count == 0) + return 0; + bp->rcursor_next = NULL; ext = bp->wcursor; if (ext) { diff --git a/lib/ext2fs/tst_bitmaps_cmds b/lib/ext2fs/tst_bitmaps_cmds index 7492592..dc116b1 100644 --- a/lib/ext2fs/tst_bitmaps_cmds +++ b/lib/ext2fs/tst_bitmaps_cmds @@ -1,3 +1,4 @@ +setb 12 0 setb 12 setb 12 clearb 12 @@ -37,6 +38,7 @@ setb 15 testb 12 7 clearb 15 testb 12 7 +setb 12 0 setb 12 7 dump_bb seti 2 diff --git a/lib/ext2fs/tst_bitmaps_exp b/lib/ext2fs/tst_bitmaps_exp index 6b22666..9cfea13 100644 --- a/lib/ext2fs/tst_bitmaps_exp +++ b/lib/ext2fs/tst_bitmaps_exp @@ -1,5 +1,7 @@ tst_bitmaps 1.0. Type '?' for a list of commands. +tst_bitmaps: setb 12 0 +Marking blocks 12 to 11 tst_bitmaps: setb 12 Setting block 12, was clear before tst_bitmaps: setb 12 @@ -79,6 +81,8 @@ tst_bitmaps: clearb 15 Clearing block 15, was set before tst_bitmaps: testb 12 7 Blocks 12 to 18 are all clear. +tst_bitmaps: setb 12 0 +Marking blocks 12 to 11 tst_bitmaps: setb 12 7 Marking blocks 12 to 18 tst_bitmaps: dump_bb -- 1.8.3.1