From e6402f03899af646016864255bf485f2994a9c2b Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 23 Mar 2023 00:44:21 +0000 Subject: [PATCH] AOSP: ext2simg: fix off-by-one errors causing corruption The chunk_end parameter to add_chunk() is exclusive, but two callers incorrectly treat it as inclusive: when the maximum chunk length of 'INT32_MAX - 12' bytes is reached, and when a chunk extends to the very end of the filesystem. The result is that the output simg file contains zeroes for the last block of these chunks instead of the correct data. A related bug is that the expanded size of the simg file is set to the filesystem size (in blocks) minus s_first_data_block. On filesystems where s_first_data_block != 0, i.e. 1K blocksize filesystems without bigalloc enabled, this truncates the last block of the filesystem. Fix these bugs by (a) making add_chunk() take the chunk length and passing the correct values, and (b) using the filesystem size properly. Here is a reproducer that shows the last block of the filesystem being truncated (bsize=1024) and being corrupted with zeroes (bsize=4096): for bsize in 1024 4096; do rm -f ext4.img mkfs.ext4 -b $bsize ext4.img 10000 mkdir -p mnt sudo mount -t ext4 -o loop ext4.img mnt sudo cp /dev/urandom mnt/fill sudo umount mnt ext2simg ext4.img ext4.simg simg2img ext4.simg ext4.img.restored cmp ext4.img ext4.img.restored done Fixes: db6f320912cf ("AOSP: android: add the ext2simg tool") Reported-by: Clemens Lang Change-Id: I3b64c4fbffa5821b431f29e99b36168617da7563 Signed-off-by: Eric Biggers From AOSP commit: 1e498908c6ac13b4d5ec0117f4ddcd577aac607e --- contrib/android/ext2simg.c | 49 +++++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/contrib/android/ext2simg.c b/contrib/android/ext2simg.c index 017e16f..9ef54cf 100644 --- a/contrib/android/ext2simg.c +++ b/contrib/android/ext2simg.c @@ -63,12 +63,16 @@ static struct buf_item { void *buf[0]; } *buf_list; -static void add_chunk(ext2_filsys fs, struct sparse_file *s, blk_t chunk_start, blk_t chunk_end) +/* + * Add @num_blks blocks, starting at index @chunk_start, of the filesystem @fs + * to the sparse file @s. + */ +static void add_chunk(ext2_filsys fs, struct sparse_file *s, + blk_t chunk_start, int num_blks) { int retval; - unsigned int nb_blk = chunk_end - chunk_start; - size_t len = nb_blk * fs->blocksize; - int64_t offset = (int64_t)chunk_start * (int64_t)fs->blocksize; + uint64_t len = (uint64_t)num_blks * fs->blocksize; + int64_t offset = (int64_t)chunk_start * fs->blocksize; if (params.overwrite_input == false) { if (sparse_file_add_file(s, params.in_file, offset, len, chunk_start) < 0) @@ -86,9 +90,9 @@ static void add_chunk(ext2_filsys fs, struct sparse_file *s, blk_t chunk_start, buf_list = bi; } - retval = io_channel_read_blk64(fs->io, chunk_start, nb_blk, bi->buf); + retval = io_channel_read_blk64(fs->io, chunk_start, num_blks, bi->buf); if (retval < 0) - ext2fs_fatal(retval, "reading block %u - %u", chunk_start, chunk_end); + ext2fs_fatal(retval, "reading data from %s", params.in_file); if (sparse_file_add_data(s, bi->buf, len, chunk_start) < 0) sparse_fatal("adding data to the sparse file"); @@ -112,7 +116,7 @@ static struct sparse_file *ext_to_sparse(const char *in_file) ext2_filsys fs; struct sparse_file *s; int64_t chunk_start = -1; - blk_t first_blk, last_blk, nb_blk, cur_blk; + blk_t fs_blks, cur_blk; retval = ext2fs_open(in_file, 0, 0, 0, unix_io_manager, &fs); if (retval) @@ -122,11 +126,9 @@ static struct sparse_file *ext_to_sparse(const char *in_file) if (retval) ext2fs_fatal(retval, "while reading block bitmap of %s", in_file); - first_blk = ext2fs_get_block_bitmap_start2(fs->block_map); - last_blk = ext2fs_get_block_bitmap_end2(fs->block_map); - nb_blk = last_blk - first_blk + 1; + fs_blks = ext2fs_blocks_count(fs->super); - s = sparse_file_new(fs->blocksize, (uint64_t)fs->blocksize * (uint64_t)nb_blk); + s = sparse_file_new(fs->blocksize, (uint64_t)fs_blks * fs->blocksize); if (!s) sparse_fatal("creating sparse file"); @@ -140,22 +142,37 @@ static struct sparse_file *ext_to_sparse(const char *in_file) */ int64_t max_blk_per_chunk = (INT32_MAX - 12) / fs->blocksize; - /* Iter on the blocks to merge contiguous chunk */ - for (cur_blk = first_blk; cur_blk <= last_blk; ++cur_blk) { + /* + * Iterate through the filesystem's blocks, identifying "chunks" that + * are contiguous ranges of blocks that are in-use by the filesystem. + * Add each chunk to the sparse_file. + */ + for (cur_blk = ext2fs_get_block_bitmap_start2(fs->block_map); + cur_blk < fs_blks; ++cur_blk) { if (ext2fs_test_block_bitmap2(fs->block_map, cur_blk)) { + /* + * @cur_blk is in-use. Append it to the pending chunk + * if there is one, otherwise start a new chunk. + */ if (chunk_start == -1) { chunk_start = cur_blk; } else if (cur_blk - chunk_start + 1 == max_blk_per_chunk) { - add_chunk(fs, s, chunk_start, cur_blk); + /* + * Appending @cur_blk to the pending chunk made + * it reach the maximum length, so end it. + */ + add_chunk(fs, s, chunk_start, max_blk_per_chunk); chunk_start = -1; } } else if (chunk_start != -1) { - add_chunk(fs, s, chunk_start, cur_blk); + /* @cur_blk is not in-use, so end the pending chunk. */ + add_chunk(fs, s, chunk_start, cur_blk - chunk_start); chunk_start = -1; } } + /* If there's still a pending chunk, end it. */ if (chunk_start != -1) - add_chunk(fs, s, chunk_start, cur_blk - 1); + add_chunk(fs, s, chunk_start, cur_blk - chunk_start); ext2fs_free(fs); return s; -- 1.8.3.1