linux-ext4 - Re: [PATCH] libext2fs: fix to read the bitmaps for image file correctly

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 19 Mar 2018 19:28:52 -0400
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     Kazuya Mio <k-mio@...jp.nec.com>
Cc:     "adilger.kernel@...ger.ca" <adilger.kernel@...ger.ca>,
        "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] libext2fs: fix to read the bitmaps for image file
 correctly

I noted one further bug with your patch when I looked more closely at
fixing it; and that's that in the e2image format, the data for the
bitmap is written in a compacted format.  For example, consider this
mke2fs command:

	mke2fs -t ext4 -b 1024 -N 600 -g 3000 /tmp/foo.img 12M

This will create a file system with 120 inodes per group, and 3000
blocks per group, with 5 block groups.  Each block group will have a
full 1k block and inode allocation bitmap, but only the first 15 bytes
of the inode bitmap, and the first 375 bytes of the block bitmap, will
actually be used.  In the e 2image file, those 375*5 = 1875 bytes will
be store contiguously, and will take up 2 1k blocks worth of space in
the e2i file.  However, in the actual file system, each block group
will have its own 1k block for its portion of the block allocation
bitmap.

What this means is that your patch would not properly read in the
e2image file's bitmap information.  I noticed this when I ran the
following test:

	  dumpe2fs /dev/sda3 > /tmp/a
	  e2image /dev/sda3 /tmp/sda3.e2i
	  dumpe2fs -i /tmp/sda3.e2i > /tmp/b
	  diff /tmp/a /tmp/b

In particular, since usually there are fewer than blocksize*8 inodes
per block group, the above sequence is almost guarnteed to show
discrepancies in the free inode list for any non-trivally non-empty
file system.

As a result, I've used this patch to address the problem which you
were trying to fix.

						- Ted

commit eac24cd3484341bafaf77bf2d823a6aaff0b61c8
Author: Theodore Ts'o <tytso@....edu>
Date:   Mon Mar 19 18:58:09 2018 -0400

    libext2fs: fix reading bitmaps from e2image files

    The loop termination code was broken, so that only the first block's
    worth of bitmap data was getting read.

    Signed-off-by: Theodore Ts'o <tytso@....edu>
    Reported-by: Kazuya Mio <k-mio@...jp.nec.com>

diff --git a/lib/ext2fs/rw_bitmaps.c b/lib/ext2fs/rw_bitmaps.c
index 9db76eb94..0c4fecc48 100644
--- a/lib/ext2fs/rw_bitmaps.c
+++ b/lib/ext2fs/rw_bitmaps.c
@@ -255,7 +255,7 @@ static errcode_t read_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 	if (fs->flags & EXT2_FLAG_IMAGE_FILE) {
 		blk = (fs->image_header->offset_inodemap / fs->blocksize);
 		ino_cnt = fs->super->s_inodes_count;
-		while (inode_nbytes > 0) {
+		while (ino_cnt > 0) {
 			retval = io_channel_read_blk64(fs->image_io, blk++,
 						     1, inode_bitmap);
 			if (retval)
@@ -267,15 +267,14 @@ static errcode_t read_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 					       ino_itr, cnt, inode_bitmap);
 			if (retval)
 				goto cleanup;
-			ino_itr += fs->blocksize << 3;
-			ino_cnt -= fs->blocksize << 3;
-			inode_nbytes -= fs->blocksize;
+			ino_itr += cnt;
+			ino_cnt -= cnt;
 		}
 		blk = (fs->image_header->offset_blockmap /
 		       fs->blocksize);
 		blk_cnt = EXT2_GROUPS_TO_CLUSTERS(fs->super,
 						  fs->group_desc_count);
-		while (block_nbytes > 0) {
+		while (blk_cnt > 0) {
 			retval = io_channel_read_blk64(fs->image_io, blk++,
 						     1, block_bitmap);
 			if (retval)
@@ -287,9 +286,8 @@ static errcode_t read_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 				       blk_itr, cnt, block_bitmap);
 			if (retval)
 				goto cleanup;
-			blk_itr += fs->blocksize << 3;
-			blk_cnt -= fs->blocksize << 3;
-			block_nbytes -= fs->blocksize;
+			blk_itr += cnt;
+			blk_cnt -= cnt;
 		}
 		goto success_cleanup;
 	}