Switch to str.startswith for matching like the FIT etype does since the current version doesn't ignore 'hash-1', 'hash-2', etc.

Signed-off-by: Alper Nebi Yasak alpernebiyasak@gmail.com ---

Changes in v2: - Add test to check that sections ignore hash*, signature* nodes

tools/binman/etype/section.py | 2 +- tools/binman/ftest.py | 6 +++ .../165_section_ignore_hash_signature.dts | 40 +++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/165_section_ignore_hash_signature.dts

diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 73c5553c81..c5166a5b57 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -83,7 +83,7 @@ class Entry_section(Entry):

def _ReadEntries(self): for node in self._node.subnodes: - if node.name == 'hash': + if node.name.startswith('hash') or node.name.startswith('signature'): continue entry = Entry.Create(self, node) entry.ReadNode() diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 5f650b5f94..ab88ee96ab 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3477,5 +3477,11 @@ class TestFunctional(unittest.TestCase): fnode = dtb.GetNode('/images/kernel') self.assertNotIn('data', fnode.props)

+ def testSectionIgnoreHashSignature(self): + """Test that sections ignore hash, signature nodes for its data""" + data = self._DoReadFile('165_section_ignore_hash_signature.dts') + expected = (U_BOOT_DATA + U_BOOT_DATA) + self.assertEqual(expected, data) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/165_section_ignore_hash_signature.dts b/tools/binman/test/165_section_ignore_hash_signature.dts new file mode 100644 index 0000000000..8adbe25512 --- /dev/null +++ b/tools/binman/test/165_section_ignore_hash_signature.dts @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + section@0 { + u-boot { + }; + hash { + algo = "sha256"; + }; + signature { + algo = "sha256,rsa2048"; + key-name-hint = "dev"; + }; + }; + section@1 { + u-boot { + }; + hash-1 { + algo = "sha1"; + }; + hash-2 { + algo = "sha256"; + }; + signature-1 { + algo = "sha1,rsa2048"; + key-name-hint = "dev"; + }; + signature-2 { + algo = "sha256,rsa2048"; + key-name-hint = "dev"; + }; + }; + }; +};

When reading subentries of each image, the FIT entry type directly concatenates their contents without padding them according to their offset, size, align, align-size, align-end, pad-before, pad-after properties.

This patch makes sure these properties are respected by offloading this image-data building to the section etype, where each subnode of the "images" node is processed as a section. Alignments and offsets are respective to the beginning of each image. For example, the following fragment can end up having "u-boot-spl" start at 0x88 within the final FIT binary, while "u-boot" would then end up starting at e.g. 0x20088.

fit { description = "example";

images { kernel-1 { description = "U-Boot with SPL"; type = "kernel"; arch = "arm64"; os = "linux"; compression = "none";

u-boot-spl { }; u-boot { align = <0x10000>; }; }; }; }

Signed-off-by: Alper Nebi Yasak alpernebiyasak@gmail.com ---

Changes in v2: - Renumber test to accommodate for the first patch's new test - Use 'section' instead of 'image' for FIT subimage sections - Also rename 'Members:' comment for the renamed _fit_content variable - Clarify comments around FIT subimage section/content processing - Don't check for section.ObtainContents() returning False (never does)

tools/binman/etype/fit.py | 41 +++++++------ tools/binman/ftest.py | 23 ++++++++ .../test/167_fit_image_subentry_alignment.dts | 57 +++++++++++++++++++ 3 files changed, 104 insertions(+), 17 deletions(-) create mode 100644 tools/binman/test/167_fit_image_subentry_alignment.dts

diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index 75712f4409..acba53aa92 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -55,14 +55,14 @@ class Entry_fit(Entry): """ Members: _fit: FIT file being built - _fit_content: dict: + _fit_sections: dict: key: relative path to entry Node (from the base of the FIT) - value: List of Entry objects comprising the contents of this + value: Entry_section object comprising the contents of this node """ super().__init__(section, etype, node) self._fit = None - self._fit_content = defaultdict(list) + self._fit_sections = {} self._fit_props = {}

def ReadNode(self): @@ -91,15 +91,23 @@ class Entry_fit(Entry):

