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 <clemens.lang@bmw.de>
Change-Id: I3b64c4fbffa5821b431f29e99b36168617da7563
Signed-off-by: Eric Biggers <ebiggers@google.com>
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 @@
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 @@
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 @@
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 @@
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 @@
*/
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;
