[U-Boot] [PATCH v2] Prevent a buffer overflow in mkimage when signing with SHA256
Due to the FIT_MAX_HASH_LEN constant not having been updated to support SHA256 signatures one will always see a buffer overflow in fit_image_process_hash when signing images that use this larger hash. This is exposed by vboot_test.sh.
Signed-off-by: Michael van der Westhuizen michael@smart-africa.com --- Changes in v2: * Use the HASH_MAX_DIGEST_SIZE constant from hash.h for the FIT_MAX_HASH_LEN. * Hide use of struct lmb behind USE_HOSTCC being undefined.
include/image.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/image.h b/include/image.h index 1886168..cbbdf26 100644 --- a/include/image.h +++ b/include/image.h @@ -45,6 +45,8 @@ struct lmb; #endif /* USE_HOSTCC */
#if defined(CONFIG_FIT) +#include <command.h> +#include <hash.h> #include <libfdt.h> #include <fdt_support.h> # ifdef CONFIG_SPL_BUILD @@ -328,7 +330,7 @@ typedef struct bootm_headers { #define BOOTM_STATE_OS_GO (0x00000400) int state;
-#ifdef CONFIG_LMB +#if defined(CONFIG_LMB) && !defined(USE_HOSTCC) struct lmb lmb; /* for memory mgmt */ #endif } bootm_headers_t; @@ -703,7 +705,7 @@ int bootz_setup(ulong image, ulong *start, ulong *end); #define FIT_FDT_PROP "fdt" #define FIT_DEFAULT_PROP "default"
-#define FIT_MAX_HASH_LEN 20 /* max(crc32_len(4), sha1_len(20)) */ +#define FIT_MAX_HASH_LEN HASH_MAX_DIGEST_SIZE
/* cmdline argument format parsing */ int fit_parse_conf(const char *spec, ulong addr_curr,
Hi Michael,
On 26 May 2014 07:09, Michael van der Westhuizen michael@smart-africa.com wrote:
Due to the FIT_MAX_HASH_LEN constant not having been updated to support SHA256 signatures one will always see a buffer overflow in fit_image_process_hash when signing images that use this larger hash. This is exposed by vboot_test.sh.
Signed-off-by: Michael van der Westhuizen michael@smart-africa.com
Changes in v2:
- Use the HASH_MAX_DIGEST_SIZE constant from hash.h for the FIT_MAX_HASH_LEN.
- Hide use of struct lmb behind USE_HOSTCC being undefined.
This seems to have whitespace problems, or it could be your mailer. There should be tabs in there somewhere. Did you use patman to check/send?
Regards, Simon
Hi Simon,
That's very odd. I'll regenerate the patch and resend.
I'm using git send-email, so things should not be getting mangled.
Michael
On 30 May 2014, at 8:42 PM, Simon Glass sjg@chromium.org wrote:
Hi Michael,
On 26 May 2014 07:09, Michael van der Westhuizen michael@smart-africa.com wrote:
Due to the FIT_MAX_HASH_LEN constant not having been updated to support SHA256 signatures one will always see a buffer overflow in fit_image_process_hash when signing images that use this larger hash. This is exposed by vboot_test.sh.
Signed-off-by: Michael van der Westhuizen michael@smart-africa.com
Changes in v2:
- Use the HASH_MAX_DIGEST_SIZE constant from hash.h for the FIT_MAX_HASH_LEN.
- Hide use of struct lmb behind USE_HOSTCC being undefined.
This seems to have whitespace problems, or it could be your mailer. There should be tabs in there somewhere. Did you use patman to check/send?
Regards, Simon
From: Michael van der Westhuizen michael@smart-africa.com
Due to the FIT_MAX_HASH_LEN constant not having been updated to support SHA256 signatures one will always see a buffer overflow in fit_image_process_hash when signing images that use this larger hash. This is exposed by vboot_test.sh.
Signed-off-by: Michael van der Westhuizen michael@smart-africa.com --- Changes in v3: * Regenerate to correct mailer damage. Changes in v2: * Use the HASH_MAX_DIGEST_SIZE constant from hash.h for the FIT_MAX_HASH_LEN. * Hide use of struct lmb behind USE_HOSTCC being undefined.
include/image.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/image.h b/include/image.h index 1886168..cbbdf26 100644 --- a/include/image.h +++ b/include/image.h @@ -45,6 +45,8 @@ struct lmb; #endif /* USE_HOSTCC */
#if defined(CONFIG_FIT) +#include <command.h> +#include <hash.h> #include <libfdt.h> #include <fdt_support.h> # ifdef CONFIG_SPL_BUILD @@ -328,7 +330,7 @@ typedef struct bootm_headers { #define BOOTM_STATE_OS_GO (0x00000400) int state;
-#ifdef CONFIG_LMB +#if defined(CONFIG_LMB) && !defined(USE_HOSTCC) struct lmb lmb; /* for memory mgmt */ #endif } bootm_headers_t; @@ -703,7 +705,7 @@ int bootz_setup(ulong image, ulong *start, ulong *end); #define FIT_FDT_PROP "fdt" #define FIT_DEFAULT_PROP "default"
-#define FIT_MAX_HASH_LEN 20 /* max(crc32_len(4), sha1_len(20)) */ +#define FIT_MAX_HASH_LEN HASH_MAX_DIGEST_SIZE
/* cmdline argument format parsing */ int fit_parse_conf(const char *spec, ulong addr_curr,
On 30 May 2014 12:59, michael@smart-africa.com wrote:
From: Michael van der Westhuizen michael@smart-africa.com
Due to the FIT_MAX_HASH_LEN constant not having been updated to support SHA256 signatures one will always see a buffer overflow in fit_image_process_hash when signing images that use this larger hash. This is exposed by vboot_test.sh.
Signed-off-by: Michael van der Westhuizen michael@smart-africa.com
Acked-by: Simon Glass sjg@chromium.org
Regards, Simon
On Fri, May 30, 2014 at 08:59:00PM +0200, Michael van der Westhuizen wrote:
From: Michael van der Westhuizen michael@smart-africa.com
Due to the FIT_MAX_HASH_LEN constant not having been updated to support SHA256 signatures one will always see a buffer overflow in fit_image_process_hash when signing images that use this larger hash. This is exposed by vboot_test.sh.
Signed-off-by: Michael van der Westhuizen michael@smart-africa.com Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot/master (with a rework to hash.h to avoid breaking various platforms when host tools start adding command.h) , thanks!
Hi Tom,
On 5 June 2014 16:50, Tom Rini trini@ti.com wrote:
On Fri, May 30, 2014 at 08:59:00PM +0200, Michael van der Westhuizen wrote:
From: Michael van der Westhuizen michael@smart-africa.com
Due to the FIT_MAX_HASH_LEN constant not having been updated to support SHA256 signatures one will always see a buffer overflow in fit_image_process_hash when signing images that use this larger hash. This is exposed by vboot_test.sh.
Signed-off-by: Michael van der Westhuizen michael@smart-africa.com Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot/master (with a rework to hash.h to avoid breaking various platforms when host tools start adding command.h) , thanks!
Thanks - I was about to mention that!
Regards, Simon
participants (4)
-
Michael van der Westhuizen -
michael＠smart-africa.com
-
Simon Glass -
Tom Rini