From 1e498908c6ac13b4d5ec0117f4ddcd577aac607e Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 23 Mar 2023 00:44:21 +0000 Subject: 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 --- 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 017e16ff..9ef54cff 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; -- cgit v1.2.3 From 0749f83a2cf4c134a2403701ab78388500e53f76 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 23 Mar 2023 00:44:21 +0000 Subject: ext2simg: fix same_file() to check st_dev File identity is determined by the combination of st_dev and st_ino, not by st_ino alone. This fixes a bug where ext2simg would needlessly make a copy of all the data when the input and output files happened to have the same st_ino. Fixes: db6f320912cf ("AOSP: android: add the ext2simg tool") Change-Id: I94e4bf57d9f91b31e5438768805e9f10bec3411d Signed-off-by: Eric Biggers --- contrib/android/ext2simg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/android/ext2simg.c b/contrib/android/ext2simg.c index 9ef54cff..1bc23080 100644 --- a/contrib/android/ext2simg.c +++ b/contrib/android/ext2simg.c @@ -189,7 +189,7 @@ static bool same_file(const char *in, const char *out) ext2fs_fatal(errno, "stat %s\n", in); if (lstat(out, &st2) == -1) ext2fs_fatal(errno, "stat %s\n", out); - return st1.st_ino == st2.st_ino; + return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino; } int main(int argc, char *argv[]) -- cgit v1.2.3 From 2728c6e766976acbf442d3721f2d93960e13682e Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 23 Mar 2023 00:44:21 +0000 Subject: ext2simg: use bool where appropriate For the values that get used as the 'bool' parameters of sparse_file_write(), use 'bool' in ext2simg too. No change in behavior. Change-Id: I05f7d6fd3027eb10231c035f9fdc8e946e2c4c90 Signed-off-by: Eric Biggers --- contrib/android/ext2simg.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/contrib/android/ext2simg.c b/contrib/android/ext2simg.c index 1bc23080..efb4e41a 100644 --- a/contrib/android/ext2simg.c +++ b/contrib/android/ext2simg.c @@ -24,16 +24,14 @@ #include struct { - int crc; - int sparse; - int gzip; + bool crc; + bool sparse; + bool gzip; char *in_file; char *out_file; bool overwrite_input; } params = { - .crc = 0, - .sparse = 1, - .gzip = 0, + .sparse = true, }; #define ext2fs_fatal(Retval, Format, ...) \ @@ -201,13 +199,13 @@ int main(int argc, char *argv[]) while ((opt = getopt(argc, argv, "czS")) != -1) { switch(opt) { case 'c': - params.crc = 1; + params.crc = true; break; case 'z': - params.gzip = 1; + params.gzip = true; break; case 'S': - params.sparse = 0; + params.sparse = false; break; default: usage(argv[0]); -- cgit v1.2.3 From c88ea796fbf7f4c79155196ec483681b3733bbff Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 23 Mar 2023 00:44:21 +0000 Subject: ext2simg: use a standard flexible array Use a standard flexible array instead of a nonstandard zero-length array. No change in behavior. Change-Id: Ifdce24f5d6e2471634bb785527def3fe8fefc202 Signed-off-by: Eric Biggers --- contrib/android/ext2simg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/android/ext2simg.c b/contrib/android/ext2simg.c index efb4e41a..9b594f37 100644 --- a/contrib/android/ext2simg.c +++ b/contrib/android/ext2simg.c @@ -58,7 +58,7 @@ static void usage(char *path) static struct buf_item { struct buf_item *next; - void *buf[0]; + void *buf[]; } *buf_list; /* -- cgit v1.2.3 From 7d0f5c1aca332da22e4878f5825e0ffb5122f96b Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 23 Mar 2023 00:44:22 +0000 Subject: ext2simg: clean up add_chunk() Remove a level of indentation, check a bool in the normal way, and simplify the linked list handling. No change in behavior. Change-Id: I12589a254f155b1c40418458a666b87c7ef5c1cf Signed-off-by: Eric Biggers --- contrib/android/ext2simg.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/contrib/android/ext2simg.c b/contrib/android/ext2simg.c index 9b594f37..811c60d6 100644 --- a/contrib/android/ext2simg.c +++ b/contrib/android/ext2simg.c @@ -68,33 +68,27 @@ static struct buf_item { static void add_chunk(ext2_filsys fs, struct sparse_file *s, blk_t chunk_start, int num_blks) { - int retval; uint64_t len = (uint64_t)num_blks * fs->blocksize; int64_t offset = (int64_t)chunk_start * fs->blocksize; + struct buf_item *bi; + int retval; - if (params.overwrite_input == false) { + if (!params.overwrite_input) { if (sparse_file_add_file(s, params.in_file, offset, len, chunk_start) < 0) sparse_fatal("adding data to the sparse file"); - } else { - /* - * The input file will be overwritten, make a copy of - * the blocks - */ - struct buf_item *bi = calloc(1, sizeof(struct buf_item) + len); - if (buf_list == NULL) - buf_list = bi; - else { - bi->next = buf_list; - buf_list = bi; - } + return; + } - retval = io_channel_read_blk64(fs->io, chunk_start, num_blks, bi->buf); - if (retval < 0) - ext2fs_fatal(retval, "reading data from %s", params.in_file); + /* The input file will be overwritten, so make a copy of the blocks. */ + bi = calloc(1, sizeof(*bi) + len); + bi->next = buf_list; + buf_list = bi; + retval = io_channel_read_blk64(fs->io, chunk_start, num_blks, bi->buf); + if (retval < 0) + 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"); - } + if (sparse_file_add_data(s, bi->buf, len, chunk_start) < 0) + sparse_fatal("adding data to the sparse file"); } static void free_chunks(void) -- cgit v1.2.3 From 8fff11068c100be627745967992fb88759dea9c1 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 23 Mar 2023 00:44:22 +0000 Subject: ext2simg: clean up integer types and check for too-large fs libsparse assumes 32-bit block numbers. Also, ext2simg might read nearly the entire filesystem into memory. Therefore, make ext2simg use appropriate integer types, and explicitly check for when the filesystem is too large or allocating memory failed. Change-Id: Ic415d0e974dce2b4ff6e7fa9265f6e86d371a274 Signed-off-by: Eric Biggers --- contrib/android/ext2simg.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/contrib/android/ext2simg.c b/contrib/android/ext2simg.c index 811c60d6..13a9d567 100644 --- a/contrib/android/ext2simg.c +++ b/contrib/android/ext2simg.c @@ -80,7 +80,11 @@ static void add_chunk(ext2_filsys fs, struct sparse_file *s, } /* The input file will be overwritten, so make a copy of the blocks. */ + if (len > SIZE_MAX - sizeof(*bi)) + sparse_fatal("filesystem is too large"); bi = calloc(1, sizeof(*bi) + len); + if (!bi) + sparse_fatal("out of memory"); bi->next = buf_list; buf_list = bi; retval = io_channel_read_blk64(fs->io, chunk_start, num_blks, bi->buf); @@ -102,6 +106,16 @@ static void free_chunks(void) } } +static blk_t fs_blocks_count(ext2_filsys fs) +{ + blk64_t blks = ext2fs_blocks_count(fs->super); + + /* libsparse assumes 32-bit block numbers. */ + if ((blk_t)blks != blks) + sparse_fatal("filesystem is too large"); + return blks; +} + static struct sparse_file *ext_to_sparse(const char *in_file) { errcode_t retval; @@ -118,7 +132,7 @@ static struct sparse_file *ext_to_sparse(const char *in_file) if (retval) ext2fs_fatal(retval, "while reading block bitmap of %s", in_file); - fs_blks = ext2fs_blocks_count(fs->super); + fs_blks = fs_blocks_count(fs); s = sparse_file_new(fs->blocksize, (uint64_t)fs_blks * fs->blocksize); if (!s) @@ -132,7 +146,7 @@ static struct sparse_file *ext_to_sparse(const char *in_file) * larger than INT32_MAX (32-bit _and_ 64-bit systems). * Make sure we do not create chunks larger than this limit. */ - int64_t max_blk_per_chunk = (INT32_MAX - 12) / fs->blocksize; + int32_t max_blk_per_chunk = (INT32_MAX - 12) / fs->blocksize; /* * Iterate through the filesystem's blocks, identifying "chunks" that -- cgit v1.2.3 From 60634ff32a0d8c0d7942c6e522a74a5051d4b6e9 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 23 Mar 2023 02:49:43 +0000 Subject: ext2simg: fix error check of io_channel_read_blk64() Check the return value of io_channel_read_blk64() correctly, considering that it returns an errcode_t, which can be positive. Fixes: db6f320912cf ("AOSP: android: add the ext2simg tool") Change-Id: Iafc6c0169bc8ac79198f285da0246ff3b841ded8 Signed-off-by: Eric Biggers --- contrib/android/ext2simg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/android/ext2simg.c b/contrib/android/ext2simg.c index 13a9d567..d1b5dc4e 100644 --- a/contrib/android/ext2simg.c +++ b/contrib/android/ext2simg.c @@ -88,7 +88,7 @@ static void add_chunk(ext2_filsys fs, struct sparse_file *s, bi->next = buf_list; buf_list = bi; retval = io_channel_read_blk64(fs->io, chunk_start, num_blks, bi->buf); - if (retval < 0) + if (retval) ext2fs_fatal(retval, "reading data from %s", params.in_file); if (sparse_file_add_data(s, bi->buf, len, chunk_start) < 0) -- cgit v1.2.3 From 08c122f12fc231029a74c24b969e337203c7b6e2 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 23 Mar 2023 02:52:09 +0000 Subject: ext2simg: fix same_file() with symlinks Fix same_file() to use stat() instead of lstat() when checking the paths, so that symlinks are dereferenced. This is needed to be consistent with how the paths are actually accessed later. Otherwise, not all cases where the input and output file are the same are detected. Also just use the stat() result to check whether the output file exists, instead of using a separate call to access(). Fixes: db6f320912cf ("AOSP: android: add the ext2simg tool") Change-Id: Ie36981f9dbc19494732f518488a75fb92c0f0343 Signed-off-by: Eric Biggers --- contrib/android/ext2simg.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contrib/android/ext2simg.c b/contrib/android/ext2simg.c index d1b5dc4e..2bf76b91 100644 --- a/contrib/android/ext2simg.c +++ b/contrib/android/ext2simg.c @@ -188,13 +188,13 @@ static bool same_file(const char *in, const char *out) { struct stat st1, st2; - if (access(out, F_OK) == -1) - return false; - - if (lstat(in, &st1) == -1) + if (stat(in, &st1) == -1) ext2fs_fatal(errno, "stat %s\n", in); - if (lstat(out, &st2) == -1) + if (stat(out, &st2) == -1) { + if (errno == ENOENT) + return false; ext2fs_fatal(errno, "stat %s\n", out); + } return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino; } -- cgit v1.2.3 From 0a5c3705c30c87afbe28a64381a44262d574fd83 Mon Sep 17 00:00:00 2001 From: Sadaf Ebrahimi Date: Thu, 23 Mar 2023 16:05:40 +0000 Subject: Adding METADATA file to e2fsprogs Test: TreeHugger Change-Id: Iefd48e3c50ce6d402624e2acaf650c6ef594cd64 --- METADATA | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/METADATA b/METADATA index 6d8601bb..d5d84871 100644 --- a/METADATA +++ b/METADATA @@ -1,3 +1,19 @@ +# This project was upgraded with external_updater. +# Usage: tools/external_updater/updater.sh update e2fsprogs +# For more info, check https://cs.android.com/android/platform/superproject/+/master:tools/external_updater/README.md + +name: "e2fsprogs" +description: "Ext2/3/4 filesystem userspace utilities" third_party { + url { + type: GIT + value: "https://git.kernel.org/pub/scm/fs/ext2/e2fsprogs.git" + } + version: "v1.46.6" license_type: RESTRICTED + last_upgrade_date { + year: 2023 + month: 3 + day: 23 + } } -- cgit v1.2.3