Align LZMA and padding behavior with Simplicity Commander

.github/workflows/ci.yml

@@ -130,6 +130,22 @@ jobs:
       - name: Set up uv
         run: |
           curl -LsSf https://astral.sh/uv/install.sh | sh
+      - name: Cache Simplicity Commander
+        uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
+        with:
+          path: ~/commander
+          key: 1-${{ runner.os }}-commander-1.24.1
+      - name: Install Simplicity Commander
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y --no-install-recommends libgl1 libglib2.0-0t64
+          if [ ! -x "$HOME/commander/commander" ]; then
+            curl -sSfL -o /tmp/commander.tar.bz \
+              https://updates.silabs.com/studio/v6/updates/update_site/archives/commander/1.24.1/Commander_linux_x86_64_1v24p1b1980.tar.bz
+            echo "3ba24eeaeb560e9db306a4d070e2bbe40b456701b4b87c53643a93ab1101b2c4  /tmp/commander.tar.bz" | sha256sum -c
+            tar -xjf /tmp/commander.tar.bz -C "$HOME"
+          fi
+          echo "$HOME/commander" >> "$GITHUB_PATH"
       - name: Register Python problem matcher
         run: |
           echo "::add-matcher::.github/workflows/matchers/python.json"

README.md

@@ -3,9 +3,8 @@
 A parser for the EBL and GBL (v3 and v4) firmware image formats used by the
 [Silicon Labs Gecko bootloader](https://www.silabs.com/documents/public/user-guides/ug489-gecko-bootloader-user-guide-gsdk-4.pdf).
 
-Images are parsed into structured tags, and round-trip byte-for-byte: anything the
-library reads it can write back unchanged, including the trailing data that some vendors
-append after the end tag.
+Images are parsed into structured tags and round-trip byte-for-byte, with anything
+following the end tag handed back separately rather than folded into the image.
 
 ## Installation
 
@@ -41,8 +40,15 @@ for tag in image.tags:
 
 print(image.get_first_tag(GBL3ApplicationInfo).version)
 print(image.get_metadata())  # opaque bytes, the schema is vendor defined
+```
+
+`serialize` pads up to a word boundary, as `commander` does. `parse_firmware_image`
+discards anything after the end tag; `deserialize` hands it back:
+
+```python
+image, trailing = GBL3Image.deserialize(data)
 
-assert image.serialize() == data
+assert image.serialize(block_size=1) + trailing == data
 ```
 
 Modify an image. Tags are frozen dataclasses, so `dataclasses.replace` works, and
@@ -73,9 +79,12 @@ private_key = load_pem_private_key(
 key = bytes.fromhex("7F8FE53979B31BC556FCB131AFF42414")
 
 sealed = image.compress(GBL3Compression.LZMA).encrypt(key).sign(private_key)
-pathlib.Path("signed.gbl").write_bytes(sealed.serialize(block_size=4))
+pathlib.Path("signed.gbl").write_bytes(sealed.serialize())
 ```
 
+The bootloader decompresses into fixed buffers, so `compress` uses the only LZMA
+parameters it can accept. Overriding them past what it can allocate raises `ValueError`.
+
 And unwrap it again:
 
 ```python
@@ -100,21 +109,18 @@ with open("bootloader.out", "rb") as f:
     bootloader = build_bootloader_gbl3(f)
 ```
 
-Both reproduce `commander gbl create` byte-for-byte. Bootloader payloads carry a CRC32
-of themselves, which `build_bootloader_gbl3` appends for you.
-
 ## GBLv4
 
 Series 3 parts use GBLv4, a different format that nests tags inside a signed manifest
-and can bundle several updates in one file. `parse_firmware_image` recognizes it by its
-magic and returns a `GBL4Image`, which round-trips byte-for-byte like the rest:
+and can bundle several updates in one file.
 
 ```python
 from pygbl import GBL4Image, GBL4MemorySectionInfo, GBL4UpdateMemorySection
 
-image = parse_firmware_image(pathlib.Path("light-simg301.gbl4").read_bytes())
-assert isinstance(image, GBL4Image)
-assert image.serialize() == data
+data = pathlib.Path("light-simg301.gbl4").read_bytes()
+image, trailing = GBL4Image.deserialize(data)
+
+assert image.serialize() + trailing == data
 
 # `get_tags` searches the whole tree, at any depth
 for update in image.get_tags(GBL4UpdateMemorySection):

pygbl/__init__.py

@@ -30,6 +30,7 @@
     read_loadable_segments,
 )
 from pygbl.gbl3 import (
+    GBL3_BLOCK_SIZE,
     GBL3_MAGIC,
     BootloaderVersion,
     GBL3ApplicationInfo,
@@ -122,6 +123,7 @@ def parse_firmware_image(data: bytes, *, validate: bool = True) -> FirmwareImage
 __all__ = [
     "EBL_BLOCK_SIZE",
     "EBL_MAGIC",
+    "GBL3_BLOCK_SIZE",
     "GBL3_MAGIC",
     "VALID_CRC32",
     "BootloaderVersion",

pygbl/compression.py

@@ -2,7 +2,7 @@
 
 import lzma
 
-from pygbl.types import MissingDependencyError
+from pygbl.types import MissingDependencyError, ValidationError
 
 try:
     import lz4.block
@@ -11,14 +11,14 @@
 else:
     HAVE_LZ4 = True
 
-# The Gecko bootloader's LZMA decoder is memory constrained. Its probability model needs
-# `4 KiB + 1.5 KiB * (1 << (lc + lp))` of RAM and its dictionary buffer is statically
-# sized, so images cannot use the LZMA defaults of lc=3, lp=0 and a large dictionary.
-# `commander` emits lc=1, lp=1, pb=2 with an 8 KiB dictionary.
 LZMA_LC = 1
 LZMA_LP = 1
 LZMA_PB = 2
 LZMA_DICT_SIZE = 8192
+LZMA_MAX_LC_PLUS_LP = 2
+LZMA_MAX_DICT_SIZE = 8192
+LZMA_NICE_LEN = 32
+LZMA_DEPTH = 32
 
 LZMA_PROPS_SIZE = 5
 LZMA_SIZE_FIELD_SIZE = 8
@@ -35,18 +35,52 @@ def lzma_filters(
     lp: int = LZMA_LP,
     pb: int = LZMA_PB,
     dict_size: int = LZMA_DICT_SIZE,
+    nice_len: int = LZMA_NICE_LEN,
+    depth: int = LZMA_DEPTH,
 ) -> list[dict[str, int]]:
+    # The device rejects an image it cannot allocate buffers for, so catch it here
+    # rather than at flash time.
+    if lc + lp > LZMA_MAX_LC_PLUS_LP:
+        raise ValueError(
+            f"lc + lp must be at most {LZMA_MAX_LC_PLUS_LP}, got {lc} + {lp}"
+        )
+
+    if dict_size > LZMA_MAX_DICT_SIZE:
+        raise ValueError(
+            f"dict_size must be at most {LZMA_MAX_DICT_SIZE}, got {dict_size}"
+        )
+
     return [
         {
             "id": lzma.FILTER_LZMA1,
             "lc": lc,
             "lp": lp,
             "pb": pb,
             "dict_size": dict_size,
+            "mode": lzma.MODE_NORMAL,
+            "mf": lzma.MF_BT4,
+            "nice_len": nice_len,
+            "depth": depth,
         }
     ]
 
 
+def ends_with_end_marker(container: bytes) -> bool:
+    """Check that an `.lzma` container's stream terminates with an end marker."""
+    # `eof` alone is not enough: the decompressor stops at the declared size and never
+    # looks for the marker. Declaring the size unknown again forces it to rely on one.
+    probe = (
+        container[:LZMA_PROPS_SIZE]
+        + b"\xff" * LZMA_SIZE_FIELD_SIZE
+        + container[LZMA_HEADER_SIZE:]
+    )
+
+    decompressor = lzma.LZMADecompressor(format=lzma.FORMAT_ALONE)
+    decompressor.decompress(probe)
+
+    return decompressor.eof
+
+
 def lzma_compress(data: bytes, **kwargs: int) -> bytes:
     """Compress into the `.lzma` container that the Gecko bootloader expects."""
     compressed = lzma.compress(
@@ -55,12 +89,19 @@ def lzma_compress(data: bytes, **kwargs: int) -> bytes:
 
     # Python writes an unknown-size marker into the 8 byte size field. The bootloader
     # skips the field entirely, but `commander` writes the real size, so we match it.
-    return (
+    container = (
         compressed[:LZMA_PROPS_SIZE]
         + len(data).to_bytes(LZMA_SIZE_FIELD_SIZE, "little")
         + compressed[LZMA_HEADER_SIZE:]
     )
 
+    # The bootloader requires `LZMA_STATUS_FINISHED_WITH_MARK` and fails the update
+    # otherwise, after having already flashed past the end of the payload.
+    if not ends_with_end_marker(container):
+        raise ValidationError("LZMA stream does not end with an end marker")
+
+    return container
+
 
 def lzma_decompress(data: bytes) -> bytes:
     return lzma.decompress(data, format=lzma.FORMAT_ALONE)

pygbl/ebl.py

@@ -219,14 +219,9 @@ def serialize_tag(tag: EBLTagBase) -> bytes:
 class EBLImage:
     tags: list[EBLTagBase]
 
-    # Bytes following the end tag: the padding that aligns the image to a 64 byte
-    # boundary, and for some vendors an entire second firmware payload appended after
-    # it. Outside of the CRC and preserved verbatim so that images round-trip
-    # byte-for-byte.
-    trailing_data: bytes = b""
-
     @classmethod
-    def from_bytes(cls, data: bytes, *, validate: bool = True) -> EBLImage:
+    def deserialize(cls, data: bytes, *, validate: bool = True) -> tuple[Self, bytes]:
+        """Parse an image, returning it and whatever followed its end tag."""
         data = bytes(data)
         tags: list[EBLTagBase] = []
         offset = 0
@@ -253,21 +248,26 @@ def from_bytes(cls, data: bytes, *, validate: bool = True) -> EBLImage:
             if tag_id == EBLTagId.END:
                 break
 
-        image = cls(tags=tags, trailing_data=data[offset:])
+        image = cls(tags=tags)
 
         if validate:
             image.validate()
 
-        return image
+        return image, data[offset:]
+
+    @classmethod
+    def from_bytes(cls, data: bytes, *, validate: bool = True) -> Self:
+        """Parse an image, discarding anything that followed its end tag."""
+        return cls.deserialize(data, validate=validate)[0]
 
     def serialize_tags(self) -> bytes:
         """Serialize only the tag region, which is what the CRC covers."""
         return b"".join(serialize_tag(t) for t in self.tags)
 
-    def serialize(self, *, block_size: int = 1, padding: bytes = b"\xff") -> bytes:
-        return pad_to_multiple(
-            self.serialize_tags() + self.trailing_data, block_size, padding
-        )
+    def serialize(
+        self, *, block_size: int = EBL_BLOCK_SIZE, padding: bytes = b"\xff"
+    ) -> bytes:
+        return pad_to_multiple(self.serialize_tags(), block_size, padding)
 
     def validate(self) -> None:
         if not self.tags:
@@ -308,9 +308,7 @@ def regenerate_crc(self) -> EBLImage:
         placeholder = type(self)(tags=[*tags, EBLEnd(crc=0)])
         crc = zlib.crc32(placeholder.serialize_tags()[:-4]) & 0xFFFFFFFF
 
-        return type(self)(
-            tags=[*tags, EBLEnd(crc=crc)], trailing_data=self.trailing_data
-        )
+        return type(self)(tags=[*tags, EBLEnd(crc=crc)])
 
 
 def _parse_tag(tag_id: int, payload: bytes) -> EBLTagBase:

pygbl/gbl3.py

@@ -29,6 +29,9 @@
 
 GBL3_MAGIC = b"\xeb\x17\xa6\x03"
 
+# `commander` pads its output up to a word boundary with 0xFF
+GBL3_BLOCK_SIZE = 4
+
 
 class GBL3TagId(enum.IntEnum):
     """GBL tag identifiers, as decoded from the little-endian 32-bit value on the wire."""
@@ -678,30 +681,36 @@ def parse_tag_stream(
 class GBL3Image:
     tags: list[GBL3TagBase]
 
-    # Bytes following the end tag. Real-world images pad with `0xFF` or `0x00`, and some
-    # vendors append an entire second firmware payload. It is outside of the CRC and is
-    # preserved verbatim so that images round-trip byte-for-byte.
-    trailing_data: bytes = b""
-
     @classmethod
-    def from_bytes(cls, data: bytes, *, validate: bool = True) -> GBL3Image:
+    def deserialize(cls, data: bytes, *, validate: bool = True) -> tuple[Self, bytes]:
+        """Parse an image, returning it and whatever followed its end tag.
+
+        Real-world files carry `commander`'s word padding, vendor junk or even a second
+        firmware payload after the end tag. None of it is covered by the CRC, so what to
+        do with it is the caller's decision.
+        """
         data = bytes(data)
         tags, offset = parse_tag_stream(data, stop_at_end=True)
-        image = cls(tags=tags, trailing_data=data[offset:])
+        image = cls(tags=tags)
 
         if validate:
             image.validate()
 
-        return image
+        return image, data[offset:]
+
+    @classmethod
+    def from_bytes(cls, data: bytes, *, validate: bool = True) -> Self:
+        """Parse an image, discarding anything that followed its end tag."""
+        return cls.deserialize(data, validate=validate)[0]
 
     def serialize_tags(self) -> bytes:
         """Serialize only the tag region, which is what the CRC covers."""
         return b"".join(serialize_tag(t) for t in self.tags)
 
-    def serialize(self, *, block_size: int = 1, padding: bytes = b"\xff") -> bytes:
-        return pad_to_multiple(
-            self.serialize_tags() + self.trailing_data, block_size, padding
-        )
+    def serialize(
+        self, *, block_size: int = GBL3_BLOCK_SIZE, padding: bytes = b"\xff"
+    ) -> bytes:
+        return pad_to_multiple(self.serialize_tags(), block_size, padding)
 
     def validate(self) -> None:
         if not self.tags:
@@ -748,9 +757,7 @@ def regenerate_crc(self) -> GBL3Image:
         placeholder = type(self)(tags=[*tags, GBL3End(crc=0)])
         crc = zlib.crc32(placeholder.serialize_tags()[:-4]) & 0xFFFFFFFF
 
-        return type(self)(
-            tags=[*tags, GBL3End(crc=crc)], trailing_data=self.trailing_data
-        )
+        return type(self)(tags=[*tags, GBL3End(crc=crc)])
 
     def signing_digest(self) -> bytes:
         """The SHA-256 digest an ECDSA signature covers.
@@ -783,9 +790,7 @@ def sign(self, private_key: ec.EllipticCurvePrivateKey) -> GBL3Image:
         unsigned = type(self)(tags=tags)
         r, s = sign_digest(private_key, unsigned.signing_digest())
 
-        return type(self)(
-            tags=[*tags, GBL3Signature(r=r, s=s)], trailing_data=self.trailing_data
-        ).regenerate_crc()
+        return type(self)(tags=[*tags, GBL3Signature(r=r, s=s)]).regenerate_crc()
 
     def verify_signature(self, public_key: ec.EllipticCurvePublicKey) -> bool:
         signature = self.find_first_tag(GBL3Signature)
@@ -836,7 +841,6 @@ def encrypt(self, key: bytes, *, nonce: bytes | None = None) -> GBL3Image:
                 GBL3EncryptionInitAesCcm(msg_len=len(ciphertext), nonce=nonce),
                 *encrypted,
             ],
-            trailing_data=self.trailing_data,
         ).regenerate_crc()
 
     def decrypt(self, key: bytes) -> GBL3Image:
@@ -863,7 +867,6 @@ def decrypt(self, key: bytes) -> GBL3Image:
                 ),
                 *tags,
             ],
-            trailing_data=self.trailing_data,
         ).regenerate_crc()
 
     def compress(self, algorithm: GBL3Compression) -> GBL3Image:
@@ -877,7 +880,7 @@ def compress(self, algorithm: GBL3Compression) -> GBL3Image:
             else:
                 tags.append(tag)
 
-        return type(self)(tags=tags, trailing_data=self.trailing_data).regenerate_crc()
+        return type(self)(tags=tags).regenerate_crc()
 
     def decompress(self) -> GBL3Image:
         """Return a copy with every compressed program data tag expanded."""
@@ -895,7 +898,7 @@ def decompress(self) -> GBL3Image:
             else:
                 tags.append(tag)
 
-        return type(self)(tags=tags, trailing_data=self.trailing_data).regenerate_crc()
+        return type(self)(tags=tags).regenerate_crc()
 
     def is_combined_bootloader_app(self) -> bool:
         app_info = self.find_first_tag(GBL3ApplicationInfo)