upgrade: move cameras on OpenIPC's own bootloader, in the memory they have

README.md

@@ -308,14 +308,21 @@ 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`
   starts it from defaults instead;
 * 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

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);

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;

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<root_idx> 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<root_idx> 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 <addr> <off> <size>", 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);