Whamcloud - gitweb
libext2fs: don't corrupt an blkmap64_rb when marking a range of size zero
authorTheodore Ts'o <tytso@mit.edu>
Fri, 22 Jun 2018 14:46:52 +0000 (10:46 -0400)
committerTheodore Ts'o <tytso@mit.edu>
Fri, 22 Jun 2018 15:26:15 +0000 (11:26 -0400)
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 <tytso@mit.edu>
lib/ext2fs/blkmap64_rb.c
lib/ext2fs/tst_bitmaps_cmds
lib/ext2fs/tst_bitmaps_exp

index 5570477..4cbfb1e 100644 (file)
@@ -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) {
index 7492592..dc116b1 100644 (file)
@@ -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
index 6b22666..9cfea13 100644 (file)
@@ -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