From c8d64c54c9397f19555cb23b87c5170595ed5e7a Mon Sep 17 00:00:00 2001 From: Antonio Nino Diaz Date: Fri, 13 Jan 2017 15:03:07 +0000 Subject: [PATCH 1/2] Fix declarations of cache maintenance functions Fix the parameter type of the maintenance functions of data cache. Add missing declarations for AArch32 versions of dcsw_op_louis and dcsw_op_all to match the AAch64 ones. Change-Id: I4226e8ea4f8b2b5bc2972992c83de659ee0da52c --- include/lib/aarch32/arch_helpers.h | 7 +++++-- include/lib/aarch64/arch_helpers.h | 14 ++++++++------ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/include/lib/aarch32/arch_helpers.h b/include/lib/aarch32/arch_helpers.h index 7955d62e9..3a82a7b34 100644 --- a/include/lib/aarch32/arch_helpers.h +++ b/include/lib/aarch32/arch_helpers.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, ARM Limited and Contributors. All rights reserved. + * Copyright (c) 2016-2017, ARM Limited and Contributors. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: @@ -33,7 +33,7 @@ #include /* for additional register definitions */ #include -#include +#include /********************************************************************** * Macros which create inline functions to read or write CPU system @@ -187,6 +187,9 @@ void flush_dcache_range(uintptr_t addr, size_t size); void clean_dcache_range(uintptr_t addr, size_t size); void inv_dcache_range(uintptr_t addr, size_t size); +void dcsw_op_louis(u_register_t op_type); +void dcsw_op_all(u_register_t op_type); + void disable_mmu_secure(void); void disable_mmu_icache_secure(void); diff --git a/include/lib/aarch64/arch_helpers.h b/include/lib/aarch64/arch_helpers.h index aa2620312..d70c9aee1 100644 --- a/include/lib/aarch64/arch_helpers.h +++ b/include/lib/aarch64/arch_helpers.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013-2015, ARM Limited and Contributors. All rights reserved. + * Copyright (c) 2013-2017, ARM Limited and Contributors. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: @@ -34,6 +34,7 @@ #include /* for additional register definitions */ #include /* For __dead2 */ #include +#include /********************************************************************** * Macros which create inline functions to read or write CPU system @@ -143,11 +144,12 @@ DEFINE_SYSOP_TYPE_PARAM_FUNC(at, s12e1w) DEFINE_SYSOP_TYPE_PARAM_FUNC(at, s12e0r) DEFINE_SYSOP_TYPE_PARAM_FUNC(at, s12e0w) -void flush_dcache_range(uint64_t, uint64_t); -void clean_dcache_range(uint64_t, uint64_t); -void inv_dcache_range(uint64_t, uint64_t); -void dcsw_op_louis(uint32_t); -void dcsw_op_all(uint32_t); +void flush_dcache_range(uintptr_t addr, size_t size); +void clean_dcache_range(uintptr_t addr, size_t size); +void inv_dcache_range(uintptr_t addr, size_t size); + +void dcsw_op_louis(u_register_t op_type); +void dcsw_op_all(u_register_t op_type); void disable_mmu_el3(void); void disable_mmu_icache_el3(void); From 51c5e1a29fad07ad2758f44db868c1a4cdcd4e32 Mon Sep 17 00:00:00 2001 From: Antonio Nino Diaz Date: Fri, 13 Jan 2017 15:03:19 +0000 Subject: [PATCH 2/2] Clear static variables in X509 parser on error In mbedtls_x509_parser.c there are some static arrays that are filled during the integrity check and then read whenever an authentication parameter is requested. However, they aren't cleared in case of an integrity check failure, which can be problematic from a security point of view. This patch clears these arrays in the case of failure. Change-Id: I9d48f5bc71fa13e5a75d6c45b5e34796ef13aaa2 Signed-off-by: Antonio Nino Diaz --- drivers/auth/mbedtls/mbedtls_x509_parser.c | 39 ++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/drivers/auth/mbedtls/mbedtls_x509_parser.c b/drivers/auth/mbedtls/mbedtls_x509_parser.c index 1a6a9a75e..73da9d1e7 100644 --- a/drivers/auth/mbedtls/mbedtls_x509_parser.c +++ b/drivers/auth/mbedtls/mbedtls_x509_parser.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved. + * Copyright (c) 2015-2017, ARM Limited and Contributors. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: @@ -36,6 +36,7 @@ * extensions field, such as an image hash or a public key. */ +#include #include #include #include @@ -63,6 +64,26 @@ static mbedtls_asn1_buf pk; static mbedtls_asn1_buf sig_alg; static mbedtls_asn1_buf signature; +/* + * Clear all static temporary variables. + */ +static void clear_temp_vars(void) +{ +#define ZERO_AND_CLEAN(x) \ + do { \ + memset(&x, 0, sizeof(x)); \ + clean_dcache_range((uintptr_t)&x, sizeof(x)); \ + } while (0); + + ZERO_AND_CLEAN(tbs) + ZERO_AND_CLEAN(v3_ext); + ZERO_AND_CLEAN(pk); + ZERO_AND_CLEAN(sig_alg); + ZERO_AND_CLEAN(signature); + +#undef ZERO_AND_CLEAN +} + /* * Get X509v3 extension * @@ -134,7 +155,12 @@ static int get_ext(const char *oid, void **ext, unsigned int *ext_len) /* * Check the integrity of the certificate ASN.1 structure. + * * Extract the relevant data that will be used later during authentication. + * + * This function doesn't clear the static variables located on the top of this + * file in case of an error. It is only called from check_integrity(), which + * performs the cleanup if necessary. */ static int cert_parse(void *img, unsigned int img_len) { @@ -398,9 +424,18 @@ static void init(void) mbedtls_init(); } +/* + * Wrapper for cert_parse() that clears the static variables used by it in case + * of an error. + */ static int check_integrity(void *img, unsigned int img_len) { - return cert_parse(img, img_len); + int rc = cert_parse(img, img_len); + + if (rc != IMG_PARSER_OK) + clear_temp_vars(); + + return rc; } /*