[PATCH 0/4] Clean-up patch set for MbedTLS integration

This patch set is picked from the previously posted serie: "[RFC] Integrate MbedTLS v3.6 LTS with U-Boot"
They are not directly related to MbedTLS integration, but the prerequisite for a few clean-up, refactoring and minor fixes.
Raymond Mao (4): image: remove redundant hash includes efi_loader: remove redundant hash includes md5: Use typedef for MD5 context arm: EFI linker script text section alignment
arch/arm/lib/elf_aarch64_efi.lds | 1 + boot/image-fit.c | 4 ---- boot/image.c | 2 -- drivers/crypto/hash/hash_sw.c | 8 ++++---- include/u-boot/md5.h | 10 +++++----- lib/efi_loader/efi_signature.c | 1 - lib/efi_loader/efi_tcg2.c | 3 --- lib/md5.c | 10 +++++----- 8 files changed, 15 insertions(+), 24 deletions(-)

Remove the redundant includes of u-boot/md5.h, u-boot/sha1.h, u-boot/sha256.h and u-boot/sha512.h
Signed-off-by: Raymond Mao raymond.mao@linaro.org --- boot/image-fit.c | 4 ---- boot/image.c | 2 -- 2 files changed, 6 deletions(-)
diff --git a/boot/image-fit.c b/boot/image-fit.c index 89e377563ce..1efc39f4408 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -38,10 +38,6 @@ DECLARE_GLOBAL_DATA_PTR; #include <image.h> #include <bootstage.h> #include <u-boot/crc.h> -#include <u-boot/md5.h> -#include <u-boot/sha1.h> -#include <u-boot/sha256.h> -#include <u-boot/sha512.h>
/*****************************************************************************/ /* New uImage format routines */ diff --git a/boot/image.c b/boot/image.c index 073931cd7a3..e57d6eae52d 100644 --- a/boot/image.c +++ b/boot/image.c @@ -26,8 +26,6 @@ #endif
#include <asm/global_data.h> -#include <u-boot/md5.h> -#include <u-boot/sha1.h> #include <linux/errno.h> #include <asm/io.h>

On Thu, May 09, 2024 at 07:37:38AM -0700, Raymond Mao wrote:
Remove the redundant includes of u-boot/md5.h, u-boot/sha1.h, u-boot/sha256.h and u-boot/sha512.h
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Reviewed-by: Tom Rini trini@konsulko.com

On Thu, 9 May 2024 at 17:53, Tom Rini trini@konsulko.com wrote:
On Thu, May 09, 2024 at 07:37:38AM -0700, Raymond Mao wrote:
Remove the redundant includes of u-boot/md5.h, u-boot/sha1.h, u-boot/sha256.h and u-boot/sha512.h
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Reviewed-by: Tom Rini trini@konsulko.com
-- Tom
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org

Remove the redundant includes of u-boot/sha1.h, u-boot/sha256.h and u-boot/sha512.h
Signed-off-by: Raymond Mao raymond.mao@linaro.org --- lib/efi_loader/efi_signature.c | 1 - lib/efi_loader/efi_tcg2.c | 3 --- 2 files changed, 4 deletions(-)
diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c index f338e732759..184eac8cddb 100644 --- a/lib/efi_loader/efi_signature.c +++ b/lib/efi_loader/efi_signature.c @@ -17,7 +17,6 @@ #include <linux/oid_registry.h> #include <u-boot/hash-checksum.h> #include <u-boot/rsa.h> -#include <u-boot/sha256.h>
const efi_guid_t efi_guid_sha256 = EFI_CERT_SHA256_GUID; const efi_guid_t efi_guid_cert_rsa2048 = EFI_CERT_RSA2048_GUID; diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index b07e0099c27..ac056dcfc55 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -19,9 +19,6 @@ #include <tpm-v2.h> #include <tpm_api.h> #include <u-boot/hash-checksum.h> -#include <u-boot/sha1.h> -#include <u-boot/sha256.h> -#include <u-boot/sha512.h> #include <linux/unaligned/be_byteshift.h> #include <linux/unaligned/le_byteshift.h> #include <linux/unaligned/generic.h>

