On Mar 19, 2018, at 5:28 PM, Theodore Y. Ts'o <tytso@xxxxxxx> 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@xxxxxxx>
> 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@xxxxxxx>
> Reported-by: Kazuya Mio <k-mio@xxxxxxxxxxxxx>
>
> 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
Attachment:
signature.asc
Description: Message signed with OpenPGP
