diff --git a/docs/installation_usage.md b/docs/installation_usage.md index cc6b006..eafef96 100644 --- a/docs/installation_usage.md +++ b/docs/installation_usage.md @@ -158,8 +158,10 @@ void main(void) { # Install Python tool dependencies python3 -m pip install -r requirements.txt -# Pack firmware with eboot image header -python tools/imgpack.py firmware.bin --output firmware.packed.bin +# Pack firmware with eboot image header. The input limits (size, load and +# entry addresses) are listed in quickstart.md and in tools/imgpack.py. +python tools/imgpack.py --input firmware.bin --output firmware.packed.bin \ + --version 1.0.0 --load-addr 0x08010000 --entry-addr 0x08010100 # Sign the image python tools/sign_image.py firmware.packed.bin --key signing_key.pem diff --git a/docs/quickstart.md b/docs/quickstart.md index 638b448..19e1eaf 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -94,7 +94,13 @@ python3 sign_image.py --genkey --output keys/ cmake -B build -DEBLDR_BOARD= -DCMAKE_BUILD_TYPE=Release \ -DEBLDR_PRODUCTION_KEY=$(cat keys/public_key.hex) -# Pack a raw firmware binary into an .eimg container. +# Pack a raw firmware binary into an .eimg container. imgpack refuses what +# the bootloader refuses (core/image_verify.c): an input that is empty or +# over 16 MiB (16777216 bytes), checked before it is read; addresses or +# flags that do not fit 32 bits; and, when --load-addr is non-zero, a load +# address + size past 0xFFFFFFFF or an --entry-addr outside +# [load-addr, load-addr + size). --load-addr 0 means "unset" and skips the +# entry check, as the bootloader does. python3 imgpack.py --input firmware.bin --output firmware.eimg \ --load-addr 0x08010000 --entry-addr 0x08010100 --version 1.0.0 diff --git a/llms-full.txt b/llms-full.txt index cf6dc61..ddd4f9d 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -1695,8 +1695,10 @@ void main(void) { # Install Python tool dependencies python3 -m pip install -r requirements.txt -# Pack firmware with eboot image header -python tools/imgpack.py firmware.bin --output firmware.packed.bin +# Pack firmware with eboot image header. The input limits (size, load and +# entry addresses) are listed in quickstart.md and in tools/imgpack.py. +python tools/imgpack.py --input firmware.bin --output firmware.packed.bin \ + --version 1.0.0 --load-addr 0x08010000 --entry-addr 0x08010100 # Sign the image python tools/sign_image.py firmware.packed.bin --key signing_key.pem @@ -1864,7 +1866,13 @@ python3 sign_image.py --genkey --output keys/ cmake -B build -DEBLDR_BOARD= -DCMAKE_BUILD_TYPE=Release \ -DEBLDR_PRODUCTION_KEY=$(cat keys/public_key.hex) -# Pack a raw firmware binary into an .eimg container. +# Pack a raw firmware binary into an .eimg container. imgpack refuses what +# the bootloader refuses (core/image_verify.c): an input that is empty or +# over 16 MiB (16777216 bytes), checked before it is read; addresses or +# flags that do not fit 32 bits; and, when --load-addr is non-zero, a load +# address + size past 0xFFFFFFFF or an --entry-addr outside +# [load-addr, load-addr + size). --load-addr 0 means "unset" and skips the +# entry check, as the bootloader does. python3 imgpack.py --input firmware.bin --output firmware.eimg \ --load-addr 0x08010000 --entry-addr 0x08010100 --version 1.0.0 diff --git a/tests/unit/test_imgpack.py b/tests/unit/test_imgpack.py index 7cee682..2ad98e2 100644 --- a/tests/unit/test_imgpack.py +++ b/tests/unit/test_imgpack.py @@ -12,6 +12,8 @@ identically to '1.0.0' — numerically *older* than 1.0.1). """ +import os +import stat import struct import subprocess import sys @@ -22,16 +24,28 @@ REPO_ROOT = Path(__file__).resolve().parents[2] TOOLS = REPO_ROOT / "tools" +IMAGE_SIZE_OFFSET = 8 IMAGE_VERSION_OFFSET = 20 +MAX_IMAGE_SIZE = 16 * 1024 * 1024 # core/image_verify.c +posix_only = pytest.mark.skipif( + os.name != "posix", reason="needs POSIX symlinks and permission bits") -def _pack(tmp_path, version): - (tmp_path / "fw.bin").write_bytes(b"\x5A" * 256) + +def _pack( + tmp_path, + version, + *, + load_addr="0x08010000", + entry_addr="0x08010010", + firmware=b"\x5A" * 256, +): + (tmp_path / "fw.bin").write_bytes(firmware) return subprocess.run( [sys.executable, str(TOOLS / "imgpack.py"), "--input", str(tmp_path / "fw.bin"), "--output", str(tmp_path / "fw.eimg"), - "--load-addr", "0x08010000", "--entry-addr", "0x08010100", + "--load-addr", load_addr, "--entry-addr", entry_addr, # `--version=X`, not `--version X`: a value such as "-1.0.0" is a # separate token in the second form, and whether argparse reads it # as a value or as an unknown option depends on the interpreter -- @@ -42,6 +56,21 @@ def _pack(tmp_path, version): capture_output=True, text=True) +def _pack_paths(input_path, output_path): + return subprocess.run( + [sys.executable, str(TOOLS / "imgpack.py"), + "--input", str(input_path), "--output", str(output_path), + "--version=1.2.3", "--load-addr", "0x08010000", + "--entry-addr", "0x08010010"], + capture_output=True, text=True, + ) + + +def _leftover_temporaries(directory): + return sorted(p.name for p in directory.iterdir() + if p.name.startswith(".") and p.name.endswith(".tmp")) + + def test_in_range_version_encodes_correctly(tmp_path): r = _pack(tmp_path, "1.2.3") assert r.returncode == 0, r.stderr @@ -72,3 +101,135 @@ def test_out_of_range_version_is_rejected(tmp_path, version): assert not (tmp_path / "fw.eimg").exists(), ( f"output image was written for rejected version '{version}'") assert "out of range" in r.stderr + + +@pytest.mark.parametrize( + ("load_addr", "entry_addr", "reason"), + [ + ("0x08010000", "0x08010100", "outside the image range"), # exclusive image end + ("0x08010000", "0x0800FFFF", "outside the image range"), # before the image + ("0x08010000", "0x100000000", "entry address"), # entry does not fit uint32 + # Only the load address is wrong here, so the rejection can come from + # nothing else. + ("0x100000000", "0x08010010", "load address"), # load does not fit uint32 + ("0xFFFFFFF0", "0xFFFFFFF0", "overflows uint32"), # load + 256-byte image wraps + ], +) +def test_invalid_runtime_layout_is_rejected_before_output( + tmp_path, load_addr, entry_addr, reason +): + r = _pack( + tmp_path, "1.2.3", load_addr=load_addr, entry_addr=entry_addr + ) + assert r.returncode != 0 + assert not (tmp_path / "fw.eimg").exists() + assert "Error:" in r.stderr + assert reason in r.stderr + + +def test_empty_firmware_is_rejected(tmp_path): + r = _pack(tmp_path, "1.2.3", firmware=b"") + assert r.returncode != 0 + assert not (tmp_path / "fw.eimg").exists() + assert "image size" in r.stderr + + +def test_image_of_exactly_16_mib_is_accepted(tmp_path): + r = _pack(tmp_path, "1.2.3", firmware=b"\x5A" * MAX_IMAGE_SIZE) + assert r.returncode == 0, r.stderr + data = (tmp_path / "fw.eimg").read_bytes() + assert struct.unpack_from(" int: return zlib.crc32(data) & 0xFFFFFFFF +def parse_uint32(value: str, label: str) -> int: + """Parse a hexadecimal CLI value that fits the image's uint32 fields.""" + try: + parsed = int(value, 16) + except ValueError as exc: + raise ValueError(f"{label} must be a hexadecimal uint32, got '{value}'") from exc + if not 0 <= parsed <= UINT32_MAX: + raise ValueError(f"{label} is outside the uint32 range: '{value}'") + return parsed + + +def validate_image_size(image_size: int) -> None: + """Reject an image size the bootloader refuses (0, or over 16 MiB).""" + if image_size <= 0 or image_size > MAX_IMAGE_SIZE: + raise ValueError( + f"image size must be between 1 and {MAX_IMAGE_SIZE} bytes; got {image_size}" + ) + + +def validate_image_layout(image_size: int, load_addr: int, entry_addr: int) -> None: + """Reject images the bootloader will refuse before writing an artifact.""" + validate_image_size(image_size) + if not 0 <= load_addr <= UINT32_MAX or not 0 <= entry_addr <= UINT32_MAX: + raise ValueError("load and entry addresses must fit uint32") + if load_addr == 0: + # The firmware parser treats zero as an unset load address and skips + # the runtime entry-range check for compatibility with XIP targets. + return + if image_size > UINT32_MAX - load_addr: + raise ValueError("load address plus image size overflows uint32") + image_end = load_addr + image_size + if entry_addr < load_addr or entry_addr >= image_end: + raise ValueError( + f"entry address 0x{entry_addr:08X} is outside the image range " + f"[0x{load_addr:08X}, 0x{image_end:08X})" + ) + + def build_header( image_size: int, load_addr: int, @@ -66,6 +125,12 @@ def build_header( ) -> bytes: """Build the eos_image_header_t binary structure.""" + validate_image_layout(image_size, load_addr, entry_addr) + if not 0 <= image_version <= UINT32_MAX: + raise ValueError("image version must fit uint32") + if not 0 <= image_crc <= UINT32_MAX or not 0 <= flags <= UINT32_MAX: + raise ValueError("image CRC and flags must fit uint32") + # Hash field: store CRC32 in first 4 bytes, rest zeroed hash_field = struct.pack(' None: + """Write data to output_path so that a failed run leaves no partial image. + + The data goes to a temporary file that mkstemp creates exclusively, with + a unique name, in the destination directory: it can never be the input, + another run's file or a planted symlink. It is then renamed over the + destination. An output symlink is written through, as a direct write + would: its target is replaced and the link is left in place. + """ + target = output_path.resolve() if output_path.is_symlink() else output_path + fd, temp_name = tempfile.mkstemp( + dir=target.parent, prefix=f".{target.name}.", suffix=".tmp") + try: + with os.fdopen(fd, "wb") as temp_file: + temp_file.write(data) + temp_file.flush() + os.fsync(temp_file.fileno()) + try: + # Repacking must not loosen an existing image's permissions. + shutil.copymode(target, temp_name) + except FileNotFoundError: + # A new output gets what a plain create would give it; mkstemp's + # 0600 would be stricter than the old direct write. + umask = os.umask(0) + os.umask(umask) + os.chmod(temp_name, 0o666 & ~umask) + os.replace(temp_name, target) + except BaseException: + try: + os.unlink(temp_name) + except FileNotFoundError: + pass + except OSError as cleanup_error: + print(f"Warning: cannot remove temporary file {temp_name}: " + f"{cleanup_error}", file=sys.stderr) + raise + + def main(): parser = argparse.ArgumentParser( description='eBootloader Image Packer — creates .eimg images' @@ -104,21 +207,38 @@ def main(): args = parser.parse_args() input_path = Path(args.input) - if not input_path.exists(): + if not input_path.is_file(): print(f"Error: input file not found: {args.input}", file=sys.stderr) sys.exit(1) - firmware_data = input_path.read_bytes() + output_path = Path(args.output) + if input_path.resolve() == output_path.resolve(): + print("Error: input and output paths must be different", file=sys.stderr) + sys.exit(1) + + try: + # Check the size before reading, and never read more than one byte + # past the limit, so an oversized input cannot exhaust memory. + validate_image_size(input_path.stat().st_size) + with input_path.open("rb") as input_file: + firmware_data = input_file.read(MAX_IMAGE_SIZE + 1) + except OSError as e: + print(f"Error: cannot read input file: {e}", file=sys.stderr) + sys.exit(1) + except ValueError as e: + print(f"Error: {e}", file=sys.stderr) + sys.exit(1) image_size = len(firmware_data) - load_addr = int(args.load_addr, 16) - entry_addr = int(args.entry_addr, 16) try: + load_addr = parse_uint32(args.load_addr, "load address") + entry_addr = parse_uint32(args.entry_addr, "entry address") + flags = parse_uint32(args.flags, "flags") image_version = parse_version(args.version) + validate_image_layout(image_size, load_addr, entry_addr) except ValueError as e: print(f"Error: {e}", file=sys.stderr) sys.exit(1) - flags = int(args.flags, 16) image_crc = compute_crc32(firmware_data) header = build_header( @@ -130,8 +250,11 @@ def main(): flags=flags, ) - output_path = Path(args.output) - output_path.write_bytes(header + firmware_data) + try: + write_output_atomically(output_path, header + firmware_data) + except OSError as e: + print(f"Error: cannot write output image: {e}", file=sys.stderr) + sys.exit(1) ver_major = (image_version >> 24) & 0xFF ver_minor = (image_version >> 16) & 0xFF