On Thu, May 09, 2024 at 07:37:39AM -0700, Raymond Mao wrote:
Remove the redundant includes of u-boot/sha1.h, u-boot/sha256.h and u-boot/sha512.h
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Reviewed-by: Tom Rini trini@konsulko.com

On Thu, 9 May 2024 at 17:38, Raymond Mao raymond.mao@linaro.org wrote:
Remove the redundant includes of u-boot/sha1.h, u-boot/sha256.h and u-boot/sha512.h
Signed-off-by: Raymond Mao raymond.mao@linaro.org
lib/efi_loader/efi_signature.c | 1 - lib/efi_loader/efi_tcg2.c | 3 --- 2 files changed, 4 deletions(-)
diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c index f338e732759..184eac8cddb 100644 --- a/lib/efi_loader/efi_signature.c +++ b/lib/efi_loader/efi_signature.c @@ -17,7 +17,6 @@ #include <linux/oid_registry.h> #include <u-boot/hash-checksum.h> #include <u-boot/rsa.h> -#include <u-boot/sha256.h>
const efi_guid_t efi_guid_sha256 = EFI_CERT_SHA256_GUID; const efi_guid_t efi_guid_cert_rsa2048 = EFI_CERT_RSA2048_GUID; diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index b07e0099c27..ac056dcfc55 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -19,9 +19,6 @@ #include <tpm-v2.h> #include <tpm_api.h> #include <u-boot/hash-checksum.h> -#include <u-boot/sha1.h> -#include <u-boot/sha256.h> -#include <u-boot/sha512.h> #include <linux/unaligned/be_byteshift.h> #include <linux/unaligned/le_byteshift.h>
#include <linux/unaligned/generic.h>
2.25.1
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org

