[PATCH] binman: Skip processing "hash" subnodes of FIT subsections
Binman's FIT entry type can have image subentries with "hash" subnodes intended to be processed by mkimage, but not binman. However, the Entry class and any subclass that reuses its implementation tries to process these unconditionally. This can lead to an error when boards specify hash algorithms that binman doesn't support, but mkimage supports.
Let entries skip processing these "hash" subnodes based on an instance variable, and set this instance variable for FIT subsections. Also re-enable processing of calculated and missing properties of FIT entries which was disabled to mitigate this issue.
Signed-off-by: Alper Nebi Yasak alpernebiyasak@gmail.com --- This applies on top of u-boot-dm/master, and does not resend my "binman: Update image positions of FIT subentries" patch [1] which should be applied on top of this.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20220207220809.4497-6-alper...
tools/binman/entry.py | 15 +++++++++++---- tools/binman/etype/fit.py | 12 ++---------- 2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/tools/binman/entry.py b/tools/binman/entry.py index dc26f8f167b0..6f98353c8569 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -78,6 +78,8 @@ class Entry(object): external: True if this entry contains an external binary blob bintools: Bintools used by this entry (only populated for Image) missing_bintools: List of missing bintools for this entry + update_hash: True if this entry's "hash" subnode should be + updated with a hash of the entry contents """ def __init__(self, section, etype, node, name_prefix=''): # Put this here to allow entry-docs and help to work without libfdt @@ -111,6 +113,7 @@ def __init__(self, section, etype, node, name_prefix=''): self.allow_fake = False self.bintools = {} self.missing_bintools = [] + self.update_hash = True
@staticmethod def FindEntryClass(etype, expanded): @@ -315,9 +318,11 @@ def AddMissingProperties(self, have_image_pos):
if self.compress != 'none': state.AddZeroProp(self._node, 'uncomp-size') - err = state.CheckAddHashProp(self._node) - if err: - self.Raise(err) + + if self.update_hash: + err = state.CheckAddHashProp(self._node) + if err: + self.Raise(err)
def SetCalculatedProperties(self): """Set the value of device-tree properties calculated by binman""" @@ -333,7 +338,9 @@ def SetCalculatedProperties(self): state.SetInt(self._node, 'orig-size', self.orig_size, True) if self.uncomp_size is not None: state.SetInt(self._node, 'uncomp-size', self.uncomp_size) - state.CheckSetHashValue(self._node, self.GetData) + + if self.update_hash: + state.CheckSetHashValue(self._node, self.GetData)
def ProcessFdt(self, fdt): """Allow entries to adjust the device tree diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index a56b0564f9a1..a979d0f1a613 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -186,6 +186,8 @@ def _AddNode(base_node, depth, node): # 'data' property later. entry = Entry.Create(self.section, node, etype='section') entry.ReadNode() + # The hash subnodes here are for mkimage, not binman. + entry.update_hash = False self._entries[rel_path] = entry
for subnode in node.subnodes: @@ -294,13 +296,3 @@ def _BuildInput(self, fdt): def AddBintools(self, tools): super().AddBintools(tools) self.mkimage = self.AddBintool(tools, 'mkimage') - - def AddMissingProperties(self, have_image_pos): - # We don't want to interfere with any hash properties in the FIT, so - # disable this for now. - pass - - def SetCalculatedProperties(self): - # We don't want to interfere with any hash properties in the FIT, so - # disable this for now. - pass
Hi Alper,
On Tue, 8 Feb 2022 at 16:07, Alper Nebi Yasak alpernebiyasak@gmail.com wrote:
Binman's FIT entry type can have image subentries with "hash" subnodes intended to be processed by mkimage, but not binman. However, the Entry class and any subclass that reuses its implementation tries to process these unconditionally. This can lead to an error when boards specify hash algorithms that binman doesn't support, but mkimage supports.
Let entries skip processing these "hash" subnodes based on an instance variable, and set this instance variable for FIT subsections. Also re-enable processing of calculated and missing properties of FIT entries which was disabled to mitigate this issue.
Signed-off-by: Alper Nebi Yasak alpernebiyasak@gmail.com
This applies on top of u-boot-dm/master, and does not resend my "binman: Update image positions of FIT subentries" patch [1] which should be applied on top of this.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20220207220809.4497-6-alper...
Perfect, thanks
tools/binman/entry.py | 15 +++++++++++---- tools/binman/etype/fit.py | 12 ++---------- 2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/tools/binman/entry.py b/tools/binman/entry.py index dc26f8f167b0..6f98353c8569 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -78,6 +78,8 @@ class Entry(object): external: True if this entry contains an external binary blob bintools: Bintools used by this entry (only populated for Image) missing_bintools: List of missing bintools for this entry
update_hash: True if this entry's "hash" subnode should beupdated with a hash of the entry contents@@ -111,6 +113,7 @@ def __init__(self, section, etype, node, name_prefix=''): self.allow_fake = False self.bintools = {} self.missing_bintools = []
self.update_hash = True@staticmethod def FindEntryClass(etype, expanded):
@@ -315,9 +318,11 @@ def AddMissingProperties(self, have_image_pos):
if self.compress != 'none': state.AddZeroProp(self._node, 'uncomp-size')
err = state.CheckAddHashProp(self._node)if err:self.Raise(err)
if self.update_hash:err = state.CheckAddHashProp(self._node)if err:self.Raise(err)def SetCalculatedProperties(self): """Set the value of device-tree properties calculated by binman"""
@@ -333,7 +338,9 @@ def SetCalculatedProperties(self): state.SetInt(self._node, 'orig-size', self.orig_size, True) if self.uncomp_size is not None: state.SetInt(self._node, 'uncomp-size', self.uncomp_size)
state.CheckSetHashValue(self._node, self.GetData)
if self.update_hash:state.CheckSetHashValue(self._node, self.GetData)def ProcessFdt(self, fdt): """Allow entries to adjust the device tree
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index a56b0564f9a1..a979d0f1a613 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -186,6 +186,8 @@ def _AddNode(base_node, depth, node): # 'data' property later. entry = Entry.Create(self.section, node, etype='section') entry.ReadNode()
# The hash subnodes here are for mkimage, not binman.entry.update_hash = False
I know it is less Pythonic but I would like to have this be a function in Entry, like SetUpdateHash(bool), so that it feels more like that class has control over things.
self._entries[rel_path] = entry
Also can you please add a test that uses a FIT containing hash{} nodes?
for subnode in node.subnodes:@@ -294,13 +296,3 @@ def _BuildInput(self, fdt): def AddBintools(self, tools): super().AddBintools(tools) self.mkimage = self.AddBintool(tools, 'mkimage')
- def AddMissingProperties(self, have_image_pos):
# We don't want to interfere with any hash properties in the FIT, so# disable this for now.pass- def SetCalculatedProperties(self):
# We don't want to interfere with any hash properties in the FIT, so# disable this for now.pass-- 2.34.1
Regards, Simon
participants (2)
-
Alper Nebi Yasak -
Simon Glass