scripts: add script for processing IMX6ULL system images#286
scripts: add script for processing IMX6ULL system images#286jmaksymowicz wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Python script, scripts/imx6ull_image_create.py, designed to fix up and sign IMX6ULL system images using the IMX CST tool. The reviewer feedback highlights several critical areas for improvement: replacing configparser with a regex-based replacement to avoid case-sensitivity and duplicate section issues, adding boundary validation for calculated offsets (boot_data_offset, dcd_offset, and dcd_size) to prevent out-of-bounds file operations, and replacing runtime assert statements with explicit exceptions to ensure reliability when assertions are disabled.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| cst = configparser.ConfigParser() | ||
| success = cst.read(cst_path) | ||
| if not success: | ||
| raise RuntimeError(f"Could not read CST file: {cst_path}") | ||
|
|
||
| cst["Authenticate Data"]["Blocks"] = ", ".join( | ||
| map(lambda x: f"0x{x.load_addr:08x} 0x{x.file_offset:08x} 0x{x.size:08x} {quote_path(x.file)}", blocks) | ||
| ) | ||
|
|
||
| with ( | ||
| tempfile.NamedTemporaryFile("w", suffix=".cst", dir=sign.temp_dir, delete_on_close=False) as filled_cst, | ||
| tempfile.NamedTemporaryFile("w", suffix=".bin", dir=sign.temp_dir, delete_on_close=False) as sig_file, | ||
| ): | ||
| logging.debug("Filled CST in %s, signature in %s", filled_cst.name, sig_file.name) | ||
| cst.write(filled_cst, True) | ||
| filled_cst.close() | ||
| sig_file.close() | ||
| sig_file_path = Path(sig_file.name).absolute() | ||
| filled_cst_path = Path(filled_cst.name).absolute() | ||
| cmd_result = subprocess.run( | ||
| [str(sign.cst_bin), "--o", str(sig_file_path), "--i", str(filled_cst_path)], | ||
| cwd=sign.cst_dir, | ||
| capture_output=True, | ||
| ) | ||
|
|
||
| output = cmd_result.stdout.decode("utf-8").strip() + "\n" + cmd_result.stderr.decode("utf-8").strip() | ||
| if cmd_result.returncode != 0: | ||
| raise RuntimeError(f"CST tool failed with code {cmd_result.returncode}: {output}") | ||
| else: | ||
| logging.debug(output) | ||
|
|
||
| sig_bin = sig_file_path.read_bytes() |
There was a problem hiding this comment.
Using configparser.ConfigParser to parse and write the CST file has several critical issues:
- Case Sensitivity: By default,
configparserconverts all keys to lowercase (e.g.,Enginebecomesengine). The NXP CST tool is case-sensitive and will fail to parse the modified configuration. - Duplicate Sections: CST files often contain duplicate sections (e.g., multiple
[Install Key]sections).configparserwill raise aDuplicateSectionErroror overwrite them, corrupting the configuration. - Loss of Comments/Formatting: Writing back via
configparser.write()strips all comments and alters the original formatting of the template. - Python 3.12+ Dependency: The use of
delete_on_close=Falseintempfile.NamedTemporaryFilewas introduced in Python 3.12. It will crash on older Python versions (like 3.10 or 3.8) which are common in build environments.
Instead of parsing the entire file with configparser, it is much safer and more robust to use a regular expression to replace only the Blocks line under the [Authenticate Data] section, and use delete=False with manual cleanup for Python compatibility.
import re
cst_text = cst_path.read_text()
blocks_str = ", ".join(
f"0x{x.load_addr:08x} 0x{x.file_offset:08x} 0x{x.size:08x} {quote_path(x.file)}" for x in blocks
)
pattern = r"(\[Authenticate Data\][^\[]*?Blocks\s*=\s*)[^\n]*"
if not re.search(pattern, cst_text, re.DOTALL):
raise RuntimeError("Could not find [Authenticate Data] section with Blocks option in CST template")
new_cst_text = re.sub(pattern, lambda m: m.group(1) + blocks_str, cst_text, flags=re.DOTALL)
filled_cst = tempfile.NamedTemporaryFile("w", suffix=".cst", dir=sign.temp_dir, delete=False)
sig_file = tempfile.NamedTemporaryFile("w", suffix=".bin", dir=sign.temp_dir, delete=False)
try:
logging.debug("Filled CST in %s, signature in %s", filled_cst.name, sig_file.name)
filled_cst.write(new_cst_text)
filled_cst.close()
sig_file.close()
sig_file_path = Path(sig_file.name).absolute()
filled_cst_path = Path(filled_cst.name).absolute()
cmd_result = subprocess.run(
[str(sign.cst_bin), "--o", str(sig_file_path), "--i", str(filled_cst_path)],
cwd=sign.cst_dir,
capture_output=True,
)
output = cmd_result.stdout.decode("utf-8").strip() + "\n" + cmd_result.stderr.decode("utf-8").strip()
if cmd_result.returncode != 0:
raise RuntimeError(f"CST tool failed with code {cmd_result.returncode}: {output}")
else:
logging.debug(output)
sig_bin = sig_file_path.read_bytes()
finally:
for f in (filled_cst, sig_file):
try:
os.unlink(f.name)
except OSError:
passThere was a problem hiding this comment.
Case Sensitivity
Loss of Comments/Formatting
These are not problems, the CST tool accepts these files without issue
Duplicate Sections
This may be a problem, so far only tried to install one key
Python 3.12+ Dependency
Oh well, you try to do something the right way then it turns out you can't 🙃
| if boot_data_ptr != 0: | ||
| boot_data_offset = boot_data_ptr - ivt_self + ivt_offset | ||
| image_size_with_csf = image_size + csf_size | ||
| # Write size of the whole image to boot_data.size | ||
| f.seek(boot_data_offset + BootDataOffsets.size) | ||
| f.write(image_size_with_csf.to_bytes(4, "little")) |
There was a problem hiding this comment.
When parsing the IVT header, the boot_data_ptr and ivt_self values are read from the file. If these values are corrupted or malicious, the calculated boot_data_offset could point outside the file bounds, causing f.seek() to fail or write to an arbitrary location.
We should validate that boot_data_offset is within the file bounds before seeking and writing.
| if boot_data_ptr != 0: | |
| boot_data_offset = boot_data_ptr - ivt_self + ivt_offset | |
| image_size_with_csf = image_size + csf_size | |
| # Write size of the whole image to boot_data.size | |
| f.seek(boot_data_offset + BootDataOffsets.size) | |
| f.write(image_size_with_csf.to_bytes(4, "little")) | |
| if boot_data_ptr != 0: | |
| boot_data_offset = boot_data_ptr - ivt_self + ivt_offset | |
| if not (0 <= boot_data_offset <= image_size - 8): | |
| raise RuntimeError("Calculated boot_data_offset is out of bounds") | |
| image_size_with_csf = image_size + csf_size | |
| # Write size of the whole image to boot_data.size | |
| f.seek(boot_data_offset + BootDataOffsets.size) | |
| f.write(image_size_with_csf.to_bytes(4, "little")) |
| if serial_dl: | ||
| # Zero out DCD pointer because it will be written to RAM separately | ||
| dcd_ptr = get_u32_from_file(f, ivt_offset + IvtOffsets.dcd) | ||
| if dcd_ptr != 0: | ||
| dcd_offset = dcd_ptr - ivt_self + ivt_offset | ||
| f.seek(ivt_offset + IvtOffsets.dcd) | ||
| f.write(b"\0\0\0\0") | ||
| dcd_header = get_u32_from_file(f, dcd_offset, True) | ||
| dcd_size = (dcd_header >> 8) & 0xFFFF | ||
| blocks.append(VerificationBlock(DCD_WRITE_ADDR, dcd_offset, dcd_size, img_out)) |
There was a problem hiding this comment.
In serial_dl mode, the script parses the DCD pointer and size from the image. If dcd_ptr is invalid, dcd_offset could be out of bounds. Furthermore, if dcd_size is corrupted or extremely large, the CST tool will fail when trying to read beyond the end of the file.
We should validate both dcd_offset and dcd_size to ensure they are within the file bounds.
| if serial_dl: | |
| # Zero out DCD pointer because it will be written to RAM separately | |
| dcd_ptr = get_u32_from_file(f, ivt_offset + IvtOffsets.dcd) | |
| if dcd_ptr != 0: | |
| dcd_offset = dcd_ptr - ivt_self + ivt_offset | |
| f.seek(ivt_offset + IvtOffsets.dcd) | |
| f.write(b"\0\0\0\0") | |
| dcd_header = get_u32_from_file(f, dcd_offset, True) | |
| dcd_size = (dcd_header >> 8) & 0xFFFF | |
| blocks.append(VerificationBlock(DCD_WRITE_ADDR, dcd_offset, dcd_size, img_out)) | |
| if serial_dl: | |
| # Zero out DCD pointer because it will be written to RAM separately | |
| dcd_ptr = get_u32_from_file(f, ivt_offset + IvtOffsets.dcd) | |
| if dcd_ptr != 0: | |
| dcd_offset = dcd_ptr - ivt_self + ivt_offset | |
| if not (0 <= dcd_offset < image_size): | |
| raise RuntimeError("Calculated DCD offset is out of bounds") | |
| f.seek(ivt_offset + IvtOffsets.dcd) | |
| f.write(b"\0\0\0\0") | |
| dcd_header = get_u32_from_file(f, dcd_offset, True) | |
| dcd_size = (dcd_header >> 8) & 0xFFFF | |
| if dcd_offset + dcd_size > image_size: | |
| raise RuntimeError("DCD size exceeds image boundaries") | |
| blocks.append(VerificationBlock(DCD_WRITE_ADDR, dcd_offset, dcd_size, img_out)) |
There was a problem hiding this comment.
I don't know if this level of paranoia is warranted. Of course someone may input a malformed image, but they will find out soon enough when it fails to boot. Maybe it would be nice to verify the image during build but I assume someone messing with the kernel image layout knows what they're doing.
| file.seek(offset) | ||
| b = file.read(4) | ||
| file.seek(prev_offset) | ||
| assert len(b) == 4 |
There was a problem hiding this comment.
Using assert for runtime data validation is discouraged in Python because assertions can be globally disabled when Python is run with the -O (optimize) flag. If assertions are disabled, this check is bypassed, which can lead to silent data corruption or unexpected errors later in the script.
It is safer to raise a ValueError or RuntimeError instead.
| assert len(b) == 4 | |
| if len(b) != 4: | |
| raise ValueError(f"Failed to read 4 bytes from offset {offset}") |
Unit Test Results10 860 tests +275 10 190 ✅ +275 53m 13s ⏱️ +32s Results for commit 8877017. ± Comparison against base commit 1d0b74e. This pull request removes 3 and adds 278 tests. Note that renamed tests count towards both. |
1 participant
Description
After changes introduced by phoenix-rtos/phoenix-rtos-kernel#802 additional fixups of the kernel image will be necessary to make it bootable. This script can perform these changes as well as sign the image using IMX CST tool: https://www.nxp.com/webapp/Download?colCode=IMX_CST_TOOL
TODO: the CST template name will probably need to be configurable (currently hardcoded
phoenix.cst)Motivation and Context
YT: MSH-42
Types of changes
How Has This Been Tested?
Checklist:
Special treatment