Use of typedef is beneficial for porting with other crypto libs without changing the API callers. Secondly, it is for the code consistency with other digest libs. SHA1, SHA256 and SHA512 are all using typedef for their context.
Signed-off-by: Raymond Mao raymond.mao@linaro.org --- drivers/crypto/hash/hash_sw.c | 8 ++++---- include/u-boot/md5.h | 10 +++++----- lib/md5.c | 10 +++++----- 3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/crypto/hash/hash_sw.c b/drivers/crypto/hash/hash_sw.c index d8065d68ea4..a5033677930 100644 --- a/drivers/crypto/hash/hash_sw.c +++ b/drivers/crypto/hash/hash_sw.c @@ -51,17 +51,17 @@ static void hash_finish_crc32(void *ctx, void *obuf) /* MD5 */ static void hash_init_md5(void *ctx) { - MD5Init((struct MD5Context *)ctx); + MD5Init((MD5Context *)ctx); }
static void hash_update_md5(void *ctx, const void *ibuf, uint32_t ilen) { - MD5Update((struct MD5Context *)ctx, ibuf, ilen); + MD5Update((MD5Context *)ctx, ibuf, ilen); }
static void hash_finish_md5(void *ctx, void *obuf) { - MD5Final(obuf, (struct MD5Context *)ctx); + MD5Final(obuf, (MD5Context *)ctx); }
/* SHA1 */ @@ -159,7 +159,7 @@ static struct sw_hash_impl sw_hash_impl[HASH_ALGO_NUM] = { .init = hash_init_md5, .update = hash_update_md5, .finish = hash_finish_md5, - .ctx_alloc_sz = sizeof(struct MD5Context), + .ctx_alloc_sz = sizeof(MD5Context), },
[HASH_ALGO_SHA1] = { diff --git a/include/u-boot/md5.h b/include/u-boot/md5.h index d61364c0ae3..c465925ea8d 100644 --- a/include/u-boot/md5.h +++ b/include/u-boot/md5.h @@ -10,18 +10,18 @@
#define MD5_SUM_LEN 16
-struct MD5Context { +typedef struct MD5Context { __u32 buf[4]; __u32 bits[2]; union { unsigned char in[64]; __u32 in32[16]; }; -}; +} MD5Context;
-void MD5Init(struct MD5Context *ctx); -void MD5Update(struct MD5Context *ctx, unsigned char const *buf, unsigned len); -void MD5Final(unsigned char digest[16], struct MD5Context *ctx); +void MD5Init(MD5Context *ctx); +void MD5Update(MD5Context *ctx, unsigned char const *buf, unsigned int len); +void MD5Final(unsigned char digest[16], MD5Context *ctx);
/* * Calculate and store in 'output' the MD5 digest of 'len' bytes at diff --git a/lib/md5.c b/lib/md5.c index faf3f78ab1e..34343cf8e23 100644 --- a/lib/md5.c +++ b/lib/md5.c @@ -55,7 +55,7 @@ byteReverse(unsigned char *buf, unsigned longs) * initialization constants. */ void -MD5Init(struct MD5Context *ctx) +MD5Init(MD5Context *ctx) { ctx->buf[0] = 0x67452301; ctx->buf[1] = 0xefcdab89; @@ -71,7 +71,7 @@ MD5Init(struct MD5Context *ctx) * of bytes. */ void -MD5Update(struct MD5Context *ctx, unsigned char const *buf, unsigned len) +MD5Update(MD5Context *ctx, unsigned char const *buf, unsigned int len) { register __u32 t;
@@ -120,7 +120,7 @@ MD5Update(struct MD5Context *ctx, unsigned char const *buf, unsigned len) * 1 0* (64-bit count of bits processed, MSB-first) */ void -MD5Final(unsigned char digest[16], struct MD5Context *ctx) +MD5Final(unsigned char digest[16], MD5Context *ctx) { unsigned int count; unsigned char *p; @@ -269,7 +269,7 @@ MD5Transform(__u32 buf[4], __u32 const in[16]) void md5 (unsigned char *input, int len, unsigned char output[16]) { - struct MD5Context context; + MD5Context context;
MD5Init(&context); MD5Update(&context, input, len); @@ -286,7 +286,7 @@ void md5_wd(const unsigned char *input, unsigned int len, unsigned char output[16], unsigned int chunk_sz) { - struct MD5Context context; + MD5Context context; #if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) const unsigned char *end, *curr; int chunk;

On Thu, May 09, 2024 at 07:37:40AM -0700, Raymond Mao wrote:
Use of typedef is beneficial for porting with other crypto libs without changing the API callers. Secondly, it is for the code consistency with other digest libs. SHA1, SHA256 and SHA512 are all using typedef for their context.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Reviewed-by: Tom Rini trini@konsulko.com

On Thu, 9 May 2024 at 17:38, Raymond Mao raymond.mao@linaro.org wrote:
Use of typedef is beneficial for porting with other crypto libs without changing the API callers. Secondly, it is for the code consistency with other digest libs. SHA1, SHA256 and SHA512 are all using typedef for their context.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
drivers/crypto/hash/hash_sw.c | 8 ++++---- include/u-boot/md5.h | 10 +++++----- lib/md5.c | 10 +++++----- 3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/crypto/hash/hash_sw.c b/drivers/crypto/hash/hash_sw.c index d8065d68ea4..a5033677930 100644 --- a/drivers/crypto/hash/hash_sw.c +++ b/drivers/crypto/hash/hash_sw.c @@ -51,17 +51,17 @@ static void hash_finish_crc32(void *ctx, void *obuf) /* MD5 */ static void hash_init_md5(void *ctx) {
MD5Init((struct MD5Context *)ctx);
MD5Init((MD5Context *)ctx);
}
static void hash_update_md5(void *ctx, const void *ibuf, uint32_t ilen) {
MD5Update((struct MD5Context *)ctx, ibuf, ilen);
MD5Update((MD5Context *)ctx, ibuf, ilen);
}
static void hash_finish_md5(void *ctx, void *obuf) {
MD5Final(obuf, (struct MD5Context *)ctx);
MD5Final(obuf, (MD5Context *)ctx);
}
/* SHA1 */ @@ -159,7 +159,7 @@ static struct sw_hash_impl sw_hash_impl[HASH_ALGO_NUM] = { .init = hash_init_md5, .update = hash_update_md5, .finish = hash_finish_md5,
.ctx_alloc_sz = sizeof(struct MD5Context),
.ctx_alloc_sz = sizeof(MD5Context), }, [HASH_ALGO_SHA1] = {
diff --git a/include/u-boot/md5.h b/include/u-boot/md5.h index d61364c0ae3..c465925ea8d 100644 --- a/include/u-boot/md5.h +++ b/include/u-boot/md5.h @@ -10,18 +10,18 @@
#define MD5_SUM_LEN 16
-struct MD5Context { +typedef struct MD5Context { __u32 buf[4]; __u32 bits[2]; union { unsigned char in[64]; __u32 in32[16]; }; -}; +} MD5Context;
-void MD5Init(struct MD5Context *ctx); -void MD5Update(struct MD5Context *ctx, unsigned char const *buf, unsigned len); -void MD5Final(unsigned char digest[16], struct MD5Context *ctx); +void MD5Init(MD5Context *ctx); +void MD5Update(MD5Context *ctx, unsigned char const *buf, unsigned int len); +void MD5Final(unsigned char digest[16], MD5Context *ctx);
/*
- Calculate and store in 'output' the MD5 digest of 'len' bytes at
diff --git a/lib/md5.c b/lib/md5.c index faf3f78ab1e..34343cf8e23 100644 --- a/lib/md5.c +++ b/lib/md5.c @@ -55,7 +55,7 @@ byteReverse(unsigned char *buf, unsigned longs)
- initialization constants.
*/ void -MD5Init(struct MD5Context *ctx) +MD5Init(MD5Context *ctx) { ctx->buf[0] = 0x67452301; ctx->buf[1] = 0xefcdab89; @@ -71,7 +71,7 @@ MD5Init(struct MD5Context *ctx)
- of bytes.
*/ void -MD5Update(struct MD5Context *ctx, unsigned char const *buf, unsigned len) +MD5Update(MD5Context *ctx, unsigned char const *buf, unsigned int len) { register __u32 t;
@@ -120,7 +120,7 @@ MD5Update(struct MD5Context *ctx, unsigned char const *buf, unsigned len)
- 1 0* (64-bit count of bits processed, MSB-first)
*/ void -MD5Final(unsigned char digest[16], struct MD5Context *ctx) +MD5Final(unsigned char digest[16], MD5Context *ctx) { unsigned int count; unsigned char *p; @@ -269,7 +269,7 @@ MD5Transform(__u32 buf[4], __u32 const in[16]) void md5 (unsigned char *input, int len, unsigned char output[16]) {
struct MD5Context context;
MD5Context context; MD5Init(&context); MD5Update(&context, input, len);
@@ -286,7 +286,7 @@ void md5_wd(const unsigned char *input, unsigned int len, unsigned char output[16], unsigned int chunk_sz) {
struct MD5Context context;
MD5Context context;
#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) const unsigned char *end, *curr; int chunk; -- 2.25.1
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org

Add text section alignment to fix sbsign signing warning 'gaps in the section table may result in different checksums' which causes a failure of efi_image_verify_diges()
Signed-off-by: Raymond Mao raymond.mao@linaro.org --- arch/arm/lib/elf_aarch64_efi.lds | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/lib/elf_aarch64_efi.lds b/arch/arm/lib/elf_aarch64_efi.lds index 5dd98091698..bffd9a0447a 100644 --- a/arch/arm/lib/elf_aarch64_efi.lds +++ b/arch/arm/lib/elf_aarch64_efi.lds @@ -28,6 +28,7 @@ SECTIONS *(.dynamic); . = ALIGN(512); } + . = ALIGN(4096); .rela.dyn : { *(.rela.dyn) } .rela.plt : { *(.rela.plt) } .rela.got : { *(.rela.got) }

Hi Raymond,
Try not to post the same patches without the fixes that were asked [0], at least not without an explanation.
On Thu, 9 May 2024 at 17:38, Raymond Mao raymond.mao@linaro.org wrote:
Add text section alignment to fix sbsign signing warning 'gaps in the section table may result in different checksums' which causes a failure of efi_image_verify_diges()
Signed-off-by: Raymond Mao raymond.mao@linaro.org
arch/arm/lib/elf_aarch64_efi.lds | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/lib/elf_aarch64_efi.lds b/arch/arm/lib/elf_aarch64_efi.lds index 5dd98091698..bffd9a0447a 100644 --- a/arch/arm/lib/elf_aarch64_efi.lds +++ b/arch/arm/lib/elf_aarch64_efi.lds @@ -28,6 +28,7 @@ SECTIONS *(.dynamic); . = ALIGN(512); }
. = ALIGN(4096);
This is not what you want. There's already an ALIGN a few lines below and you must include the symbols for _etext and _text_size. You have to move the rela.* sections after the alignment. IIRC those were inspired by [1], so you can look for guidance there. On top of that riscv will need similar fixes (and any other architecture that uses those)
.rela.dyn : { *(.rela.dyn) } .rela.plt : { *(.rela.plt) } .rela.got : { *(.rela.got) }
-- 2.25.1
[0] https://lore.kernel.org/u-boot/CAC_iWjK+14L6mEi8GQN-McF-SChvhnjJ79nE1tWLV+kg... [1] https://git.code.sf.net/p/gnu-efi/code
/Ilias

Hi Ilias,
On Thu, 9 May 2024 at 14:15, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Raymond,
Try not to post the same patches without the fixes that were asked [0], at least not without an explanation.
On Thu, 9 May 2024 at 17:38, Raymond Mao raymond.mao@linaro.org wrote:
Add text section alignment to fix sbsign signing warning 'gaps in the section table may result in different checksums' which causes a failure of efi_image_verify_diges()
Signed-off-by: Raymond Mao raymond.mao@linaro.org
arch/arm/lib/elf_aarch64_efi.lds | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/lib/elf_aarch64_efi.lds
b/arch/arm/lib/elf_aarch64_efi.lds
index 5dd98091698..bffd9a0447a 100644 --- a/arch/arm/lib/elf_aarch64_efi.lds +++ b/arch/arm/lib/elf_aarch64_efi.lds @@ -28,6 +28,7 @@ SECTIONS *(.dynamic); . = ALIGN(512); }
. = ALIGN(4096);
This is not what you want. There's already an ALIGN a few lines below and you must include the symbols for _etext and _text_size. You have to move the rela.* sections after the alignment. IIRC those were inspired by [1], so you can look for guidance there. On top of that riscv will need similar fixes (and any other architecture that uses those)
In practice, I have to add ALIGN(4096) for both `.rela.*` and `.data`,
otherwise it always prompts warning when signing the EFI image as below and finally leads a failure when calling function `efi_image_verify_digest()`.
``` warning: gap in section table: .text : 0x00001000 - 0x00001c00, .data : 0x00002000 - 0x00002200, gaps in the section table may result in different checksums warning: data remaining[7680 vs 12720]: gaps between PE/COFF sections? ```
I am not sure if RISC-V is working, never tried to build with that. But arm32, x86 should be fine - they don't have the `.rela.*` sections and text and data sections are well aligned.
.rela.dyn : { *(.rela.dyn) } .rela.plt : { *(.rela.plt) } .rela.got : { *(.rela.got) }
-- 2.25.1
[0] https://lore.kernel.org/u-boot/CAC_iWjK+14L6mEi8GQN-McF-SChvhnjJ79nE1tWLV+kg... [1] https://git.code.sf.net/p/gnu-efi/code
Regards, Raymond
participants (3)
-
Ilias Apalodimas
-
Raymond Mao
-
Tom Rini