Binman can embed a copy of the image description into the images it builds as a fdtmap entry, but it omits the /binman/<image-name> prefix from the node paths while doing so. When reading an already-built image file, entries are reconstructed using this fdtmap and their associated nodes still lack that prefix.
Some entries like fit and vblock create intermediate files whose names are based on an entry unique name. This name is constructed from their node's path by concatenating the parents with dots up to the binman node, e.g. /binman/image/foo/bar becomes 'image.foo.bar'.
However, we don't have this /binman/image prefix when replacing entries in such an image. The /foo/bar entry we read when doing so erroneously has the unique name of '/.foo.bar', causing permission errors when the entry attempts to create files based on that.
Fix the unique-name generation by stopping at the '/' node like how it stops at the binman node. As the unique names are used as filenames, add tests that check if they're safe to use as filenames.
Signed-off-by: Alper Nebi Yasak alpernebiyasak@gmail.com ---
tools/binman/entry.py | 2 +- tools/binman/ftest.py | 31 ++++++++++++++++ tools/binman/test/230_unique_names.dts | 34 ++++++++++++++++++ tools/binman/test/231_unique_names_multi.dts | 38 ++++++++++++++++++++ 4 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/230_unique_names.dts create mode 100644 tools/binman/test/231_unique_names_multi.dts
diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 18a7a3510548..a07a5888643a 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -775,7 +775,7 @@ def GetUniqueName(self): node = self._node while node.parent: node = node.parent - if node.name == 'binman': + if node.name in ('binman', '/'): break name = '%s.%s' % (node.name, name) return name diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 876953f11324..e6f0159a229f 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -5471,6 +5471,37 @@ def testFitSplitElfFaked(self): err, "Image '.*' is missing external blobs and is non-functional: .*")
+ def _CheckSafeUniqueNames(self, *images): + """Check all entries of given images for unsafe unique names""" + for image in images: + entries = {} + image._CollectEntries(entries, {}, image) + for entry in entries.values(): + uniq = entry.GetUniqueName() + + # Used as part of a filename, so must not be absolute paths. + self.assertFalse(os.path.isabs(uniq)) + + def testSafeUniqueNames(self): + """Test entry unique names are safe in single image configuration""" + data = self._DoReadFileRealDtb('230_unique_names.dts') + + orig_image = control.images['image'] + image_fname = tools.get_output_filename('image.bin') + image = Image.FromFile(image_fname) + + self._CheckSafeUniqueNames(orig_image, image) + + def testSafeUniqueNamesMulti(self): + """Test entry unique names are safe with multiple images""" + data = self._DoReadFileRealDtb('231_unique_names_multi.dts') + + orig_image = control.images['image'] + image_fname = tools.get_output_filename('image.bin') + image = Image.FromFile(image_fname) + + self._CheckSafeUniqueNames(orig_image, image) +
if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/230_unique_names.dts b/tools/binman/test/230_unique_names.dts new file mode 100644 index 000000000000..6780d37f71f4 --- /dev/null +++ b/tools/binman/test/230_unique_names.dts @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <0xc00>; + allow-repack; + + u-boot { + }; + + fdtmap { + }; + + u-boot2 { + type = "u-boot"; + }; + + text { + text = "some text"; + }; + + u-boot-dtb { + }; + + image-header { + location = "end"; + }; + }; +}; diff --git a/tools/binman/test/231_unique_names_multi.dts b/tools/binman/test/231_unique_names_multi.dts new file mode 100644 index 000000000000..db63afb445e0 --- /dev/null +++ b/tools/binman/test/231_unique_names_multi.dts @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + multiple-images; + + image { + size = <0xc00>; + allow-repack; + + u-boot { + }; + + fdtmap { + }; + + u-boot2 { + type = "u-boot"; + }; + + text { + text = "some text"; + }; + + u-boot-dtb { + }; + + image-header { + location = "end"; + }; + }; + }; +};