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 for Android: free password hash cracker in your pocket

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

Message-Id: <A0685C48-A7CF-4AEB-9CF4-A8406163E980@dilger.ca>
Date:   Mon, 19 Mar 2018 17:31:49 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     "Theodore Y. Ts'o" <tytso@....edu>
Cc:     Kazuya Mio <k-mio@...jp.nec.com>,
        "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] libext2fs: fix to read the bitmaps for image file
 correctly

On Mar 19, 2018, at 5:28 PM, Theodore Y. Ts'o <tytso@....edu> wrote:
> 
> 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.

It would be useful to have this embodied in a test case, to catch such
issues in the future...

Cheers, Andreas

> 
> 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;
> 	}
> 


Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)