From 087be2e3c63c43b50969c95f643b84e2370f0942 Mon Sep 17 00:00:00 2001 From: Dmitry Ermakov <7954832+dimerr@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:54:03 +0000 Subject: [PATCH 1/2] mtd: an alias is a partition others cover, not one that holds another mtd_mark_aliases() dropped any partition that contained another whole. That is right for Novatek's "all", and wrong for a layout that nests partitions without covering the parent: with firmware 0x50000-0x800000 split into kernel and rootfs, and rootfs_data as the tail of rootfs, it dropped firmware and also rootfs for holding rootfs_data. Since #235, on any kernel that reports offsets, that took 0x250000-0x6d0000 out of everything enumerated. `backup` and `upload --backup` left out the squashfs without a word: 0x380296 bytes for an 8 MB flash in an emulated run of that layout. `restore --raw` refused with "no partition covers 0x250000-0x6d0000", which is how this was found, on a hi3516cv6xx camera. A partition is now an alias only if smaller ones inside it reach from its start to its end. firmware goes, rootfs stays. raw_image_plan() takes the larger of two partitions starting together, so a parent that remains is written through rather than taken for an overlap with its child. Run on an emulated hi3516ev300 with the layout above, placing partitions from sysfs, the command line and the boot log: restore --raw leaves the flash equal to the image, and the backup comes to 0x9302d7 bytes. rootfs_data is in that backup twice, and plain `restore` refuses it with "Offset algorithm error" and the flash unchanged, as it did before. --- src/mtd_alias.c | 19 +++++++++++++------ src/mtd_alias.h | 9 ++++++--- src/mtd_alias_test.c | 33 +++++++++++++++++++++++++++++++++ src/raw_image.c | 10 ++++++++-- src/raw_image_test.c | 30 +++++++++++++++++++++++++++++- 5 files changed, 89 insertions(+), 12 deletions(-) diff --git a/src/mtd_alias.c b/src/mtd_alias.c index 2eec197..8c65315 100644 --- a/src/mtd_alias.c +++ b/src/mtd_alias.c @@ -9,14 +9,21 @@ void mtd_mark_aliases(int n, const uint64_t *off, const bool *has_off, for (int i = 0; i < n; i++) { alias[i] = false; if (has_off[i]) { - for (int j = 0; j < n; j++) { - if (j == i || !has_off[j] || size[j] >= size[i]) - continue; - if (off[j] >= off[i] && off[j] + size[j] <= off[i] + size[i]) { - alias[i] = true; - break; + // How far from its start the partitions inside it reach + uint64_t end = off[i] + size[i], reached = off[i]; + for (bool moved = true; moved && reached < end;) { + moved = false; + for (int j = 0; j < n; j++) { + if (j == i || !has_off[j] || size[j] >= size[i] || + off[j] < off[i] || off[j] + size[j] > end) + continue; + if (off[j] <= reached && off[j] + size[j] > reached) { + reached = off[j] + size[j]; + moved = true; + } } } + alias[i] = reached == end; continue; } uint64_t others = 0; diff --git a/src/mtd_alias.h b/src/mtd_alias.h index 5f98fce..a960d7c 100644 --- a/src/mtd_alias.h +++ b/src/mtd_alias.h @@ -7,9 +7,12 @@ /* Marks alias[i] for each partition that is another name for flash the * others already cover -- Novatek's "all", 0..chip size, after "loader" to - * "encrypt". Where the kernel gives offsets (has_off), a partition that - * contains another one whole is the alias. Without them, one whose size is - * the sum of all the others' is. Backing it up would put the chip in twice. */ + * "encrypt". Where the kernel gives offsets (has_off), it is a partition the + * smaller ones inside it cover from end to end: holding one is not enough. + * OpenIPC's "firmware" is kernel + rootfs and goes, while "rootfs_data" is + * only the tail of "rootfs", which stays because the squashfs is nowhere else. + * Without offsets, an alias is one whose size is the sum of all the others'. + * Backing up an alias would put the chip in twice. */ void mtd_mark_aliases(int n, const uint64_t *off, const bool *has_off, const uint64_t *size, bool *alias); diff --git a/src/mtd_alias_test.c b/src/mtd_alias_test.c index dfb0c7a..c4646ca 100644 --- a/src/mtd_alias_test.c +++ b/src/mtd_alias_test.c @@ -48,6 +48,39 @@ int main(void) { for (int i = 0; i < 4; i++) CHECK(!nalias[i], "NOR partition %d called an alias", i); + // A parent partition with a split inside it (hi3516cv610, 8 MB NOR): + // firmware is kernel + rootfs, while rootfs_data is only rootfs's tail, + // and dropping rootfs for holding it lost the squashfs from backups. + const char *cv[6] = {"u-boot", "env", "firmware", + "kernel", "rootfs", "rootfs_data"}; + uint64_t coff[6] = {0, 0x40000, 0x50000, 0x50000, 0x250000, 0x6d0000}; + uint64_t csize[6] = {0x40000, 0x10000, 0x7b0000, + 0x200000, 0x5b0000, 0x130000}; + bool chas[6] = {true, true, true, true, true, true}, calias[6]; + mtd_mark_aliases(6, coff, chas, csize, calias); + for (int i = 0; i < 6; i++) + CHECK(calias[i] == (i == 2), "%s alias=%d", cv[i], calias[i]); + + // A parent of parents goes too; a parent with a hole in it stays + uint64_t goff[5] = {0, 0x100000, 0, 0x100000, 0x300000}; + uint64_t gsize[5] = {0x800000, 0x700000, 0x100000, 0x200000, 0x500000}; + bool ghas[5] = {true, true, true, true, true}, galias[5]; + mtd_mark_aliases(5, goff, ghas, gsize, galias); + CHECK(galias[0] && galias[1] && !galias[2] && !galias[3] && !galias[4], + "parent of parents"); + uint64_t hoff[3] = {0, 0, 0x200000}; + uint64_t hsize[3] = {0x400000, 0x100000, 0x200000}; + bool hhas[3] = {true, true, true}, halias[3]; + mtd_mark_aliases(3, hoff, hhas, hsize, halias); + CHECK(!halias[0], "a parent with a hole called an alias"); + + // Listed in any order + uint64_t roff[3] = {0x200000, 0, 0}; + uint64_t rsize[3] = {0x200000, 0x400000, 0x200000}; + bool rhas[3] = {true, true, true}, ralias[3]; + mtd_mark_aliases(3, roff, rhas, rsize, ralias); + CHECK(!ralias[0] && ralias[1] && !ralias[2], "out of order"); + // One partition alone is never an alias. uint64_t one = 0x1000000, zero = 0; bool f = false, a; diff --git a/src/raw_image.c b/src/raw_image.c index 510b40b..bc239e8 100644 --- a/src/raw_image.c +++ b/src/raw_image.c @@ -29,7 +29,10 @@ size_t raw_image_plan(const raw_part_t *parts, size_t nparts, } } - // By offset; few enough that this is all the sort it needs + // By offset, the larger first where two start together: a parent that + // nothing could drop (its children leave a hole) covers them, whereas + // picking a child first would make the parent look like an overlap. + // Few enough that this is all the sort it needs. static raw_part_t sorted[64]; if (nparts > sizeof(sorted) / sizeof(sorted[0])) { snprintf(why, whylen, "too many partitions"); @@ -37,7 +40,10 @@ size_t raw_image_plan(const raw_part_t *parts, size_t nparts, } for (size_t k = 0; k < nparts; k++) { size_t at = k; - for (; at && sorted[at - 1].offset > parts[k].offset; at--) + for (; at && (sorted[at - 1].offset > parts[k].offset || + (sorted[at - 1].offset == parts[k].offset && + sorted[at - 1].size < parts[k].size)); + at--) sorted[at] = sorted[at - 1]; sorted[at] = parts[k]; } diff --git a/src/raw_image_test.c b/src/raw_image_test.c index ed4860d..e7af876 100644 --- a/src/raw_image_test.c +++ b/src/raw_image_test.c @@ -114,11 +114,39 @@ int main(void) { p[4].size = ES; planned(p, 5, (int[]){0, 1, 2, 3}, 4); + // hi3516cv610 on a kernel with mtdsplit: what is left once firmware has + // gone as an alias. rootfs_data is the tail of rootfs and is not written + // twice. + layout(p); + p[3].size = 0x5b0000; + p[4] = p[3]; + p[4].num = 4; + p[4].offset = 0x6d0000; + p[4].size = 0x130000; + planned(p, 5, (int[]){0, 1, 2, 3}, 4); + + // A parent still standing is written through, in either listing order + layout(p); + p[2] = (raw_part_t){.num = 2, + .size = 0x7b0000, + .erasesize = ES, + .nor = true, + .has_offset = true, + .offset = 0x50000}; + p[3] = p[2]; + p[3].num = 3; + p[3].size = 0x200000; + planned(p, 4, (int[]){0, 1, 2}, 3); + raw_part_t parent = p[2]; + p[2] = p[3]; + p[3] = parent; + planned(p, 4, (int[]){0, 1, 2}, 3); + // The sizes add up, and only the offsets show these two share flash layout(p); p[2].offset -= ES; p[3].offset += ES; - refused(p, 4, FLASH, "mtd2 overlaps"); + refused(p, 4, FLASH, "no partition covers 0x240000-0x260000"); layout(p); p[3].offset = p[2].offset; p[3].offset += p[2].size - ES; From 51cbef11b980c3e16eb2453ddfcfeaa2a20d07fb Mon Sep 17 00:00:00 2001 From: Dmitry Ermakov <7954832+dimerr@users.noreply.github.com> Date: Wed, 4 Nov 2026 16:11:22 +0000 Subject: [PATCH 2/2] mtd: flash size is what partitions cover, not what their sizes add up to Partitions can nest now that a partially covered parent is kept, so the sum stopped being the flash. The report said 9M for an 8 MB chip, as Qodo found on this PR; it had said 3M before, with the parent dropped. The sum is also where upgrade takes the end of the flash from, which was worse than the report. With rootfs_data inside rootfs and no firmware parent, the settings region ran 0x130000 past the end, those offsets mapped into rootfs_data, which is the tail of the flash, and they erased what had just been written there. In an emulated run of a full nor8m bundle, 8 blocks of the new rootfs at 0x6d0000-0x750000 came out as 0xff, with the upgrade reporting success. Master refused that layout, so this was new in the previous commit. enum_mtd_info() now works out the union of the ranges it keeps, for the report, restore and upgrade alike. A partition with no known offset can only be added on, as before. The same run writes the rootfs intact. --- src/backup.c | 2 ++ src/mtd.c | 19 ++++++++++++++++--- src/mtd.h | 3 +++ src/mtd_alias.c | 26 ++++++++++++++++++++++++++ src/mtd_alias.h | 5 +++++ src/mtd_alias_test.c | 18 ++++++++++++++++++ 6 files changed, 70 insertions(+), 3 deletions(-) diff --git a/src/backup.c b/src/backup.c index 723e650..4c54a60 100644 --- a/src/backup.c +++ b/src/backup.c @@ -1058,6 +1058,7 @@ static int restore_backup(const char *arg, bool skip_env, bool force) { mtd.env_dev = -1; } enum_mtd_info(&mtd, cb_mtd_restore); + mtd.totalsz = mtd_flash_size(); if (skip_env) { if (mtd.env_dev != -1) printf("U-Boot env detected at #%d/0x%x\n", mtd.env_dev, @@ -1506,6 +1507,7 @@ static int do_upgrade(const char *filename, bool force, bool wipe_settings) { mtd_restore_ctx_t mtd = {.env_dev = -1}; enum_mtd_info(&mtd, cb_mtd_restore); + mtd.totalsz = mtd_flash_size(); if (mtd.env_dev == -1) { fprintf(stderr, "Cannot find env, internal error\n"); return 1; diff --git a/src/mtd.c b/src/mtd.c index d207761..4e7fd02 100644 --- a/src/mtd.c +++ b/src/mtd.c @@ -436,6 +436,11 @@ bool mtd_flash_offset(int num, const char *name, uint64_t size, return known; } +// What the last enum_mtd_info() found the partitions to cover +static uint64_t flash_size; + +uint64_t mtd_flash_size() { return flash_size; } + void enum_mtd_info(void *ctx, cb_mtd cb) { FILE *fp; char dev[80]; @@ -506,9 +511,17 @@ void enum_mtd_info(void *ctx, cb_mtd cb) { idx[m++] = i; } mtd_mark_aliases(m, offs, has_off, sizes, alias); - for (int k = 0; k < m; k++) - if (alias[k]) + int kept = 0; + for (int k = 0; k < m; k++) { + if (alias[k]) { mtds[idx[k]].valid = false; + continue; + } + offs[kept] = offs[k]; + has_off[kept] = has_off[k]; + sizes[kept++] = sizes[k]; + } + flash_size = mtd_covered_size(kept, offs, has_off, sizes); for (int i = 0; i < n; i++) { if (mtds[i].valid && !cb(mtds[i].i, mtds[i].name, &mtds[i].mtd, ctx)) @@ -533,7 +546,7 @@ cJSON *get_mtd_info() { cJSON *j_inner = ctx.json; if (ctx.totalsz) - ADD_PARAM_FMT("size", "%dM", ctx.totalsz / 1024 / 1024); + ADD_PARAM_FMT("size", "%dM", (int)(mtd_flash_size() / 1024 / 1024)); if (hal_fmc_mode) { const char *fmc_mode = hal_fmc_mode(); diff --git a/src/mtd.h b/src/mtd.h index e870c49..bb0c6b8 100644 --- a/src/mtd.h +++ b/src/mtd.h @@ -10,6 +10,9 @@ typedef bool (*cb_mtd)(int i, const char *name, struct mtd_info_user *mtd, cJSON *get_mtd_info(); char *open_mtdblock(int i, int *fd, uint32_t size, int flags); void enum_mtd_info(void *ctx, cb_mtd cb); +// How much flash the partitions of the last enum_mtd_info() cover, each byte +// once. Partitions can nest, so adding up what it hands out overstates it. +uint64_t mtd_flash_size(); // Where partition `num` starts on the flash: the kernel's word where it gives // one, then the command line and the boot log, which a long uptime can lose bool mtd_flash_offset(int num, const char *name, uint64_t size, diff --git a/src/mtd_alias.c b/src/mtd_alias.c index 8c65315..2fa9749 100644 --- a/src/mtd_alias.c +++ b/src/mtd_alias.c @@ -34,6 +34,32 @@ void mtd_mark_aliases(int n, const uint64_t *off, const bool *has_off, } } +uint64_t mtd_covered_size(int n, const uint64_t *off, const bool *has_off, + const uint64_t *size) { + uint64_t total = 0, reached = 0; + for (int i = 0; i < n; i++) + if (!has_off[i]) + total += size[i]; + // The lowest start past what is counted, n times at most + for (;;) { + int next = -1; + uint64_t next_from = 0; + for (int i = 0; i < n; i++) { + if (!has_off[i] || off[i] + size[i] <= reached) + continue; + uint64_t from = off[i] < reached ? reached : off[i]; + if (next < 0 || from < next_from) { + next = i; + next_from = from; + } + } + if (next < 0) + return total; + total += off[next] + size[next] - next_from; + reached = off[next] + size[next]; + } +} + // The kernel's memparse(): strtoull with a size suffix static uint64_t memparse(const char *s, const char **end) { char *e; diff --git a/src/mtd_alias.h b/src/mtd_alias.h index a960d7c..c624329 100644 --- a/src/mtd_alias.h +++ b/src/mtd_alias.h @@ -16,6 +16,11 @@ void mtd_mark_aliases(int n, const uint64_t *off, const bool *has_off, const uint64_t *size, bool *alias); +/* How much flash `n` partitions cover, each byte once: nested partitions are + * not a bigger chip. One with no offset can only be added on. */ +uint64_t mtd_covered_size(int n, const uint64_t *off, const bool *has_off, + const uint64_t *size); + /* * Where a partition starts, for kernels before 4.4: they have no * /sys/class/mtd/mtdN/offset. A partition is found by name and size, and is diff --git a/src/mtd_alias_test.c b/src/mtd_alias_test.c index c4646ca..2eec3c7 100644 --- a/src/mtd_alias_test.c +++ b/src/mtd_alias_test.c @@ -81,6 +81,24 @@ int main(void) { mtd_mark_aliases(3, roff, rhas, rsize, ralias); CHECK(!ralias[0] && ralias[1] && !ralias[2], "out of order"); + // What is left of that layout is 8 MB, not the 9.2 of its sizes + uint64_t kept_off[5] = {0, 0x40000, 0x50000, 0x250000, 0x6d0000}; + uint64_t kept_size[5] = {0x40000, 0x10000, 0x200000, 0x5b0000, 0x130000}; + bool kept_has[5] = {true, true, true, true, true}; + uint64_t sz = mtd_covered_size(5, kept_off, kept_has, kept_size); + CHECK(sz == 0x800000, "covered %llx", (unsigned long long)sz); + // In any order, with a hole not counted, and a tail that outruns + uint64_t any_off[4] = {0x700000, 0, 0x600000, 0x100000}; + uint64_t any_size[4] = {0x100000, 0x100000, 0x180000, 0x100000}; + bool any_has[4] = {true, true, true, true}; + sz = mtd_covered_size(4, any_off, any_has, any_size); + CHECK(sz == 0x400000, "covered %llx", (unsigned long long)sz); + // Unknown offsets are all that can be added up, and none at all is 0 + kept_has[1] = false; + sz = mtd_covered_size(5, kept_off, kept_has, kept_size); + CHECK(sz == 0x800000, "partly known %llx", (unsigned long long)sz); + CHECK(!mtd_covered_size(0, NULL, NULL, NULL), "no partitions"); + // One partition alone is never an alias. uint64_t one = 0x1000000, zero = 0; bool f = false, a;