From 2f80c5ef134c2c339f6d4ad2f9a21aa0ffd465a8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 7 Jan 2023 14:07:14 -0700 Subject: [PATCH] binman: Support new op-tee binary format OP-TEE has a format with a binary header that can be used instead of the ELF file. With newer versions of OP-TEE this may be required on some platforms. Add support for this in binman. First, add a method to obtain the ELF sections from an entry, then use that in the FIT support. We then end up with the ability to support both types of OP-TEE files, depending on which one is passed in with the entry argument (TEE=xxx in the U-Boot build). Signed-off-by: Simon Glass --- tools/binman/entries.rst | 37 ++++++++- tools/binman/entry.py | 13 +++ tools/binman/etype/fit.py | 72 +++++++++-------- tools/binman/etype/section.py | 9 +++ tools/binman/etype/tee_os.py | 76 +++++++++++++++++- tools/binman/ftest.py | 83 ++++++++++++++++++++ tools/binman/test/263_tee_os_opt.dts | 22 ++++++ tools/binman/test/264_tee_os_opt_fit.dts | 33 ++++++++ tools/binman/test/265_tee_os_opt_fit_bad.dts | 40 ++++++++++ 9 files changed, 352 insertions(+), 33 deletions(-) create mode 100644 tools/binman/test/263_tee_os_opt.dts create mode 100644 tools/binman/test/264_tee_os_opt_fit.dts create mode 100644 tools/binman/test/265_tee_os_opt_fit_bad.dts diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index 5b9eb8b82c..f6cc800385 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -1508,12 +1508,47 @@ Entry: tee-os: Entry containing an OP-TEE Trusted OS (TEE) blob Properties / Entry arguments: - tee-os-path: Filename of file to read into entry. This is typically - called tee-pager.bin + called tee.bin or tee.elf This entry holds the run-time firmware, typically started by U-Boot SPL. See the U-Boot README for your architecture or board for how to use it. See https://github.com/OP-TEE/optee_os for more information about OP-TEE. +Note that if the file is in ELF format, it must go in a FIT. In that case, +this entry will mark itself as absent, providing the data only through the +read_elf_segments() method. + +Marking this entry as absent means that it if is used in the wrong context +it can be automatically dropped. Thus it is possible to add an OP-TEE entry +like this:: + + binman { + tee-os { + }; + }; + +and pass either an ELF or plain binary in with -a tee-os-path +and have binman do the right thing: + + - include the entry if tee.bin is provided and it does NOT have the v1 + header + - drop it otherwise + +When used within a FIT, we can do:: + + binman { + fit { + tee-os { + }; + }; + }; + +which will split the ELF into separate nodes for each segment, if an ELF +file is provided (see :ref:`etype_fit`), or produce a single node if the +OP-TEE binary v1 format is provided (see optee_doc_) . + +.. _optee_doc: https://optee.readthedocs.io/en/latest/architecture/core.html#partitioning-of-the-binary + .. _etype_text: diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 637aece370..de51d29589 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1290,3 +1290,16 @@ features to produce new behaviours. def mark_absent(self, msg): tout.info("Entry '%s' marked absent: %s" % (self._node.path, msg)) self.absent = True + + def read_elf_segments(self): + """Read segments from an entry that can generate an ELF file + + Returns: + tuple: + list of segments, each: + int: Segment number (0 = first) + int: Start address of segment in memory + bytes: Contents of segment + int: entry address of ELF file + """ + return None diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index 8ad4f3a8a8..fea3adcc68 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -540,41 +540,35 @@ class Entry_fit(Entry_section): else: self.Raise("Generator node requires 'fit,fdt-list' property") - def _gen_split_elf(base_node, node, elf_data, missing): + def _gen_split_elf(base_node, node, segments, entry_addr): """Add nodes for the ELF file, one per group of contiguous segments Args: base_node (Node): Template node from the binman definition node (Node): Node to replace (in the FIT being built) - data (bytes): ELF-format data to process (may be empty) - missing (bool): True if any of the data is missing - + segments (list): list of segments, each: + int: Segment number (0 = first) + int: Start address of segment in memory + bytes: Contents of segment + entry_addr (int): entry address of ELF file """ - # If any pieces are missing, skip this. The missing entries will - # show an error - if not missing: - try: - segments, entry = elf.read_loadable_segments(elf_data) - except ValueError as exc: - self._raise_subnode(node, - f'Failed to read ELF file: {str(exc)}') - for (seq, start, data) in segments: - node_name = node.name[1:].replace('SEQ', str(seq + 1)) - with fsw.add_node(node_name): - loadables.append(node_name) - for pname, prop in node.props.items(): - if not pname.startswith('fit,'): - fsw.property(pname, prop.bytes) - elif pname == 'fit,load': - fsw.property_u32('load', start) - elif pname == 'fit,entry': - if seq == 0: - fsw.property_u32('entry', entry) - elif pname == 'fit,data': - fsw.property('data', bytes(data)) - elif pname != 'fit,operation': - self._raise_subnode( - node, f"Unknown directive '{pname}'") + for (seq, start, data) in segments: + node_name = node.name[1:].replace('SEQ', str(seq + 1)) + with fsw.add_node(node_name): + loadables.append(node_name) + for pname, prop in node.props.items(): + if not pname.startswith('fit,'): + fsw.property(pname, prop.bytes) + elif pname == 'fit,load': + fsw.property_u32('load', start) + elif pname == 'fit,entry': + if seq == 0: + fsw.property_u32('entry', entry_addr) + elif pname == 'fit,data': + fsw.property('data', bytes(data)) + elif pname != 'fit,operation': + self._raise_subnode( + node, f"Unknown directive '{pname}'") def _gen_node(base_node, node, depth, in_images, entry): """Generate nodes from a template @@ -598,6 +592,8 @@ class Entry_fit(Entry_section): depth (int): Current node depth (0 is the base 'fit' node) in_images (bool): True if this is inside the 'images' node, so that 'data' properties should be generated + entry (entry_Section): Entry for the section containing the + contents of this node """ oper = self._get_operation(base_node, node) if oper == OP_GEN_FDT_NODES: @@ -609,10 +605,24 @@ class Entry_fit(Entry_section): missing_list = [] entry.ObtainContents() entry.Pack(0) - data = entry.GetData() entry.CheckMissing(missing_list) - _gen_split_elf(base_node, node, data, bool(missing_list)) + # If any pieces are missing, skip this. The missing entries will + # show an error + if not missing_list: + segs = entry.read_elf_segments() + if segs: + segments, entry_addr = segs + else: + elf_data = entry.GetData() + try: + segments, entry_addr = ( + elf.read_loadable_segments(elf_data)) + except ValueError as exc: + self._raise_subnode( + node, f'Failed to read ELF file: {str(exc)}') + + _gen_split_elf(base_node, node, segments, entry_addr) def _add_node(base_node, depth, node): """Add nodes to the output FIT diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index dcb7a06204..57bfee0b28 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -948,3 +948,12 @@ class Entry_section(Entry): super().AddBintools(btools) for entry in self._entries.values(): entry.AddBintools(btools) + + def read_elf_segments(self): + entries = self.GetEntries() + + # If the section only has one entry, see if it can provide ELF segments + if len(entries) == 1: + for entry in entries.values(): + return entry.read_elf_segments() + return None diff --git a/tools/binman/etype/tee_os.py b/tools/binman/etype/tee_os.py index 6ce4b672de..5529727e83 100644 --- a/tools/binman/etype/tee_os.py +++ b/tools/binman/etype/tee_os.py @@ -4,19 +4,93 @@ # Entry-type module for OP-TEE Trusted OS firmware blob # +import struct + from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg +from binman import elf class Entry_tee_os(Entry_blob_named_by_arg): """Entry containing an OP-TEE Trusted OS (TEE) blob Properties / Entry arguments: - tee-os-path: Filename of file to read into entry. This is typically - called tee-pager.bin + called tee.bin or tee.elf This entry holds the run-time firmware, typically started by U-Boot SPL. See the U-Boot README for your architecture or board for how to use it. See https://github.com/OP-TEE/optee_os for more information about OP-TEE. + + Note that if the file is in ELF format, it must go in a FIT. In that case, + this entry will mark itself as absent, providing the data only through the + read_elf_segments() method. + + Marking this entry as absent means that it if is used in the wrong context + it can be automatically dropped. Thus it is possible to add an OP-TEE entry + like this:: + + binman { + tee-os { + }; + }; + + and pass either an ELF or plain binary in with -a tee-os-path + and have binman do the right thing: + + - include the entry if tee.bin is provided and it does NOT have the v1 + header + - drop it otherwise + + When used within a FIT, we can do:: + + binman { + fit { + tee-os { + }; + }; + }; + + which will split the ELF into separate nodes for each segment, if an ELF + file is provided (see :ref:`etype_fit`), or produce a single node if the + OP-TEE binary v1 format is provided (see optee_doc_) . + + .. _optee_doc: https://optee.readthedocs.io/en/latest/architecture/core.html#partitioning-of-the-binary """ def __init__(self, section, etype, node): super().__init__(section, etype, node, 'tee-os') self.external = True + + @staticmethod + def is_optee_bin_v1(data): + return len(data) >= 8 and data[0:5] == b'OPTE\x01' + + def ObtainContents(self, fake_size=0): + result = super().ObtainContents(fake_size) + if not self.missing: + # If using the flat binary (without the OP-TEE header), then it is + # just included as a blob. But if it is an ELF or usees the v1 + # binary header, then the FIT implementation will call + # read_elf_segments() to get the segment information + if elf.is_valid(self.data): + self.mark_absent('uses Elf format which must be in a FIT') + elif self.is_optee_bin_v1(self.data): + # The FIT implementation will call read_elf_segments() to get + # the segment information + self.mark_absent('uses v1 format which must be in a FIT') + return result + + def read_elf_segments(self): + data = self.GetData() + if self.is_optee_bin_v1(data): + # OP-TEE v1 format (tee.bin) + init_sz, start_hi, start_lo, _, paged_sz = ( + struct.unpack_from('<5I', data, 0x8)) + if paged_sz != 0: + self.Raise("OP-TEE paged mode not supported") + e_entry = (start_hi << 32) + start_lo + p_addr = e_entry + p_data = data[0x1c:] + if len(p_data) != init_sz: + self.Raise("Invalid OP-TEE file: size mismatch (expected %#x, have %#x)" % + (init_sz, len(p_data))) + return [[0, p_addr, p_data]], e_entry + return None diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index f47a745f1e..f893050e70 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -112,6 +112,8 @@ REPACK_DTB_PROPS = ['orig-offset', 'orig-size'] # Supported compression bintools COMP_BINTOOLS = ['bzip2', 'gzip', 'lz4', 'lzma_alone', 'lzop', 'xz', 'zstd'] +TEE_ADDR = 0x5678 + class TestFunctional(unittest.TestCase): """Functional tests for binman @@ -219,6 +221,9 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('tee.elf', tools.read_file(cls.ElfTestFile('elf_sections'))) + # Newer OP_TEE file in v1 binary format + cls.make_tee_bin('tee.bin') + cls.comp_bintools = {} for name in COMP_BINTOOLS: cls.comp_bintools[name] = bintool.Bintool.create(name) @@ -644,6 +649,14 @@ class TestFunctional(unittest.TestCase): def ElfTestFile(cls, fname): return os.path.join(cls._elf_testdir, fname) + @classmethod + def make_tee_bin(cls, fname, paged_sz=0, extra_data=b''): + init_sz, start_hi, start_lo, dummy = (len(U_BOOT_DATA), 0, TEE_ADDR, 0) + data = b'OPTE\x01xxx' + struct.pack('<5I', init_sz, start_hi, start_lo, + dummy, paged_sz) + U_BOOT_DATA + data += extra_data + TestFunctional._MakeInputFile(fname, data) + def AssertInList(self, grep_list, target): """Assert that at least one of a list of things is in a target @@ -6095,6 +6108,76 @@ fdt fdtmap Extract the devicetree blob from the fdtmap data = self._DoReadFile('262_absent.dts') self.assertEqual(U_BOOT_DATA + U_BOOT_IMG_DATA, data) + def testPackTeeOsOptional(self): + """Test that an image with an optional TEE binary can be created""" + entry_args = { + 'tee-os-path': 'tee.elf', + } + data = self._DoReadFileDtb('263_tee_os_opt.dts', + entry_args=entry_args)[0] + self.assertEqual(U_BOOT_DATA + U_BOOT_IMG_DATA, data) + + def checkFitTee(self, dts, tee_fname): + """Check that a tee-os entry works and returns data + + Args: + dts (str): Device tree filename to use + tee_fname (str): filename containing tee-os + + Returns: + bytes: Image contents + """ + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') + entry_args = { + 'of-list': 'test-fdt1 test-fdt2', + 'default-dt': 'test-fdt2', + 'tee-os-path': tee_fname, + } + test_subdir = os.path.join(self._indir, TEST_FDT_SUBDIR) + data = self._DoReadFileDtb(dts, entry_args=entry_args, + extra_indirs=[test_subdir])[0] + return data + + def testFitTeeOsOptionalFit(self): + """Test an image with a FIT with an optional OP-TEE binary""" + data = self.checkFitTee('264_tee_os_opt_fit.dts', 'tee.bin') + + # There should be only one node, holding the data set up in SetUpClass() + # for tee.bin + dtb = fdt.Fdt.FromData(data) + dtb.Scan() + node = dtb.GetNode('/images/tee-1') + self.assertEqual(TEE_ADDR, + fdt_util.fdt32_to_cpu(node.props['load'].value)) + self.assertEqual(TEE_ADDR, + fdt_util.fdt32_to_cpu(node.props['entry'].value)) + self.assertEqual(U_BOOT_DATA, node.props['data'].bytes) + + def testFitTeeOsOptionalFitBad(self): + """Test an image with a FIT with an optional OP-TEE binary""" + with self.assertRaises(ValueError) as exc: + self.checkFitTee('265_tee_os_opt_fit_bad.dts', 'tee.bin') + self.assertIn( + "Node '/binman/fit': subnode 'images/@tee-SEQ': Failed to read ELF file: Magic number does not match", + str(exc.exception)) + + def testFitTeeOsBad(self): + """Test an OP-TEE binary with wrong formats""" + self.make_tee_bin('tee.bad1', 123) + with self.assertRaises(ValueError) as exc: + self.checkFitTee('264_tee_os_opt_fit.dts', 'tee.bad1') + self.assertIn( + "Node '/binman/fit/images/@tee-SEQ/tee-os': OP-TEE paged mode not supported", + str(exc.exception)) + + self.make_tee_bin('tee.bad2', 0, b'extra data') + with self.assertRaises(ValueError) as exc: + self.checkFitTee('264_tee_os_opt_fit.dts', 'tee.bad2') + self.assertIn( + "Node '/binman/fit/images/@tee-SEQ/tee-os': Invalid OP-TEE file: size mismatch (expected 0x4, have 0xe)", + str(exc.exception)) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/263_tee_os_opt.dts b/tools/binman/test/263_tee_os_opt.dts new file mode 100644 index 0000000000..2e4ec24ac2 --- /dev/null +++ b/tools/binman/test/263_tee_os_opt.dts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot { + }; + tee-os { + /* + * this results in nothing being added since only the + * .bin format is supported by this etype, unless it is + * part of a FIT + */ + }; + u-boot-img { + }; + }; +}; diff --git a/tools/binman/test/264_tee_os_opt_fit.dts b/tools/binman/test/264_tee_os_opt_fit.dts new file mode 100644 index 0000000000..ae44b433ed --- /dev/null +++ b/tools/binman/test/264_tee_os_opt_fit.dts @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + fit { + description = "test-desc"; + #address-cells = <1>; + fit,fdt-list = "of-list"; + + images { + @tee-SEQ { + fit,operation = "split-elf"; + description = "TEE"; + type = "tee"; + arch = "arm64"; + os = "tee"; + compression = "none"; + fit,load; + fit,entry; + fit,data; + + tee-os { + }; + }; + }; + }; + }; +}; diff --git a/tools/binman/test/265_tee_os_opt_fit_bad.dts b/tools/binman/test/265_tee_os_opt_fit_bad.dts new file mode 100644 index 0000000000..7fa363cc19 --- /dev/null +++ b/tools/binman/test/265_tee_os_opt_fit_bad.dts @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + fit { + description = "test-desc"; + #address-cells = <1>; + fit,fdt-list = "of-list"; + + images { + @tee-SEQ { + fit,operation = "split-elf"; + description = "TEE"; + type = "tee"; + arch = "arm64"; + os = "tee"; + compression = "none"; + fit,load; + fit,entry; + fit,data; + + tee-os { + }; + + /* + * mess up the ELF data by adding + * another bit of data at the end + */ + u-boot { + }; + }; + }; + }; + }; +}; -- 2.39.5