diff --git a/README.md b/README.md index 5f0e153..b677e37 100644 --- a/README.md +++ b/README.md @@ -308,7 +308,10 @@ What a `--layout` bundle does on the camera: there (`wtf` on XiongMai); * edits `bootargs` and `bootcmd` rather than replacing them: only `root=`, `mtdparts=` and the size of the kernel read change, so a vendor - bootloader's other arguments survive; + bootloader's other arguments survive. With OpenIPC's own bootloader the + partitions live in a `mtdparts` variable that `bootargs` refers to + (`mtdparts=${mtdparts}`), and the kernel read in `bootcmdnor` as well: + those are edited the same way; * carries the settings: the jffs2 overlay is copied, once it is read-only, into a jffs2 image written where the new `rootfs_data` starts. The camera comes back with its passwords, keys and configuration. `--wipe-settings` @@ -316,6 +319,10 @@ What a `--layout` bundle does on the camera: * sets the U-Boot variables given with `--env`, such as the address the new firmware upgrades itself from. +The images are written straight from the bundle, so the bundle itself is +the memory it needs: put it in `/tmp` (a 10 MB ultimate bundle wants that +much free there), or on an SD card. + Everything that can be refused is refused before any process is stopped: an image larger than its partition, settings that will not fit the new `rootfs_data`, a bootloader environment with no literal `mtdparts=` or no diff --git a/src/backup.c b/src/backup.c index 23afd80..9462b5e 100644 --- a/src/backup.c +++ b/src/backup.c @@ -225,6 +225,11 @@ typedef struct { char sha1[9]; char name[64]; char *data; + // When set, only the first `payload` bytes are in `data` and the rest of + // `size` is erased flash (0xFF): an image is written from where it lies + // rather than copied into a partition-sized buffer. + bool padded; + size_t payload; bool is_ubi; int ubi_device; int vol_id; @@ -913,6 +918,19 @@ static bool do_flash(const char *phase, stored_mtd_t *mtdbackup, printf("%s %s\n", phase, mtdbackup[i].name); size_t chunk = mtd->erasesize; + // One block to pad the end of an image in, allocated on the trial + // pass, before anything is locked in memory + static char *pad; + static size_t padlen; + if (padlen < chunk) { + free(pad); + pad = malloc(chunk); + padlen = pad ? chunk : 0; + if (!pad) { + fprintf(stderr, "Allocation error\n"); + return false; + } + } int cnt = mtdbackup[i].size / chunk; for (int c = 0; c < cnt; c++) { size_t this_offset; @@ -935,8 +953,23 @@ static bool do_flash(const char *phase, stored_mtd_t *mtdbackup, this_offset, mtd->erasesize, mtdbackup[i].data + c * chunk, chunk); #else - if (!mtd_write(newi, this_offset, mtd->erasesize, - mtdbackup[i].data + c * chunk, chunk)) { + // A pointer into the data only where there is data: the + // settings partition of --wipe-settings has none at all + const char *src; + size_t off = c * chunk; + if (mtdbackup[i].padded && + off + chunk > mtdbackup[i].payload) { + size_t have = off < mtdbackup[i].payload + ? mtdbackup[i].payload - off + : 0; + memset(pad, 0xff, chunk); + if (have) + memcpy(pad, mtdbackup[i].data + off, have); + src = pad; + } else + src = mtdbackup[i].data + off; + if (!mtd_write(newi, this_offset, mtd->erasesize, src, + chunk)) { fprintf(stderr, "\nSomething went wrong, aborting...\n"); return false; @@ -1205,8 +1238,8 @@ static void skip_signals() { * jffs2 image written there along with the kernel and the rootfs. */ typedef struct { - char dir[80]; // where the old settings partition is mounted - char *region; // the new settings partition's contents, flash-sized + char dir[80]; // where the old settings partition is mounted + stored_mtd_t *region; // the new settings partition, as written size_t region_len, erasesize; } carry_t; @@ -1256,12 +1289,22 @@ static bool build_carry(void *arg) { jffs2_img_t *img = jffs2_new(c->erasesize, c->region_len - headroom); bool ok = img && jffs2_add_tree(img, c->dir, "work"); if (ok) { + // Only the image is kept; the rest of the partition is written as + // erased flash size_t len; const uint8_t *data = jffs2_data(img, &len); - memset(c->region, 0xff, c->region_len); - memcpy(c->region, data, len); - printf("Settings: %d entries in %zu of %zu KB\n", jffs2_entries(img), - len / 1024, c->region_len / 1024); + char *copy = malloc(len); + if (copy) { + memcpy(copy, data, len); + free(c->region->data); + c->region->data = copy; + c->region->payload = len; + printf("Settings: %d entries in %zu of %zu KB\n", + jffs2_entries(img), len / 1024, c->region_len / 1024); + } else { + fprintf(stderr, "Cannot carry the settings: out of memory\n"); + ok = false; + } } else fprintf(stderr, "Cannot carry the settings: %s\n", img ? jffs2_error(img) : "out of memory"); @@ -1447,11 +1490,19 @@ static int do_upgrade(const char *filename, bool force, bool wipe_settings) { } bool keep_front = !writes_boot && !mtdparts_pr && !skip_envs && !offset; if (keep_front) { + // The partitions are named in bootargs, or, as OpenIPC's own + // bootloader keeps them, in a variable bootargs refers to char id[64]; - if (!upgrade_mtd_id(uboot_env_get_param("bootargs"), id, sizeof(id))) { - fprintf(stderr, "The bootloader's bootargs name no partitions " - "(no literal mtdparts=), so there is no telling " - "what to keep. Nothing was written.\n"); + const char *ba = uboot_env_get_param("bootargs"); + if (!upgrade_mtd_id(ba, id, sizeof(id)) && + !(upgrade_bootargs_mtdparts_is_var(ba) && + upgrade_mtd_id_value(uboot_env_get_param("mtdparts"), id, + sizeof(id)))) { + fprintf(stderr, "The bootloader's environment names no " + "partitions (no literal mtdparts= in bootargs or " + "in a mtdparts variable it refers to), so there " + "is no telling what to keep. Nothing was " + "written.\n"); ret = 1; goto bailout; } @@ -1517,15 +1568,13 @@ static int do_upgrade(const char *filename, bool force, bool wipe_settings) { ret = 1; goto bailout; } + // Written from the mapped bundle: a copy padded to the partition + // would be 13 MB more for an ultimate image, on a camera that may + // have 16 MB available all told mtdwrite[i].size = ceil_up(payload, psize ? psize : mtd.erasesize); - mtdwrite[i].data = malloc(mtdwrite[i].size); - if (mtdwrite[i].data == NULL) { - fprintf(stderr, "Allocation error\n"); - return 1; - } - memcpy(mtdwrite[i].data, read_ptr, payload); - memset(mtdwrite[i].data + payload, 0xff, - mtdwrite[i].size - payload); + mtdwrite[i].data = read_ptr; + mtdwrite[i].padded = true; + mtdwrite[i].payload = payload; read_ptr += payload; } @@ -1584,7 +1633,7 @@ static int do_upgrade(const char *filename, bool force, bool wipe_settings) { i++; } - munmap(jsond, len); + // The bundle stays mapped: the images are written from it if (root_part == -1 || kernel_part == -1) { fprintf(stderr, @@ -1596,7 +1645,7 @@ static int do_upgrade(const char *filename, bool force, bool wipe_settings) { ",-(rootfs_data)"); carry_t carry = {.erasesize = mtd.erasesize}; - char new_bootargs[1024], new_bootcmd[1024]; + char new_bootargs[1024], new_bootcmd[1024], new_bootcmdnor[1024] = ""; if (keep_front) { if ((ssize_t)goff >= mtd.totalsz) { fprintf(stderr, @@ -1610,13 +1659,11 @@ static int do_upgrade(const char *filename, bool force, bool wipe_settings) { // what lies there now is the middle of the old one, and jffs2 would // mount its stale nodes as files carry.region_len = mtd.totalsz - goff; - carry.region = malloc(carry.region_len); - if (!carry.region) { - fprintf(stderr, "Allocation error\n"); - ret = 1; - goto bailout; - } - memset(carry.region, 0xff, carry.region_len); + carry.region = &mtdwrite[i]; + strcpy(mtdwrite[i].name, "rootfs_data"); + mtdwrite[i].off_flashb = goff; + mtdwrite[i].size = carry.region_len; + mtdwrite[i].padded = true; // erased unless settings are carried int data_part = -1; for (int p = 0; p < MAX_MTDBLOCKS && data_part < 0; p++) if (!strcmp(mtd.part[p].name, "rootfs_data")) @@ -1645,10 +1692,6 @@ static int do_upgrade(const char *filename, bool force, bool wipe_settings) { ret = 5; goto bailout; } - strcpy(mtdwrite[i].name, "rootfs_data"); - mtdwrite[i].off_flashb = goff; - mtdwrite[i].size = carry.region_len; - mtdwrite[i].data = carry.region; i++; if (!upgrade_bootargs(uboot_env_get_param("bootargs"), root_part, @@ -1664,8 +1707,33 @@ static int do_upgrade(const char *filename, bool force, bool wipe_settings) { ret = 1; goto bailout; } + // OpenIPC's bootloader keeps a second copy of the kernel read, which + // its setnor8m/setnor16m and failsafe run; it follows the layout too + // A copy with no kernel read is left alone; one with a read that + // cannot be edited is refused like bootcmd, or failsafe would boot + // the old kernel's place + const char *nor = uboot_env_get_param("bootcmdnor"); + bool nor_edited = false; + if (nor && strstr(nor, "sf read")) { + nor_edited = upgrade_bootcmd(nor, mtdwrite[kernel_part].off_flashb, + mtdwrite[kernel_part].size, + new_bootcmdnor, + sizeof(new_bootcmdnor)); + if (!nor_edited) { + fprintf(stderr, "bootcmdnor reads the kernel in a way this " + "cannot edit. Nothing was written.\n"); + ret = 1; + goto bailout; + } + } + if (!nor_edited) + new_bootcmdnor[0] = '\0'; printf("bootargs will be: %s\nbootcmd will be: %s\n", new_bootargs, new_bootcmd); + if (uboot_env_get_param("mtdparts")) + printf("mtdparts will be: %s\n", mtdparts); + if (*new_bootcmdnor) + printf("bootcmdnor will be: %s\n", new_bootcmdnor); } // The bundle's variables, checked while nothing is stopped. bootargs and @@ -1681,7 +1749,9 @@ static int do_upgrade(const char *filename, bool force, bool wipe_settings) { } cJSON_ArrayForEach(jvar, jenv) { if (!cJSON_IsString(jvar) || !strcmp(jvar->string, "bootargs") || - !strcmp(jvar->string, "bootcmd")) { + !strcmp(jvar->string, "bootcmd") || + !strcmp(jvar->string, "bootcmdnor") || + !strcmp(jvar->string, "mtdparts")) { fprintf(stderr, "The bundle sets '%s', which upgrade %s. Nothing was " "written.\n", @@ -1714,6 +1784,12 @@ static int do_upgrade(const char *filename, bool force, bool wipe_settings) { } if (keep_front) { + // The variable a bootargs of mtdparts=${mtdparts} reads, and the + // copy setnor8m/setnor16m keep, go with the layout either way + if (uboot_env_get_param("mtdparts")) + set_env_param_ram("mtdparts", mtdparts); + if (*new_bootcmdnor) + set_env_param_ram("bootcmdnor", new_bootcmdnor); set_env_param_ram("bootcmd", new_bootcmd); set_env_param_rom("bootargs", new_bootargs, mtd.env_dev, mtd.env_offset, mtd.erasesize); diff --git a/src/upgrade_env.c b/src/upgrade_env.c index ee134bc..1e8791c 100644 --- a/src/upgrade_env.c +++ b/src/upgrade_env.c @@ -15,6 +15,29 @@ static const char *find_word(const char *s, const char *key) { static size_t word_len(const char *w) { return strcspn(w, " "); } +bool upgrade_mtd_id_value(const char *w, char *id, size_t idsz) { + if (!w) + return false; + size_t n = strcspn(w, ": "); + if (!n || w[n] != ':' || memchr(w, '$', n) || n >= idsz) + return false; + memcpy(id, w, n); + id[n] = '\0'; + return true; +} + +bool upgrade_bootargs_mtdparts_is_var(const char *bootargs) { + const char *w = bootargs ? find_word(bootargs, "mtdparts=") : NULL; + if (!w) + return false; + // Only the variable upgrade sets: a reference to any other would leave + // the kernel the old partitions while root= moved on + w += strlen("mtdparts="); + size_t n = word_len(w); + return (n == 11 && !strncmp(w, "${mtdparts}", 11)) || + (n == 9 && !strncmp(w, "$mtdparts", 9)); +} + bool upgrade_mtd_id(const char *bootargs, char *id, size_t idsz) { const char *w = bootargs ? find_word(bootargs, "mtdparts=") : NULL; if (!w) @@ -31,8 +54,9 @@ bool upgrade_mtd_id(const char *bootargs, char *id, size_t idsz) { bool upgrade_bootargs(const char *old, int root_idx, const char *mtdparts, char *out, size_t outsz) { char id[64]; - if (!old || !upgrade_mtd_id(old, id, sizeof(id)) || - !find_word(old, "root=")) + bool var = upgrade_bootargs_mtdparts_is_var(old); + if (!old || !find_word(old, "root=") || + (!var && !upgrade_mtd_id(old, id, sizeof(id)))) return false; size_t used = 0; @@ -49,7 +73,7 @@ bool upgrade_bootargs(const char *old, int root_idx, const char *mtdparts, if (!strncmp(p, "root=", 5)) w = snprintf(out + used, outsz - used, "root=/dev/mtdblock%d", root_idx); - else if (!strncmp(p, "mtdparts=", 9)) + else if (!strncmp(p, "mtdparts=", 9) && !var) w = snprintf(out + used, outsz - used, "mtdparts=%s", mtdparts); else w = snprintf(out + used, outsz - used, "%.*s", (int)n, p); @@ -70,13 +94,14 @@ bool upgrade_bootcmd(const char *old, uint32_t off, uint32_t size, char *out, if (!rd || strstr(rd + 1, "sf read ")) return false; + // The load address stays, a number or a variable const char *p = rd + strlen("sf read "); - char *end; - strtoul(p, &end, 16); // the load address stays - if (end == p || *end != ' ') + size_t alen = strcspn(p, " ;"); + if (!alen || p[alen] != ' ') return false; - const char *addr_end = end; - p = end + 1; + const char *addr_end = p + alen; + char *end; + p = addr_end + 1; strtoul(p, &end, 16); if (end == p || *end != ' ') return false; diff --git a/src/upgrade_env.h b/src/upgrade_env.h index 7a83895..a6938d7 100644 --- a/src/upgrade_env.h +++ b/src/upgrade_env.h @@ -18,17 +18,31 @@ * mtdparts= (one that names a variable, "${mtdparts}", is not literal). */ bool upgrade_mtd_id(const char *bootargs, char *id, size_t idsz); -/* bootargs with root= pointing at /dev/mtdblock and mtdparts= - * replaced by `mtdparts` (id included), every other word kept in its place. - * False, leaving `out` unspecified, when either word is missing or the - * mtdparts= value is not literal: an edit that cannot find what it edits - * must not produce a line that boots the old layout. */ +/* The same from the value of OpenIPC's own `mtdparts` variable, + * "hi_sfc:256k(boot),...", which its bootloader's default bootargs refer to + * as mtdparts=${mtdparts}. */ +bool upgrade_mtd_id_value(const char *mtdparts, char *id, size_t idsz); + +/* Whether bootargs takes its partitions from the `mtdparts` variable + * (mtdparts=${mtdparts} or mtdparts=$mtdparts), in which case that variable + * is what has to change. A reference to any other variable is not this: it + * is neither literal nor the variable upgrade sets, and is refused. */ +bool upgrade_bootargs_mtdparts_is_var(const char *bootargs); + +/* bootargs with root= pointing at /dev/mtdblock and a literal + * mtdparts= replaced by `mtdparts` (id included), every other word kept in + * its place. A mtdparts= that names a variable is left as it is: the + * variable is set instead. False, leaving `out` unspecified, when either + * word is missing: an edit that cannot find what it edits must not produce + * a line that boots the old layout. */ bool upgrade_bootargs(const char *old, int root_idx, const char *mtdparts, char *out, size_t outsz); /* bootcmd with the kernel read, "sf read ", reading - * `size` bytes from `off` instead. False when there is no such read, or more - * than one. */ + * `size` bytes from `off` instead. The address may be a variable, as + * OpenIPC's bootloader writes it ("sf read ${baseaddr} 0x50000 0x200000"); + * the offset and size are numbers. False when there is no such read, or + * more than one. */ bool upgrade_bootcmd(const char *old, uint32_t off, uint32_t size, char *out, size_t outsz); diff --git a/src/upgrade_env_test.c b/src/upgrade_env_test.c index 271c9a9..6df7324 100644 --- a/src/upgrade_env_test.c +++ b/src/upgrade_env_test.c @@ -64,9 +64,10 @@ int main(void) { CHECK(!upgrade_bootargs("console=ttyAMA0 mtdparts=sfc:1k(a)", 3, "sfc:2k(a)", out, sizeof(out)), "a line without root= edited"); - CHECK(!upgrade_bootargs("root=/dev/mtdblock3 mtdparts=${mtdparts}", 3, - "sfc:2k(a)", out, sizeof(out)), - "a variable mtdparts edited"); + CHECK(upgrade_bootargs("root=/dev/mtdblock3 mtdparts=${mtdparts}", 4, + "sfc:2k(a)", out, sizeof(out)) && + !strcmp(out, "root=/dev/mtdblock4 mtdparts=${mtdparts}"), + "a variable mtdparts was not left for its variable: '%s'", out); CHECK(!upgrade_bootargs(XM_BOOTARGS, 3, "hi_sfc:1k(a)", out, 32), "a line that does not fit returned as done"); CHECK(!upgrade_bootargs(NULL, 3, "hi_sfc:1k(a)", out, sizeof(out)), @@ -89,6 +90,46 @@ int main(void) { out, sizeof(out)), "a read without a size edited"); + // OpenIPC's own bootloader, as a camera flashed with it keeps its + // environment: the load address is a variable, and the partitions are + // also kept in a variable of their own. + const char *OI_BOOTCMD = + "setenv setargs setenv bootargs ${bootargs}; run setargs; sf probe 0; " + "sf read ${baseaddr} 0x50000 0x200000; bootm ${baseaddr}"; + CHECK(upgrade_bootcmd(OI_BOOTCMD, 0x50000, 0x300000, out, sizeof(out)) && + !strcmp(out, "setenv setargs setenv bootargs ${bootargs}; run " + "setargs; sf probe 0; sf read ${baseaddr} 0x50000 " + "0x300000; bootm ${baseaddr}"), + "variable load address: '%s'", out); + CHECK(!upgrade_bootcmd("sf read ${baseaddr}; bootm", 0x50000, 0x300000, + out, sizeof(out)), + "a read with no offset edited"); + + CHECK(upgrade_mtd_id_value("hi_sfc:256k(boot),64k(env)", id, sizeof(id)) && + !strcmp(id, "hi_sfc"), + "mtd id from the variable '%s'", id); + // Its default bootargs name the variable: root= moves, mtdparts= stays + // and the variable is what changes + const char *OI_DEFAULT = "mem=${osmem} console=ttyAMA0,115200 panic=20 " + "root=/dev/mtdblock3 rootfstype=squashfs " + "init=/init mtdparts=${mtdparts} ${extras}"; + CHECK(upgrade_bootargs_mtdparts_is_var(OI_DEFAULT), "variable not seen"); + CHECK(!upgrade_bootargs_mtdparts_is_var(XM_BOOTARGS), + "a literal taken for a variable"); + CHECK(upgrade_bootargs(OI_DEFAULT, 3, "sfc:9k(x)", out, sizeof(out)) && + !strcmp(out, OI_DEFAULT), + "a variable mtdparts= was replaced: '%s'", out); + CHECK(upgrade_bootargs_mtdparts_is_var("root=/dev/mtdblock3 " + "mtdparts=$mtdparts"), + "$mtdparts not seen"); + // Another variable is neither literal nor the one upgrade sets: refused, + // or root= would move while the kernel kept the old partitions + const char *OTHER = "root=/dev/mtdblock3 mtdparts=${mtdpartsnor} x"; + CHECK(!upgrade_bootargs_mtdparts_is_var(OTHER), + "${mtdpartsnor} taken for ${mtdparts}"); + CHECK(!upgrade_bootargs(OTHER, 3, "sfc:9k(x)", out, sizeof(out)), + "bootargs naming another variable was edited"); + if (!failed) puts("upgrade_env_test: OK"); return failed; diff --git a/tools/upgrade_bundle.py b/tools/upgrade_bundle.py index 853360e..b6f7745 100755 --- a/tools/upgrade_bundle.py +++ b/tools/upgrade_bundle.py @@ -159,7 +159,7 @@ def main(): key, sep, value = kv.partition('=') if not sep or not key: parser.error(f'--env {kv}: expected KEY=VALUE') - if key in ('bootargs', 'bootcmd'): + if key in ('bootargs', 'bootcmd', 'bootcmdnor', 'mtdparts'): parser.error(f'--env {key}: ipctool writes it for the new ' 'layout itself') env[key] = value