Bug#981346: partman-base: partman still overwrites u-boot on some systems

[harry88@gmx.ph]

Package: partman-base
Version: 214
Severity: important
Tags: d-i patch

Dear Debian Install System Team,

This is a follow-on from bug #770666 in which some systems had their firmware
overwritten by partman.  The patch there provided a function to detect the
affected systems by looking for their manufacturer names in /proc/cpuinfo,
so that partman would know not to overwrite in those cases.

Unfortunately, affected arm64 systems don't contain a suitable string in
/proc/cpuinfo, so their firmware still ends up getting overwritten.

My suggestion is to use /proc/device-tree/compatible instead of /proc/cpuinfo.
By searching the list there for an entry beginning with "allwinner," or
"fsl,imx6" or "ti,am33xx", we will hopefully match the same systems as the
current search for "Allwinner", "Freescale" or "AM33XX" would have found.

I've included a suggested patch below.

Thank you for considering it,
Harold.


diff --git a/parted_server.c b/parted_server.c
index 41784b70..4e7a0ead 100644
--- a/parted_server.c
+++ b/parted_server.c
@@ -1334,30 +1334,51 @@ command_dump()
         oprintf("OK\n");
 }

-/* Check whether we are running on a sunxi-based, freescale-based, or
-   AM33XX (beaglebone black) system. */
+/* Decide whether this system stores firmware on disk by looking up
+ * machine type in device tree. If Allwinner (bug #751704) or
+ * i.MX6 or AM33xx (bug #770666), assume yes, otherwise no.
+ */
-int
+bool
 is_system_with_firmware_on_disk()
 {
-        int cpuinfo_handle;
-        int result = 0;
+        int dt_handle;
+        int close_ret;
+        char *cur;
         char buf[4096];
         int length;

-        if ((cpuinfo_handle = open("/proc/cpuinfo", O_RDONLY)) != -1) {
-                length = read(cpuinfo_handle, buf, sizeof(buf)-1);
-                if (length > 0) {
-                        buf[length]='\0';
-                        if (strstr(buf, "Allwinner") != NULL)
-                                result = 1;
-                        else if (strstr(buf, "Freescale") != NULL)
-                                result = 1;
-                        else if (strstr(buf, "AM33XX") != NULL)
-                                result = 1;
-                }
-                close(cpuinfo_handle);
-        }
-        return result;
+        dt_handle = open("/proc/device-tree/compatible", O_RDONLY);
+        if (dt_handle == -1) {
+                log("Error opening device-tree property: %s", strerror(errno));
+                log("Assuming non-ARM system, with no firmware on disk");
+                return false;
+        }
+
+        length = read(dt_handle, buf, sizeof(buf)-1);
+        if (length == -1)
+                log("Error reading device-tree property: %s", strerror(errno));
+
+        close_ret = close(dt_handle);
+        if (close_ret == -1)
+                log("Error closing device-tree property: %s", strerror(errno));
+
+        if (length == -1 || close_ret == -1) {
+                log("Assuming no firmware on disk");
+                return false;
+        }
+
+        buf[length] = '\0';
+        for (cur = buf; cur < buf+length; cur += strlen(cur)+1) {
+                if (0 == strncmp(cur, "allwinner,", 10)
+                    || 0 == strncmp(cur, "fsl,imx6", 8)
+                    || 0 == strncmp(cur, "ti,am33xx", 9)) {
+                        log("Machine type '%s'; stores firmware on disk", cur);
+                        return true;
+                }
+        }
+
+        log("Machine type checked; assuming no firmware on disk");
+        return false;
 }

 void
@@ -1368,15 +1389,15 @@ command_commit()
                 critical_error("The device %s is not opened.", device_name);
         log("command_commit()");

-        /* The boot device on sunxi-based systems needs special handling.
+        /* The boot device on some systems needs special handling.
          * By default partman calls ped_disk_clobber when writing the
-         * partition table, but on sunxi-based systems this would overwrite
+         * partition table, but on some systems this would overwrite
          * the firmware area, resulting in an unbootable system (see
          * bug #751704).
          */
         if (is_system_with_firmware_on_disk() && !strncmp(disk->dev->path, "/dev/mmcblk", 11)) {
                 disk->needs_clobber = 0;
-                log("Sunxi/Freescale/AM33XX detected. Disabling ped_disk_clobber " \
+                log("Disabling ped_disk_clobber " \
                     "for the boot device %s to protect the firmware " \
                     "area.", disk->dev->path);
         }