[PATCH] binman: Correct the error message for a bad hash algorithm
This shows an internal type at present, rather than the algorithm name. Fix it and update the test to catch this.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/ftest.py | 2 +- tools/binman/state.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 5400f76c67..3e15de63d5 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2073,7 +2073,7 @@ class TestFunctional(unittest.TestCase): def testHashBadAlgo(self): with self.assertRaises(ValueError) as e: self._DoReadFileDtb('092_hash_bad_algo.dts', update_dtb=True) - self.assertIn("Node '/binman/u-boot': Unknown hash algorithm", + self.assertIn("Node '/binman/u-boot': Unknown hash algorithm 'invalid'", str(e.exception))
def testHashSection(self): diff --git a/tools/binman/state.py b/tools/binman/state.py index af0a65e841..843aefd28d 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -397,7 +397,7 @@ def CheckAddHashProp(node): if algo.value == 'sha256': size = 32 else: - return "Unknown hash algorithm '%s'" % algo + return "Unknown hash algorithm '%s'" % algo.value for n in GetUpdateNodes(hash_node): n.AddEmptyProp('value', size)
On 08/02/2022 20:59, Simon Glass wrote:
This shows an internal type at present, rather than the algorithm name. Fix it and update the test to catch this.
Signed-off-by: Simon Glass sjg@chromium.org
tools/binman/ftest.py | 2 +- tools/binman/state.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Alper Nebi Yasak alpernebiyasak@gmail.com
I saw the failing build for my series [1]. Looks to me like binman doesn't support crc32 in hash nodes, and turning FIT into a Section simply exposed that. I tried a sloppy fix, see below.
[1] https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/388771
diff --git a/tools/binman/state.py b/tools/binman/state.py index af0a65e841..843aefd28d 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -397,7 +397,7 @@ def CheckAddHashProp(node): if algo.value == 'sha256': size = 32
If I add here:
+ elif algo.value == 'crc32': + size = 8
and a crc32 implementation attempt to the next function:
else:
return "Unknown hash algorithm '%s'" % algo
return "Unknown hash algorithm '%s'" % algo.value for n in GetUpdateNodes(hash_node): n.AddEmptyProp('value', size)
def CheckSetHashValue(node, get_data_func): hash_node = node.FindNode('hash') if hash_node: algo = hash_node.props.get('algo').value if algo == 'sha256': m = hashlib.sha256() m.update(get_data_func()) data = m.digest() + elif algo == 'crc32': + data = zlib.crc32(get_data_func()).to_bytes(8, 'little') for n in GetUpdateNodes(hash_node): n.SetData('value', data)
the failing boards start building again. Not sure how correct this is though (especially the endianness). Do we need this and maybe a test with crc32 hash now?
Hi Alper,
On Tue, 8 Feb 2022 at 11:54, Alper Nebi Yasak alpernebiyasak@gmail.com wrote:
On 08/02/2022 20:59, Simon Glass wrote:
This shows an internal type at present, rather than the algorithm name. Fix it and update the test to catch this.
Signed-off-by: Simon Glass sjg@chromium.org
tools/binman/ftest.py | 2 +- tools/binman/state.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Alper Nebi Yasak alpernebiyasak@gmail.com
I saw the failing build for my series [1]. Looks to me like binman doesn't support crc32 in hash nodes, and turning FIT into a Section simply exposed that. I tried a sloppy fix, see below.
[1] https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/388771
diff --git a/tools/binman/state.py b/tools/binman/state.py index af0a65e841..843aefd28d 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -397,7 +397,7 @@ def CheckAddHashProp(node): if algo.value == 'sha256': size = 32
If I add here:
elif algo.value == 'crc32':size = 8and a crc32 implementation attempt to the next function:
else:
return "Unknown hash algorithm '%s'" % algo
return "Unknown hash algorithm '%s'" % algo.value for n in GetUpdateNodes(hash_node): n.AddEmptyProp('value', size)def CheckSetHashValue(node, get_data_func): hash_node = node.FindNode('hash') if hash_node: algo = hash_node.props.get('algo').value if algo == 'sha256': m = hashlib.sha256() m.update(get_data_func()) data = m.digest()
elif algo == 'crc32':data = zlib.crc32(get_data_func()).to_bytes(8, 'little') for n in GetUpdateNodes(hash_node): n.SetData('value', data)the failing boards start building again. Not sure how correct this is though (especially the endianness). Do we need this and maybe a test with crc32 hash now?
I can get your series in without the final patch (testing at u-boot-dm/testing now).
The thing is that mkimage creates the hashes so we don't want binman doing that too.
My current feeling is that we need a way to tell sections not to add calculated properties for hashes...but just for the FIT section itself, so its children should still add these.
Regards, Simon
Hi Alper,
On Tue, 8 Feb 2022 at 11:54, Alper Nebi Yasak alpernebiyasak@gmail.com wrote:
On 08/02/2022 20:59, Simon Glass wrote:
This shows an internal type at present, rather than the algorithm name. Fix it and update the test to catch this.
Signed-off-by: Simon Glass sjg@chromium.org
tools/binman/ftest.py | 2 +- tools/binman/state.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Alper Nebi Yasak alpernebiyasak@gmail.com
I saw the failing build for my series [1]. Looks to me like binman doesn't support crc32 in hash nodes, and turning FIT into a Section simply exposed that. I tried a sloppy fix, see below.
[1] https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/388771
Applied to u-boot-dm, thanks!
participants (2)
-
Alper Nebi Yasak -
Simon Glass