Fix hash verification
PATCH 1/1 Fix hash verification
From: Nikolay Dimitrov <[email protected]>
Fix issue in parse_verify_sum() which swaps handling of env-var and *address. Move hash_command() argc check earlier.
Cosmetic change on do_hash() variable declaration.
Improved help message for "hash" command.
--- common/cmd_hash.c | 28 +++++++++++++--------------- common/hash.c | 6 ++---- 2 files changed, 15 insertions(+), 19 deletions(-)
common/cmd_hash.c +13 -15
@@ -18,9 +18,9 @@static int do_hash (cmd_tbl_t *cmdtp,int flag,int argc,char *const argv[]) {char *s; -#ifdef CONFIG_HASH_VERIFY int flags =HASH_FLAG_ENV ; +#ifdef CONFIG_HASH_VERIFY if (argc <4 )return CMD_RET_USAGE ;if (!strcmp (argv[1 ],"-v" )) {
@@ -28,8 +28,6 @@ static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) argc--; argv++; } -#else -const int flags =HASH_FLAG_ENV ;#endif /* Move forward to 'algorithm' parameter */ argc--;
@@ -40,19 +38,19 @@ static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }#ifdef CONFIG_HASH_VERIFY -U_BOOT_CMD ( - hash,6 ,1 , do_hash, -"compute hash message digest" , -"algorithm address count [[*]sum_dest]\n" -" - compute message digest [save to env var / *address]\n" -"hash -v algorithm address count [*]sum\n" -" - verify hash of memory area with env var / *address" -); +#define HARGS 6#else +#define HARGS 5 +#endif +U_BOOT_CMD ( - hash,5 ,1 , do_hash, -"compute message digest" , -"algorithm address count [[*]sum_dest]\n" + hash,HARGS ,1 , do_hash, +"compute hash message digest" , +"algorithm address count [[*]hash_dest]\n" " - compute message digest [save to env var / *address]" -); +#ifdef CONFIG_HASH_VERIFY +"\nhash -v algorithm address count [*]hash\n" +" - verify message digest of memory area to immediate value, \n" +" env var or *address" #endif +);
common/hash.c +2 -4
@@ -256,7 +256,7 @@ static int parse_verify_sum(struct hash_algo *algo, char *verify_str, env_var =1 ; } -if (env_var) { +if (!env_var) {ulong addr;void *buf;
@@ -347,7 +347,7 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, {ulong addr, len; -if (argc <2 ) +if ((argc <2 ) || ((flags &HASH_FLAG_VERIFY ) && (argc <3 )))return CMD_RET_USAGE ; addr =simple_strtoul (*argv++,NULL ,16 );
@@ -380,8 +380,6 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag,#else if (0 ) {#endif -if (!argc) -return CMD_RET_USAGE ;if (parse_verify_sum (algo, *argv, vsum, flags &HASH_FLAG_ENV )) { printf("ERROR: %s does not contain a valid "
Signature · 1 line
1.7.10.4
PATCH 1/1 Fix hash verification
Hi Nikolay,
Quoted 8 lines · On 12 December 2014 at 11:01, <[email protected]> wrote:
From: Nikolay Dimitrov <[email protected]>
Fix issue in
parse_verify_sum()which swaps handling of env-var and *address. Movehash_command()argc check earlier.
Cosmetic change ondo_hash()variable declaration.
Improved help message for "hash" command.Signed-off-by Nikolay Dimitrov <[email protected]>
Thanks for this. Main change looks good, a few nits.
Quoted 48 lines
--- common/cmd_hash.c | 28 +++++++++++++--------------- common/hash.c | 6 ++---- 2 files changed, 15 insertions(+), 19 deletions(-)common/cmd_hash.c +5
-12@@ -18,9 +18,9 @@static int do_hash (cmd_tbl_t *cmdtp,int flag,int argc,char *const argv[]) {char *s; -#ifdef CONFIG_HASH_VERIFY int flags =HASH_FLAG_ENV ; +#ifdef CONFIG_HASH_VERIFY if (argc <4 )return CMD_RET_USAGE ;if (!strcmp (argv[1 ],"-v" )) {@@ -28,8 +28,6 @@ static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) argc--; argv++; } -#else -const int flags =HASH_FLAG_ENV ;#endif /* Move forward to 'algorithm' parameter */ argc--;@@ -40,19 +38,19 @@ static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }#ifdef CONFIG_HASH_VERIFY -U_BOOT_CMD ( - hash,6 ,1 , do_hash, -"compute hash message digest" , -"algorithm address count [[*]sum_dest]\n" -" - compute message digest [save to env var / *address]\n" -"hash -v algorithm address count [*]sum\n" -" - verify hash of memory area with env var / *address" -); +#define HARGS 6#else +#define HARGS 5 +#endif + U_BOOT_CMD( - hash, 5, 1, do_hash,
common/cmd_hash.c static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 53
Quoted 10 lines
common/cmd_hash.c +6
-3-"compute message digest" , -"algorithm address count [[*]sum_dest]\n" + hash,HARGS ,1 , do_hash, +"compute hash message digest" , +"algorithm address count [[*]hash_dest]\n" " - compute message digest [save to env var / *address]" -); +#ifdef CONFIG_HASH_VERIFY +"\nhash -v algorithm address count [*]hash\n" +" - verify message digest of memory area to immediate value, \n"
Perhaps " verify message digest of memory area, display result or write to env var or *address"
Quoted 25 lines
common/cmd_hash.c +1
-0#endif +);common/hash.c +2
-2@@ -256,7 +256,7 @@ static int parse_verify_sum(struct hash_algo *algo, char *verify_str, env_var =1 ; } -if (env_var) { +if (!env_var) {ulong addr;void *buf;@@ -347,7 +347,7 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, {ulong addr, len; -if (argc <2 ) +if ((argc <2 ) || ((flags &HASH_FLAG_VERIFY ) && (argc <3 )))return CMD_RET_USAGE ; addr =simple_strtoul (*argv++,NULL ,16 );
common/hash.c int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, · line 384
Quoted 5 lines
common/hash.c +0
-2@@ -380,8 +380,6 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag,#else if (0 ) {#endif -if (!argc) -return CMD_RET_USAGE ;
What does this change achieve?
Quoted 6 lines
common/hash.c +0
-1if (parse_verify_sum (algo, *argv, vsum, flags &HASH_FLAG_ENV )) { printf("ERROR: %s does not contain a valid " --1.7.10.4
Regards,
Simon
PATCH 1/1 Fix hash verification
Hi Simon,
Quoted 67 lines · Simon Glass
Hi Nikolay,
Quoted 8 lines · On 12 December 2014 at 11:01, <[email protected]> wrote:
From: Nikolay Dimitrov <[email protected]>
Fix issue in
parse_verify_sum()which swaps handling of env-var and *address. Movehash_command()argc check earlier.
Cosmetic change ondo_hash()variable declaration.
Improved help message for "hash" command.Signed-off-by Nikolay Dimitrov <[email protected]>Thanks for this. Main change looks good, a few nits.
Quoted 53 lines
--- common/cmd_hash.c | 28 +++++++++++++--------------- common/hash.c | 6 ++---- 2 files changed, 15 insertions(+), 19 deletions(-)common/cmd_hash.c +8
-14@@ -18,9 +18,9 @@static int do_hash (cmd_tbl_t *cmdtp,int flag,int argc,char *const argv[]) {char *s; -#ifdef CONFIG_HASH_VERIFY int flags =HASH_FLAG_ENV ; +#ifdef CONFIG_HASH_VERIFY if (argc <4 )return CMD_RET_USAGE ;if (!strcmp (argv[1 ],"-v" )) {@@ -28,8 +28,6 @@ static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) argc--; argv++; } -#else -const int flags =HASH_FLAG_ENV ;#endif /* Move forward to 'algorithm' parameter */ argc--;@@ -40,19 +38,19 @@ static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }#ifdef CONFIG_HASH_VERIFY -U_BOOT_CMD ( - hash,6 ,1 , do_hash, -"compute hash message digest" , -"algorithm address count [[*]sum_dest]\n" -" - compute message digest [save to env var / *address]\n" -"hash -v algorithm address count [*]sum\n" -" - verify hash of memory area with env var / *address" -); +#define HARGS 6#else +#define HARGS 5 +#endif +U_BOOT_CMD ( - hash,5 ,1 , do_hash, -"compute message digest" , - "algorithm address count [[*]sum_dest]\n" + hash,HARGS ,1 , do_hash, +"compute hash message digest" , + "algorithm address count [[*]hash_dest]\n"
Quoted 8 lines
common/cmd_hash.c static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 53
Quoted 5 lines
common/cmd_hash.c +3
-1" - compute message digest [save to env var / *address]" -); +#ifdef CONFIG_HASH_VERIFY +"\nhash -v algorithm address count [*]hash\n" +" - verify message digest of memory area to immediate value, \n" Perhaps " verify message digest of memory area, display result or write to env var or *address"
I think that the initial description was appropriate, because the "hash" command supports 3 modes of operation (I chose crc32 for shorter examples):
1. Verify hash against immediate value, like this
hash -v crc32 0x20000000 $filesize e9b11acd
2. Verify hash against environment variable, which holds the actual hash value
hash -v crc32 0x20000000 $filesize env_var_name
3, Verify hash against value, placed at specific memory address
hash -v crc32 0x20000000 $filesize *0x30000000
As far as I observed the code and its behavior, the only case when the "hash -v" result was printed was when the verification failed. Please correct me if I'm wrong.
Quoted 23 lines
Quoted 23 lines
common/cmd_hash.c +1
-0#endif +);common/hash.c +2
-2@@ -256,7 +256,7 @@ static int parse_verify_sum(struct hash_algo *algo, char *verify_str, env_var =1 ; } -if (env_var) { +if (!env_var) {ulong addr;void *buf;@@ -347,7 +347,7 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, {ulong addr, len; -if (argc <2 ) +if ((argc <2 ) || ((flags &HASH_FLAG_VERIFY ) && (argc <3 )))return CMD_RET_USAGE ;
Quoted 9 lines
common/hash.c int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, · line 384
Quoted 7 lines
common/hash.c +0
-2addr =simple_strtoul (*argv++,NULL ,16 );@@ -380,8 +380,6 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag,#else if (0 ) {#endif -if (!argc) -return CMD_RET_USAGE ;What does this change achieve?
I moved the verification earlier, because the original implementation first calculated the hash and just then complained about the last missing argument. My personal understanding is that errors should be caught as early as possible, and work should be done only as necessary :D.
Quoted 10 lines
Quoted 7 lines
common/hash.c +0
-1if (parse_verify_sum (algo, *argv, vsum, flags &HASH_FLAG_ENV )) { printf("ERROR: %s does not contain a valid " --1.7.10.4
Regards,
Simon
Kind regards,
Nikolay
PATCH 1/1 Fix hash verification
Hi Simon,
I omitted one clarification, which I think it's important.
Quoted 100 lines · Nikolay Dimitrov
Hi Simon,
Quoted 75 lines · Simon Glass
Hi Nikolay,
Quoted 9 lines · On 12 December 2014 at 11:01, <[email protected]> wrote:
From: Nikolay Dimitrov <[email protected]>
Fix issue in
parse_verify_sum()which swaps handling of env-var and *address.
Movehash_command()argc check earlier.
Cosmetic change ondo_hash()variable declaration. Improved help message for "hash" command.Signed-off-by Nikolay Dimitrov <[email protected]>Thanks for this. Main change looks good, a few nits.
Quoted 60 lines
--- common/cmd_hash.c | 28 +++++++++++++--------------- common/hash.c | 6 ++---- 2 files changed, 15 insertions(+), 19 deletions(-)common/cmd_hash.c +1
-1@@ -18,9 +18,9 @@static int do_hash (cmd_tbl_t *cmdtp,int flag,int argc,char *const argv[]) {char *s; -#ifdef CONFIG_HASH_VERIFY int flags =HASH_FLAG_ENV ; +#ifdef CONFIG_HASH_VERIFY if (argc <4 )return CMD_RET_USAGE ;if (!strcmp(argv[1], "-v")) {common/cmd_hash.c +0
-2@@ -28,8 +28,6 @@ static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) argc--; argv++; } -#else -const int flags =HASH_FLAG_ENV ;#endif /* Move forward to 'algorithm' parameter */ argc--;common/cmd_hash.c +7
-11@@ -40,19 +38,19 @@ static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } #ifdef CONFIG_HASH_VERIFY -U_BOOT_CMD( - hash, 6, 1, do_hash, - "compute hash message digest", - "algorithm address count [[*]sum_dest]\n" - " - compute message digest [save to env var / *address]\n" - "hash -v algorithm address count [*]sum\n" - " - verify hash of memory area with env var / *address" -); +#define HARGS 6 #else +#define HARGS 5 +#endif + U_BOOT_CMD( - hash, 5, 1, do_hash, - "compute message digest", - "algorithm address count [[*]sum_dest]\n" + hash, HARGS, 1, do_hash, + "compute hash message digest", +"algorithm address count [[*]hash_dest]\n" " - compute message digest [save to env var /*address]"
Quoted 8 lines
Quoted 5 lines
common/cmd_hash.c +3
-1-); +#ifdef CONFIG_HASH_VERIFY +"\nhash -v algorithm address count [*]hash\n" + " - verify message digest of memory area toimmediate value, \n"
Perhaps " verify message digest of memory area, display result or write to env var or *address"
I think that the initial description was appropriate, because the "hash" command supports 3 modes of operation (I chose crc32 for shorter examples):
1. Verify hash against immediate value, like this
hash -v crc32
0x20000000$filesizee9b11acd2. Verify hash against environment variable, which holds the actual hash value
hash -v crc32
0x20000000$filesizeenv_var_name
Quoted 7 lines
3, Verify hash against value, placed at specific memory address
hash -v crc32
0x20000000$filesize *0x30000000As far as I observed the code and its behavior, the only case when the "hash -v" result was printed was when the verification failed. Please correct me if I'm wrong.
In all these modes of verification, the last argument is used as the "reference value" to compare against, this is another reason why it would be inappropriate to say "...or write to env var or *address".
Quoted 52 lines
Quoted 26 lines
Quoted 26 lines
#endifcommon/cmd_hash.c +1
-0+);common/hash.c +2
-2@@ -256,7 +256,7 @@ static int parse_verify_sum(struct hash_algo *algo, char *verify_str, env_var =1 ; } -if (env_var) { +if (!env_var) {ulong addr;void *buf;@@ -347,7 +347,7 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, { ulong addr, len; -if (argc <2 ) +if ((argc <2 ) || ((flags &HASH_FLAG_VERIFY ) && (argc <3 )))return CMD_RET_USAGE ;Quoted 10 lines
common/hash.c int hash_command(const char *algo_name, int · line 385
Quoted 8 lines
addr = simple_strtoul(*argv++, NULL, 16);common/hash.c +0
-2@@ -380,8 +380,6 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, #else if (0) {#endif -if (!argc) -return CMD_RET_USAGE ;What does this change achieve?
I moved the verification earlier, because the original implementation first calculated the hash and just then complained about the last missing argument. My personal understanding is that errors should be caught as early as possible, and work should be done only as necessary :D.
Quoted 9 lines
Quoted 8 lines
common/hash.c +0
-0if (parse_verify_sum (algo, *argv, vsum, flags &HASH_FLAG_ENV )) {printf("ERROR: %s does not contain a
valid "
--
1.7.10.4
Quoted 5 lines
Quoted 2 lines
Regards,
SimonKind regards,
Nikolay
Kind regards,
Nikolay
PATCH 1/1 Fix hash verification
Hi Nikolay,
Quoted 116 lines · Nikolay Dimitrov
Hi Simon,
I omitted one clarification, which I think it's important.
Quoted 110 lines · Nikolay Dimitrov
Hi Simon,
Quoted 79 lines · Simon Glass
Hi Nikolay,
Quoted 10 lines · On 12 December 2014 at 11:01, <[email protected]> wrote:
From: Nikolay Dimitrov <[email protected]>
Fix issue in
parse_verify_sum()which swaps handling of env-var and *address.
Movehash_command()argc check earlier.
Cosmetic change ondo_hash()variable declaration. Improved help message for "hash" command.Signed-off-by Nikolay Dimitrov <[email protected]>Thanks for this. Main change looks good, a few nits.
Quoted 61 lines
--- common/cmd_hash.c | 28 +++++++++++++--------------- common/hash.c | 6 ++---- 2 files changed, 15 insertions(+), 19 deletions(-)common/cmd_hash.c +1
-1@@ -18,9 +18,9 @@static int do_hash (cmd_tbl_t *cmdtp,int flag,int argc,char *const argv[]) {char *s; -#ifdef CONFIG_HASH_VERIFY int flags =HASH_FLAG_ENV ; +#ifdef CONFIG_HASH_VERIFY if (argc <4 )return CMD_RET_USAGE ;if (!strcmp(argv[1], "-v")) {common/cmd_hash.c +0
-2@@ -28,8 +28,6 @@ static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) argc--; argv++; } -#else -const int flags =HASH_FLAG_ENV ;#endif /* Move forward to 'algorithm' parameter */ argc--;common/cmd_hash.c +7
-11@@ -40,19 +38,19 @@ static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } #ifdef CONFIG_HASH_VERIFY -U_BOOT_CMD( - hash, 6, 1, do_hash, - "compute hash message digest", - "algorithm address count [[*]sum_dest]\n" - " - compute message digest [save to env var / *address]\n" - "hash -v algorithm address count [*]sum\n" - " - verify hash of memory area with env var / *address" -); +#define HARGS 6 #else +#define HARGS 5 +#endif + U_BOOT_CMD( - hash, 5, 1, do_hash, - "compute message digest", - "algorithm address count [[*]sum_dest]\n" + hash, HARGS, 1, do_hash, + "compute hash message digest", +"algorithm address count [[*]hash_dest]\n" " - compute message digest [save to env var /*address]"
-);Quoted 8 lines
Quoted 4 lines
common/cmd_hash.c +3
-0+#ifdef CONFIG_HASH_VERIFY +"\nhash -v algorithm address count [*]hash\n" + " - verify message digest of memory area toimmediate value, \n"
Perhaps " verify message digest of memory area, display result or write to env var or *address"
I think that the initial description was appropriate, because the "hash" command supports 3 modes of operation (I chose crc32 for shorter examples):
1. Verify hash against immediate value, like this
hash -v crc32
0x20000000$filesizee9b11acd2. Verify hash against environment variable, which holds the actual hash value
hash -v crc32
0x20000000$filesizeenv_var_name3, Verify hash against value, placed at specific memory address
hash -v crc32
0x20000000$filesize *0x30000000
Quoted 62 lines
Quoted 3 lines
As far as I observed the code and its behavior, the only case when the "hash -v" result was printed was when the verification failed. Please correct me if I'm wrong.
In all these modes of verification, the last argument is used as the "reference value" to compare against, this is another reason why it would be inappropriate to say "...or write to env var or *address".
Quoted 52 lines
Quoted 26 lines
Quoted 26 lines
#endifcommon/cmd_hash.c +1
-0+);common/hash.c +2
-2@@ -256,7 +256,7 @@ static int parse_verify_sum(struct hash_algo *algo, char *verify_str, env_var =1 ; } -if (env_var) { +if (!env_var) {ulong addr;void *buf;@@ -347,7 +347,7 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, { ulong addr, len; -if (argc <2 ) +if ((argc <2 ) || ((flags &HASH_FLAG_VERIFY ) && (argc <3 )))return CMD_RET_USAGE ;Quoted 11 lines
common/hash.c int hash_command(const char *algo_name, int · line 385
Quoted 8 lines
addr = simple_strtoul(*argv++, NULL, 16);common/hash.c +0
-2@@ -380,8 +380,6 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, #else if (0) {#endif -if (!argc) -return CMD_RET_USAGE ;What does this change achieve?
I moved the verification earlier, because the original implementation first calculated the hash and just then complained about the last missing argument. My personal understanding is that errors should be caught as early as possible, and work should be done only as necessary :D.
Quoted 7 lines
Quoted 7 lines
common/hash.c +0
-0if (parse_verify_sum (algo, *argv, vsum, flags &HASH_FLAG_ENV )) {printf("ERROR: %s does not contain a
valid "
--
1.7.10.4
Thanks for the explanation, it looks right to me.
Regards,
Simon
Quoted 9 lines · Nikolay Dimitrov
From: Nikolay Dimitrov <[email protected]>
Fix issue in
parse_verify_sum()which swaps handling of env-var and *address. Movehash_command()argc check earlier.
Cosmetic change ondo_hash()variable declaration.
Improved help message for "hash" command.Signed-off-by Nikolay Dimitrov <[email protected]>Reviewed-by Simon Glass <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141229/3af43ba5/attachment.pgp>