Merge ~mwhudson/casper:lp-1977644 into casper:main

[Michael Hudson-Doyle]

casper-helper: preserve entries 2-4 in the MBR when creating a new partition. (LP: #1977644, #1922342)

Test ISO at https://people.canonical.com/~mwh/lp1977644.iso

[Steve Langasek (vorlon)]

review: Approve

[Steve Langasek (vorlon)]

(any chance of an integration test for this?)

[Michael Hudson-Doyle (mwhudson)]

Here's an attempt at a test. Haven't tried to run it yet though.

diff --git a/debian/changelog b/debian/changelog
index 0f55316..083da4d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+casper (1.473) UNRELEASED; urgency=medium
+
+  * casper-helper: preserve entries 2-4 in the MBR when creating a new
+    partition. (LP: #1977644, #1922342)
+
+ -- Michael Hudson-Doyle <[e-mail]>  Fri, 08 Jul 2022 12:37:44 +1200
+
 casper (1.472) kinetic; urgency=medium
 
   * Support iso-url= on the kernel command line as a synonym for url=, to
diff --git a/debian/tests/boot b/debian/tests/boot
index 5abc27e..938fdf8 100755
--- a/debian/tests/boot
+++ b/debian/tests/boot
@@ -395,9 +395,33 @@ touch /hello
     check_files_exist mnt upper/hello
 }
 
+test_mbr_preservation () {
+    echo "### creating the partition to save logs preserves other mbr entries"
+
+    ./debian/tests/prep-image image.img ""
+
+    expand_image image.img
+
+    # Forcefully create some dubious entries in the hybrid MBR which
+    # casper should ensure are preserved when creating the partition
+    # for the logs (see LP: #1977644)
+    echo -en '\0100\00\01\00\00\00\01\00\00\00\00\00\01\00\00\00\0200\00\01\00\00\00\01\00\00\00\00\00\01\00\00\00' | dd of=image.img bs=1 seek=462 conv=notrunc count=32
+
+    sfdisk --label-nested dos -d image.img | tee mbr-before
+
+    run image.img ""
+
+    sfdisk --label-nested dos -d image.img | tee mbr-after
+
+    if ! diff -u mbr-before mbr-after; then
+        echo "casper changed entries 2-4 in mbr ^^"
+        exit 1
+    fi
+}
+
 TESTS="test_basic test_auto_log_persistence test_clean_log_directory_each_boot
        test_explicit_persistence test_implicit_then_explicit_persistence
-       test_no_persistence test_todisk test_uses_casper_rw_partition"
+       test_mbr_preservation test_no_persistence test_todisk test_uses_casper_rw_partition"
 
 for f in $TESTS; do
     eval $f
diff --git a/scripts/casper-helpers b/scripts/casper-helpers
index 72a0cfe..0c9ee5b 100644
--- a/scripts/casper-helpers
+++ b/scripts/casper-helpers
@@ -301,22 +301,16 @@ find_or_create_persistent_partition () {
     if [ $((size - start*sectorsize)) -lt $((100*1024*1024)) ]; then
         return
     fi
+    MBR_STASH=/tmp/mbr_stash_$$
+    dd if="$DEVICE" bs=1 skip=462 count=48 of="$MBR_STASH"
     echo "start=$start" | sfdisk --no-reread -q $DEVICE -a || return
 
-    # If our partition table is GPT, we have a protective MBR; ensure that
-    # after adding a new partition, if there is a protective MBR, we
-    # recreate a bootable partition, the same as xorriso does, for
-    # compatibility. (GRUB doesn't actually care which partition is marked
-    # bootable, but some BIOSes care that it exists).
+    # If our partition table is GPT, we have a protective MBR; it is important
+    # for some BIOSes that we preserve entries 2-4 in this MBR.  See
+    # https://bugs.launchpad.net/ubuntu/+source/casper/+bug/1977644 and the
+    # very long https://bugs.launchpad.net/ubuntu/+source/casper/+bug/1922342.
     if sfdisk -d $DEVICE | grep -q 'label: gpt'; then
-        # detection of dash vs busybox/bash
-        if [ "$(echo -e foo)" = "-e foo" ]; then
-            escape_arg=""
-        else
-            escape_arg=-e
-        fi
-        echo $escape_arg -n '\0200\00\01\00\00\00\01\00\00\00\00\00\01\00\00\00' \
-        | dd of=$DEVICE bs=1 seek=462 conv=notrunc count=16
+        dd if="$MBR_STASH" bs=1 seek=462 conv=notrunc count=48 of="$DEVICE"
     fi
     for d in ${DEVICE}$newpartno ${DEVICE}p$newpartno ${DEVICE}-part$newpartno; do
         if [ -e $d ]; then