rel_path = node.path[len(base_node.path):] has_images = depth == 2 and rel_path.startswith('/images/') + if has_images: + # This node is a FIT subimage node (e.g. "/images/kernel") + # containing content nodes. We collect the subimage nodes and + # section entries for them here to merge the content subnodes + # together and put the merged contents in the subimage node's + # 'data' property later. + entry = Entry.Create(self.section, node, etype='section') + entry.ReadNode() + self._fit_sections[rel_path] = entry + for subnode in node.subnodes: if has_images and not (subnode.name.startswith('hash') or subnode.name.startswith('signature')): - # This is a content node. We collect all of these together - # and put them in the 'data' property. They do not appear - # in the FIT. - entry = Entry.Create(self.section, subnode) - entry.ReadNode() - self._fit_content[rel_path].append(entry) + # This subnode is a content node not meant to appear in + # the FIT (e.g. "/images/kernel/u-boot"), so don't call + # fsw.add_node() or _AddNode() for it. + pass else: with fsw.add_node(subnode.name): _AddNode(base_node, depth + 1, subnode) @@ -123,9 +131,8 @@ class Entry_fit(Entry): This adds the 'data' properties to the input ITB (Image-tree Binary) then runs mkimage to process it. """ + # self._BuildInput() either returns bytes or raises an exception. data = self._BuildInput(self._fdt) - if data == False: - return False uniq = self.GetUniqueName() input_fname = tools.GetOutputFilename('%s.itb' % uniq) output_fname = tools.GetOutputFilename('%s.fit' % uniq) @@ -150,13 +157,13 @@ class Entry_fit(Entry): Returns: New fdt contents (bytes) """ - for path, entries in self._fit_content.items(): + for path, section in self._fit_sections.items(): node = fdt.GetNode(path) - data = b'' - for entry in entries: - if not entry.ObtainContents(): - return False - data += entry.GetData() + # Entry_section.ObtainContents() either returns True or + # raises an exception. + section.ObtainContents() + section.Pack(0) + data = section.GetData() node.AddData('data', data)

fdt.Sync(auto_resize=True) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 53da709d51..760a2dd44f 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3489,6 +3489,29 @@ class TestFunctional(unittest.TestCase): expected = (U_BOOT_DATA + tools.GetBytes(ord('!'), 12) + U_BOOT_DATA + tools.GetBytes(ord('!'), 6) + U_BOOT_DATA) + + def testFitImageSubentryAlignment(self): + """Test relative alignability of FIT image subentries""" + entry_args = { + 'test-id': TEXT_DATA, + } + data, _, _, _ = self._DoReadFileDtb('167_fit_image_subentry_alignment.dts', + entry_args=entry_args) + dtb = fdt.Fdt.FromData(data) + dtb.Scan() + + node = dtb.GetNode('/images/kernel') + data = dtb.GetProps(node)["data"].bytes + align_pad = 0x10 - (len(U_BOOT_SPL_DATA) % 0x10) + expected = (tools.GetBytes(0, 0x20) + U_BOOT_SPL_DATA + + tools.GetBytes(0, align_pad) + U_BOOT_DATA) + self.assertEqual(expected, data) + + node = dtb.GetNode('/images/fdt-1') + data = dtb.GetProps(node)["data"].bytes + expected = (U_BOOT_SPL_DTB_DATA + tools.GetBytes(0, 20) + + tools.ToBytes(TEXT_DATA) + tools.GetBytes(0, 30) + + U_BOOT_DTB_DATA) self.assertEqual(expected, data)

if __name__ == "__main__": diff --git a/tools/binman/test/167_fit_image_subentry_alignment.dts b/tools/binman/test/167_fit_image_subentry_alignment.dts new file mode 100644 index 0000000000..360cac5266 --- /dev/null +++ b/tools/binman/test/167_fit_image_subentry_alignment.dts @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + fit { + description = "test-desc"; + #address-cells = <1>; + + images { + kernel { + description = "Offset-Align Test"; + type = "kernel"; + arch = "arm64"; + os = "linux"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + u-boot-spl { + offset = <0x20>; + }; + u-boot { + align = <0x10>; + }; + }; + fdt-1 { + description = "Pad-Before-After Test"; + type = "flat_dt"; + arch = "arm64"; + compression = "none"; + u-boot-spl-dtb { + }; + text { + text-label = "test-id"; + pad-before = <20>; + pad-after = <30>; + }; + u-boot-dtb { + }; + }; + }; + + configurations { + default = "conf-1"; + conf-1 { + description = "Kernel with FDT blob"; + kernel = "kernel"; + fdt = "fdt-1"; + }; + }; + }; + }; +};