From c0006458dddbf333c0b7d21dd8f1c5ab3474569e Mon Sep 17 00:00:00 2001 From: Bodong Yang <86948717+Bodong-Yang@users.noreply.github.com> Date: Mon, 6 Nov 2023 11:11:14 +0900 Subject: [PATCH] [Fix] grub: workaround for OTA failure/otaclient crashing if rootfs is not sda (#258) This PR applies a workaround to deal with otaclient crashing and OTA failed when rootfs device is not assigned name as sda. With this PR, the slot naming is decoupled with the actual device name, the slot name will be fixed to sda, and otaclient will search for ota-partition folder with the slot name. With workaround implemented this PR, OTA will work if no matter when the rootfs device's name changed and what name is assigned to rootfs device. IMPORTANT: this is only a workaround fix, intended for swift issue overcoming and backward compatibility, a fundamental fix with new mechanism proposed is still required! Behavior before fix: 1. otaclient will crash during OTA finalizing switching boot if ota-partition folders other than ota-partition.sda* presented, 2. OTA will fail if rootfs device's name changed before and after OTA reboot, 3. OTA status will be lost if rootfs device's name changed, corresponding extra ota-partition folders will be created unexpectedly(i.e., ota-partition.sdb*, ota-partition.nvme0n1p*, etc). Behaivor after fix: 1. slot naming is fixed to "sda", 2. OTA will work and otaclient will not crash even if rootfs device's name changed at any time, otaclient will always locate the correct ota-partition folder. --- otaclient/app/boot_control/_grub.py | 52 +++++++++++++++++++++++++---- tests/keys/interm.pem | 18 +++++----- tests/keys/root.pem | 18 +++++----- tests/keys/sign.key | 6 ++-- tests/keys/sign.pem | 14 ++++---- 5 files changed, 74 insertions(+), 34 deletions(-) diff --git a/otaclient/app/boot_control/_grub.py b/otaclient/app/boot_control/_grub.py index a65281298..421901897 100644 --- a/otaclient/app/boot_control/_grub.py +++ b/otaclient/app/boot_control/_grub.py @@ -11,6 +11,24 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +"""Implementation of grub boot control. + +DEPRECATION WARNING(20231026): + Current mechanism of defining and detecting slots is proved to be not robust. + The design expects that rootfs device will always be sda, which might not be guaranteed + as the sdx naming scheme is based on the order of kernel recognizing block devices. + If rootfs somehow is not named as sda, the grub boot controller will fail to identifiy + the slots and/or finding corresponding ota-partition files, finally failing the OTA. + Also slots are detected by assuming the partition layout, which is less robust comparing to + cboot and rpi_boot implementation of boot controller. + +TODO(20231026): design new mechanism to define and manage slot. + +NOTE(20231027) A workaround fix is applied to handle the edge case of rootfs not named as sda, + Check GrubABPartitionDetector class for more details. + This workaround only means to avoid OTA failed on edge condition and maintain backward compatibility, + still expecting new mechanism to fundamentally resolve this issue. +""" import re @@ -282,11 +300,21 @@ class GrubABPartitionDetector: - sdx3: A partition - sdx4: B partition - slot_name is the dev name of the A/B partition. We assume that last 2 partitions are A/B partitions, error will be raised if the current rootfs is not one of the last 2 partitions. + + NOTE(20231027): as workaround to rootfs not sda breaking OTA, slot naming schema + is fixed to "sda", and ota-partition folder is searched with this name. + For example, if current slot's device is nvme0n1p3, the slot_name is sda3. """ + # assuming that the suffix digit are the partiton id, for example, + # sda3's pid is 3, nvme0n1p3's pid is also 3. + DEV_PATH_PA: ClassVar[re.Pattern] = re.compile( + r"^/dev/(?P\w*[a-z])(?P\d+)$" + ) + SLOT_NAME_PREFIX: ClassVar[str] = "sda" + def __init__(self) -> None: self.active_slot, self.active_dev = self._detect_active_slot() self.standby_slot, self.standby_dev = self._detect_standby_slot(self.active_dev) @@ -321,23 +349,33 @@ def _get_sibling_dev(self, active_dev: str) -> str: ) def _detect_active_slot(self) -> Tuple[str, str]: - """ + """Get active slot's slot_id. + Returns: A tuple contains the slot_name and the full dev path of the active slot. """ dev_path = CMDHelperFuncs.get_current_rootfs_dev() - slot_name = dev_path.lstrip("/dev/") + _dev_path_ma = self.DEV_PATH_PA.match(dev_path) + assert _dev_path_ma, f"dev path is invalid for OTA: {dev_path}" + + _pid = _dev_path_ma.group("partition_id") + slot_name = f"{self.SLOT_NAME_PREFIX}{_pid}" return slot_name, dev_path def _detect_standby_slot(self, active_dev: str) -> Tuple[str, str]: - """ + """Get standby slot's slot_id. + Returns: A tuple contains the slot_name and the full dev path of the standby slot. """ dev_path = self._get_sibling_dev(active_dev) - slot_name = dev_path.lstrip("/dev/") + _dev_path_ma = self.DEV_PATH_PA.match(dev_path) + assert _dev_path_ma, f"dev path is invalid for OTA: {dev_path}" + + _pid = _dev_path_ma.group("partition_id") + slot_name = f"{self.SLOT_NAME_PREFIX}{_pid}" return slot_name, dev_path @@ -350,7 +388,9 @@ def __init__(self) -> None: self.standby_root_dev = ab_detector.standby_dev self.active_slot = ab_detector.active_slot self.standby_slot = ab_detector.standby_slot - logger.info(f"{self.active_slot=}, {self.standby_slot=}") + logger.info( + f"{self.active_slot=}@{self.active_root_dev}, {self.standby_slot=}@{self.standby_root_dev}" + ) self.boot_dir = Path(cfg.BOOT_DIR) self.grub_file = Path(cfg.GRUB_CFG_PATH) diff --git a/tests/keys/interm.pem b/tests/keys/interm.pem index 054da0795..1dfc0df46 100644 --- a/tests/keys/interm.pem +++ b/tests/keys/interm.pem @@ -1,13 +1,13 @@ -----BEGIN CERTIFICATE----- -MIIB6zCCAZCgAwIBAgIUWO/ANSZ6jsn+aIK9RQDgniCR92gwCgYIKoZIzj0EAwIw +MIIB6jCCAZCgAwIBAgIUclYCFrRLcGCulcyR1uc8oA7t8vYwCgYIKoZIzj0EAwIw RTELMAkGA1UEBhMCSlAxDjAMBgNVBAgMBVRva3lvMQ4wDAYDVQQKDAVUaWVyNDEW -MBQGA1UEAwwNcm9vdC50aWVyNC5qcDAeFw0yMjEwMDMwNDE5MDNaFw0zMjEwMDUw -NDE5MDNaME0xCzAJBgNVBAYTAkpQMQ4wDAYDVQQIDAVUb2t5bzEOMAwGA1UECgwF +MBQGA1UEAwwNcm9vdC50aWVyNC5qcDAeFw0yMzEwMjcwNjU2MjdaFw0zMzEwMjkw +NjU2MjdaME0xCzAJBgNVBAYTAkpQMQ4wDAYDVQQIDAVUb2t5bzEOMAwGA1UECgwF VGllcjQxHjAcBgNVBAMMFWludGVybWVkaWF0ZS50aWVyNC5qcDBZMBMGByqGSM49 -AgEGCCqGSM49AwEHA0IABBKX8h546xKxJR8qArXmyBngW/T9+XbxowBssCg0yjZ6 -/tsRNPF5RlCDzI4C8DactlQP0t27NvxZQQhXFXpr4kujVjBUMB0GA1UdDgQWBBSL -03kBOi7DPLY0IQ+Q5ci4ObMfJzAfBgNVHSMEGDAWgBTUIPiXLIF4b/36Hgu4S/KU -sBjhGDASBgNVHRMBAf8ECDAGAQH/AgEAMAoGCCqGSM49BAMCA0kAMEYCIQCWNbaK -dkGer4rjV1rdkHv+jN5iYqL0H4ZeFKZ7wbgdKQIhAMZb8AC+Va1o1L0U9294jdI0 -aooIZ7UavjKy6t0mXB99 +AgEGCCqGSM49AwEHA0IABIu6gNzIRcDFoykqMWBRCxQfBx2BGK5lfFbJAfgYvxNN +OViBFZDOPxTHhFmuFX6txaT4pqfpWhQXpP2h+VzpCuGjVjBUMB0GA1UdDgQWBBRi +ik8kqC+T+dNnU1eML+lLEKAOGzAfBgNVHSMEGDAWgBRSteexpfLelFY6oPQL1dXH +Ky5+kTASBgNVHRMBAf8ECDAGAQH/AgEAMAoGCCqGSM49BAMCA0gAMEUCIHb9ZcDN +z3RUhVXaJd8WvkUlYHPNP5LP+0IFaJ7IzumTAiEAo1nLsbCF8WkLf4VXtJNk9Hu3 +1MbvWqS/Hhk5+tV7+AI= -----END CERTIFICATE----- diff --git a/tests/keys/root.pem b/tests/keys/root.pem index 1d1b72300..a5f3702e6 100644 --- a/tests/keys/root.pem +++ b/tests/keys/root.pem @@ -1,13 +1,13 @@ -----BEGIN CERTIFICATE----- -MIIB3zCCAYWgAwIBAgIUVcCyMLO2J6JKxw8Ye4cn21ty8XUwCgYIKoZIzj0EAwIw +MIIB3jCCAYWgAwIBAgIUGYEMZsuwqHrRhrLot++BqE+xbQ4wCgYIKoZIzj0EAwIw RTELMAkGA1UEBhMCSlAxDjAMBgNVBAgMBVRva3lvMQ4wDAYDVQQKDAVUaWVyNDEW -MBQGA1UEAwwNcm9vdC50aWVyNC5qcDAeFw0yMjEwMDMwNDE5MDNaFw0zMjEwMDUw -NDE5MDNaMEUxCzAJBgNVBAYTAkpQMQ4wDAYDVQQIDAVUb2t5bzEOMAwGA1UECgwF +MBQGA1UEAwwNcm9vdC50aWVyNC5qcDAeFw0yMzEwMjcwNjU2MjdaFw0zMzEwMjkw +NjU2MjdaMEUxCzAJBgNVBAYTAkpQMQ4wDAYDVQQIDAVUb2t5bzEOMAwGA1UECgwF VGllcjQxFjAUBgNVBAMMDXJvb3QudGllcjQuanAwWTATBgcqhkjOPQIBBggqhkjO -PQMBBwNCAARH1pYCjqPrXGGVIsMWIXvRIZfNCSH07X1iPn3lyNq7kIN4bJFiHdMi -LxxOh94uvhh7psYcrDizFi/l3SsLihbxo1MwUTAdBgNVHQ4EFgQU1CD4lyyBeG/9 -+h4LuEvylLAY4RgwHwYDVR0jBBgwFoAU1CD4lyyBeG/9+h4LuEvylLAY4RgwDwYD -VR0TAQH/BAUwAwEB/zAKBggqhkjOPQQDAgNIADBFAiEAhJLAk9SRQ2Q61m9e2VoM -m86odWMLRv+EKbpWebe4WTwCICnch1agvDomA7PMbkPvg0kz/jot2YoN3xZ24O6G -zjmb +PQMBBwNCAAR6Fa70+SJgvwq0adME6CAqq4VwNeJhAUxUdgz9QS+DpI7LcdMS6cmq +3EqA2vJ5Z9jpVAOUuJ2kXqrtalhN69bCo1MwUTAdBgNVHQ4EFgQUUrXnsaXy3pRW +OqD0C9XVxysufpEwHwYDVR0jBBgwFoAUUrXnsaXy3pRWOqD0C9XVxysufpEwDwYD +VR0TAQH/BAUwAwEB/zAKBggqhkjOPQQDAgNHADBEAiACPOdQ/zjz+tokGgb6APQp +ptM9N5JXgHTYTFBBpT72cAIgIWCnbPU68z6YiI4A1JqMzO4mUfnSP+5ssWTHLC2O +Bfg= -----END CERTIFICATE----- diff --git a/tests/keys/sign.key b/tests/keys/sign.key index 7ee6428d8..7eedbdf90 100644 --- a/tests/keys/sign.key +++ b/tests/keys/sign.key @@ -2,7 +2,7 @@ BggqhkjOPQMBBw== -----END EC PARAMETERS----- -----BEGIN EC PRIVATE KEY----- -MHcCAQEEIFBCLuRbpajW14w5MMqAjprChcTHz6+s0NxLEZaL6uTXoAoGCCqGSM49 -AwEHoUQDQgAEGxHFx+A641vv8cZ4M/YnrstGo0JfyTdGyaXX7Q5T9z0yRF0B35US -sqYSlja5cgt2bmJxk8EJrHxfUBrSQLi3PQ== +MHcCAQEEIDdgqKIQ3e30Dp0ZHz1ZWxkR6OAACEVCru/Klrepmj9xoAoGCCqGSM49 +AwEHoUQDQgAET8KH6J7rVkECpv9WfGGyyv4hj1ck8vBPMpLZSneePFzMFlxi0pYA +u2qsM/vssHsWi3j3Gii0l/CtyQSK1OK75g== -----END EC PRIVATE KEY----- diff --git a/tests/keys/sign.pem b/tests/keys/sign.pem index 988725130..ca325fcab 100644 --- a/tests/keys/sign.pem +++ b/tests/keys/sign.pem @@ -1,11 +1,11 @@ -----BEGIN CERTIFICATE----- -MIIBjjCCATMCFAgcTp8kYozCqL4Aib6cooc5o2tnMAoGCCqGSM49BAMCME0xCzAJ +MIIBjTCCATMCFG4r2lF/oNv7Ew2T7XDbfuQEuOW3MAoGCCqGSM49BAMCME0xCzAJ BgNVBAYTAkpQMQ4wDAYDVQQIDAVUb2t5bzEOMAwGA1UECgwFVGllcjQxHjAcBgNV -BAMMFWludGVybWVkaWF0ZS50aWVyNC5qcDAeFw0yMjEwMDMwNDE5MDNaFw0yMzEw -MDMwNDE5MDNaMEUxCzAJBgNVBAYTAkpQMQ4wDAYDVQQIDAVUb2t5bzEOMAwGA1UE +BAMMFWludGVybWVkaWF0ZS50aWVyNC5qcDAeFw0yMzEwMjcwNjU2MjdaFw0yNDEw +MjYwNjU2MjdaMEUxCzAJBgNVBAYTAkpQMQ4wDAYDVQQIDAVUb2t5bzEOMAwGA1UE CgwFVGllcjQxFjAUBgNVBAMMDXNpZ24udGllcjQuanAwWTATBgcqhkjOPQIBBggq -hkjOPQMBBwNCAAQbEcXH4DrjW+/xxngz9ieuy0ajQl/JN0bJpdftDlP3PTJEXQHf -lRKyphKWNrlyC3ZuYnGTwQmsfF9QGtJAuLc9MAoGCCqGSM49BAMCA0kAMEYCIQCx -Pn2DN1k1M2X1+96qTkvwCibD3uDB1pPMdM0oBRWFswIhAIwU9tRlTQj1M3TShBuP -01qYR4cuhMyec7VLI0VPyfBa +hkjOPQMBBwNCAARPwofonutWQQKm/1Z8YbLK/iGPVyTy8E8yktlKd548XMwWXGLS +lgC7aqwz++ywexaLePcaKLSX8K3JBIrU4rvmMAoGCCqGSM49BAMCA0gAMEUCIQCL +U0RAelUktN4KWBk/S3ZL7pV6BHIz5pir8QN7pzGuwQIgdrkTFIGSeNoLIi1naZbU +16HdWbQJOVtrmjHx1tZBP7o= -----END CERTIFICATE-----