libext2fs: fix bounds check of the bitmap test range in get_free_blocks2

In the loop in ext2fs_get_free_blocks2, we ask the bitmap if there's a
range of free blocks starting at "b" and ending at "b + num - 1".
That quantity is the number of the last block in the range.  Since
ext2fs_blocks_count() returns the number of blocks and not the number
of the last block in the filesystem, the check is incorrect.

Put in a shortcut to exit the loop if finish > start, because in that
case it's obvious that we don't need to reset to the beginning of the
FS to continue the search for blocks.  This is needed to terminate the
loop because the broken test meant that b could get large enough to
equal finish, which would end the while loop.

The attached testcase shows that with the off by one error, it is
possible to throw e2fsck into an infinite loop while it tries to
find space for the inode table even though there's no space for one.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>

diff --git a/lib/ext2fs/alloc.c b/lib/ext2fs/alloc.c
index 0acbc4e6b41fca4e9ac50b7c4d3e105eb1cadc45..2061b9d33870b47a9a7226dbe73a9d79a1316b90 100644 (file)
--- a/lib/ext2fs/alloc.c
+++ b/lib/ext2fs/alloc.c
@@ -257,8 +257,11 @@ errcode_t ext2fs_get_free_blocks2(ext2_filsys fs, blk64_t start, blk64_t finish,
        b &= ~(c_ratio - 1);
        finish &= ~(c_ratio -1);
        do {
-               if (b+num-1 > ext2fs_blocks_count(fs->super))
+               if (b + num - 1 >= ext2fs_blocks_count(fs->super)) {
+                       if (finish > start)
+                               return EXT2_ET_BLOCK_ALLOC_FAIL;
                        b = fs->super->s_first_data_block;
+               }
                if (ext2fs_fast_test_block_bitmap_range2(map, b, num)) {
                        *ret = b;
                        return 0;

diff --git a/tests/f_boundscheck/expect.1 b/tests/f_boundscheck/expect.1
new file mode 100644 (file)
index 0000000..c2170b8
--- /dev/null
+++ b/tests/f_boundscheck/expect.1
@@ -0,0 +1,25 @@
+ext2fs_check_desc: Corrupt group descriptor: bad block for inode table
+../e2fsck/e2fsck: Group descriptors look bad... trying backup blocks...
+../e2fsck/e2fsck: Bad magic number in super-block while using the backup blocks../e2fsck/e2fsck: going back to original superblock
+Note: if several inode or block bitmap blocks or part
+of the inode table require relocation, you may wish to try
+running e2fsck with the '-b 8193' option first.  The problem
+may lie only with the primary block group descriptors, and
+the backup block group descriptors may be OK.
+
+Inode table for group 1 is not in group.  (block 4294967295)
+WARNING: SEVERE DATA LOSS POSSIBLE.
+Relocate? yes
+
+One or more block group descriptor checksums are invalid.  Fix? yes
+
+Group descriptor 1 checksum is 0x6ea2, should be 0x7edd.  FIXED.
+Pass 1: Checking inodes, blocks, and sizes
+Error allocating 256 contiguous block(s) in block group 1 for inode table: Could not allocate block in ext2 filesystem
+e2fsck: aborted
+
+test_filesys: ***** FILE SYSTEM WAS MODIFIED *****
+
+test_filesys: ********** WARNING: Filesystem still has errors **********
+
+Exit status is 0

diff --git a/tests/f_boundscheck/expect.2 b/tests/f_boundscheck/expect.2
new file mode 100644 (file)
index 0000000..c2170b8
--- /dev/null
+++ b/tests/f_boundscheck/expect.2
@@ -0,0 +1,25 @@
+ext2fs_check_desc: Corrupt group descriptor: bad block for inode table
+../e2fsck/e2fsck: Group descriptors look bad... trying backup blocks...
+../e2fsck/e2fsck: Bad magic number in super-block while using the backup blocks../e2fsck/e2fsck: going back to original superblock
+Note: if several inode or block bitmap blocks or part
+of the inode table require relocation, you may wish to try
+running e2fsck with the '-b 8193' option first.  The problem
+may lie only with the primary block group descriptors, and
+the backup block group descriptors may be OK.
+
+Inode table for group 1 is not in group.  (block 4294967295)
+WARNING: SEVERE DATA LOSS POSSIBLE.
+Relocate? yes
+
+One or more block group descriptor checksums are invalid.  Fix? yes
+
+Group descriptor 1 checksum is 0x6ea2, should be 0x7edd.  FIXED.
+Pass 1: Checking inodes, blocks, and sizes
+Error allocating 256 contiguous block(s) in block group 1 for inode table: Could not allocate block in ext2 filesystem
+e2fsck: aborted
+
+test_filesys: ***** FILE SYSTEM WAS MODIFIED *****
+
+test_filesys: ********** WARNING: Filesystem still has errors **********
+
+Exit status is 0

diff --git a/tests/f_boundscheck/image.bz2 b/tests/f_boundscheck/image.bz2
new file mode 100644 (file)
index 0000000..098d02e
Binary files /dev/null and b/tests/f_boundscheck/image.bz2 differ

diff --git a/tests/f_boundscheck/name b/tests/f_boundscheck/name
new file mode 100644 (file)
index 0000000..192d279
--- /dev/null
+++ b/tests/f_boundscheck/name
@@ -0,0 +1 @@
+infinite loop due to off by one error when finding free space for inode table relocation

diff --git a/tests/f_boundscheck/script b/tests/f_boundscheck/script
new file mode 100755 (executable)
index 0000000..fbbce62
--- /dev/null
+++ b/tests/f_boundscheck/script
@@ -0,0 +1,32 @@
+#!/bin/bash
+
+FSCK_OPT=-fy
+IMAGE=$test_dir/image.bz2
+
+bzip2 -d < $IMAGE > $TMPFILE
+#e2label $TMPFILE test_filesys
+
+# Run fsck to fix things?
+EXP1=$test_dir/expect.1
+OUT1=$test_name.1.log
+rm -rf $test_name.failed $test_name.ok
+
+$FSCK $FSCK_OPT $TMPFILE 2>&1 | head -n 1000 | tail -n +2 > $OUT1
+echo "Exit status is $?" >> $OUT1
+
+# Run a second time
+EXP2=$test_dir/expect.2
+OUT2=$test_name.2.log
+
+$FSCK $FSCK_OPT $TMPFILE 2>&1 | head -n 1000 | tail -n +2 > $OUT2
+echo "Exit status is $?" >> $OUT2
+
+# Figure out what happened
+if cmp -s $EXP1 $OUT1 && cmp -s $EXP2 $OUT2; then
+       echo "$test_name: $test_description: ok"
+       touch $test_name.ok
+else
+       echo "$test_name: $test_description: failed"
+       diff -u $EXP1 $OUT1 >> $test_name.failed
+       diff -u $EXP2 $OUT2 >> $test_name.failed
+fi