Hi Cédric,
> Subject: Re: [PATCH v1 15/22] test/qtest/hace: Add SHA-384 test cases for
> ASPEED HACE model
>
> On 3/21/25 10:26, Jamin Lin wrote:
> > Introduced SHA-384 test functions to verify hashing operations.
> > Extended support for scatter-gather (`_sg`) and accumulation (`_accum`)
> tests.
> > Updated test result vectors for SHA-384 validation.
> >
> > Signed-off-by: Jamin Lin <jamin_...@aspeedtech.com>
> > ---
> > tests/qtest/aspeed-hace-utils.h | 6 ++
> > tests/qtest/aspeed-hace-utils.c | 168
> +++++++++++++++++++++++++++++++-
> > 2 files changed, 171 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/qtest/aspeed-hace-utils.h
> > b/tests/qtest/aspeed-hace-utils.h index 598577c69b..f4440561de 100644
> > --- a/tests/qtest/aspeed-hace-utils.h
> > +++ b/tests/qtest/aspeed-hace-utils.h
> > @@ -54,14 +54,20 @@ void aspeed_test_md5(const char *machine, const
> uint32_t base,
> > const uint32_t src_addr);
> > void aspeed_test_sha256(const char *machine, const uint32_t base,
> > const uint32_t src_addr);
> > +void aspeed_test_sha384(const char *machine, const uint32_t base,
> > + const uint32_t src_addr);
> > void aspeed_test_sha512(const char *machine, const uint32_t base,
> > const uint32_t src_addr);
> > void aspeed_test_sha256_sg(const char *machine, const uint32_t base,
> > const uint32_t src_addr);
> > +void aspeed_test_sha384_sg(const char *machine, const uint32_t base,
> > + const uint32_t src_addr);
> > void aspeed_test_sha512_sg(const char *machine, const uint32_t base,
> > const uint32_t src_addr);
> > void aspeed_test_sha256_accum(const char *machine, const uint32_t
> base,
> > const uint32_t src_addr);
> > +void aspeed_test_sha384_accum(const char *machine, const uint32_t base,
> > + const uint32_t src_addr);
> > void aspeed_test_sha512_accum(const char *machine, const uint32_t
> base,
> > const uint32_t src_addr);
> > void aspeed_test_addresses(const char *machine, const uint32_t base,
> > diff --git a/tests/qtest/aspeed-hace-utils.c
> > b/tests/qtest/aspeed-hace-utils.c index 8fbbba49c1..d3146898c2 100644
> > --- a/tests/qtest/aspeed-hace-utils.c
> > +++ b/tests/qtest/aspeed-hace-utils.c
> > @@ -16,7 +16,7 @@
> > * Expected results were generated using command line utitiles:
> > *
> > * echo -n -e 'abc' | dd of=/tmp/test
> > - * for hash in sha512sum sha256sum md5sum; do $hash /tmp/test; done
> > + * for hash in sha512sum sha384sum sha256sum md5sum; do $hash
> > + /tmp/test; done
> > *
> > */
> > static const uint8_t test_vector[] = {0x61, 0x62, 0x63}; @@ -29,6
> > +29,12 @@ static const uint8_t test_result_sha512[] = {
> > 0x45, 0x4d, 0x44, 0x23, 0x64, 0x3c, 0xe8, 0x0e, 0x2a, 0x9a, 0xc9,
> 0x4f,
> > 0xa5, 0x4c, 0xa4, 0x9f};
> >
> > +static const uint8_t test_result_sha384[] = {
> > + 0xcb, 0x00, 0x75, 0x3f, 0x45, 0xa3, 0x5e, 0x8b, 0xb5, 0xa0, 0x3d, 0x69,
> > + 0x9a, 0xc6, 0x50, 0x07, 0x27, 0x2c, 0x32, 0xab, 0x0e, 0xde, 0xd1,
> 0x63,
> > + 0x1a, 0x8b, 0x60, 0x5a, 0x43, 0xff, 0x5b, 0xed, 0x80, 0x86, 0x07, 0x2b,
> > + 0xa1, 0xe7, 0xcc, 0x23, 0x58, 0xba, 0xec, 0xa1, 0x34, 0xc8, 0x25,
> > +0xa7};
> > +
> > static const uint8_t test_result_sha256[] = {
> > 0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40,
> 0xde,
> > 0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17,
> > 0x7a, 0x9c, @@ -45,7 +51,7 @@ static const uint8_t test_result_md5[] = {
> > * Expected results were generated using command line utitiles:
> > *
> > * echo -n -e 'abcdefghijkl' | dd of=/tmp/test
> > - * for hash in sha512sum sha256sum; do $hash /tmp/test; done
> > + * for hash in sha512sum sha384sum sha256sum; do $hash /tmp/test;
> > + done
> > *
> > */
> > static const uint8_t test_vector_sg1[] = {0x61, 0x62, 0x63, 0x64,
> > 0x65, 0x66}; @@ -60,6 +66,12 @@ static const uint8_t
> test_result_sg_sha512[] = {
> > 0x84, 0x25, 0x7c, 0x32, 0xc8, 0xf6, 0xd0, 0x85, 0x4a, 0xe6, 0xb5,
> 0x40,
> > 0xf8, 0x6d, 0xda, 0x2e};
> >
> > +static const uint8_t test_result_sg_sha384[] = {
> > + 0x10, 0x3c, 0xa9, 0x6c, 0x06, 0xa1, 0xce, 0x79, 0x8f, 0x08, 0xf8, 0xef,
> > + 0xf0, 0xdf, 0xb0, 0xcc, 0xdb, 0x56, 0x7d, 0x48, 0xb2, 0x85, 0xb2, 0x3d,
> > + 0x0c, 0xd7, 0x73, 0x45, 0x46, 0x67, 0xa3, 0xc2, 0xfa, 0x5f, 0x1b, 0x58,
> > + 0xd9, 0xcd, 0xf2, 0x32, 0x9b, 0xd9, 0x97, 0x97, 0x30, 0xbf, 0xaa,
> > +0xff};
> > +
> > static const uint8_t test_result_sg_sha256[] = {
> > 0xd6, 0x82, 0xed, 0x4c, 0xa4, 0xd9, 0x89, 0xc1, 0x34, 0xec, 0x94,
> 0xf1,
> > 0x55, 0x1e, 0x1e, 0xc5, 0x80, 0xdd, 0x6d, 0x5a, 0x6e, 0xcd,
> > 0xe9, 0xf3, @@ -74,7 +86,7 @@ static const uint8_t test_result_sg_sha256[]
> = {
> > * Expected results were generated using command line utitiles:
> > *
> > * echo -n -e 'abc' | dd of=/tmp/test
> > - * for hash in sha512sum sha256sum; do $hash /tmp/test; done
> > + * for hash in sha512sum sha384sum sha256sum; do $hash /tmp/test;
> > + done
> > */
> > static const uint8_t test_vector_accum_512[] = {
> > 0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00, @@ -94,6 +106,24
> > @@ static const uint8_t test_vector_accum_512[] = {
> > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
> >
> > +static const uint8_t test_vector_accum_384[] = {
> > + 0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
> > +
> > static const uint8_t test_vector_accum_256[] = {
> > 0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
> > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -112,6
> > +142,12 @@ static const uint8_t test_result_accum_sha512[] = {
> > 0x45, 0x4d, 0x44, 0x23, 0x64, 0x3c, 0xe8, 0x0e, 0x2a, 0x9a, 0xc9,
> 0x4f,
> > 0xa5, 0x4c, 0xa4, 0x9f};
> >
> > +static const uint8_t test_result_accum_sha384[] = {
> > + 0xcb, 0x00, 0x75, 0x3f, 0x45, 0xa3, 0x5e, 0x8b, 0xb5, 0xa0, 0x3d, 0x69,
> > + 0x9a, 0xc6, 0x50, 0x07, 0x27, 0x2c, 0x32, 0xab, 0x0e, 0xde, 0xd1,
> 0x63,
> > + 0x1a, 0x8b, 0x60, 0x5a, 0x43, 0xff, 0x5b, 0xed, 0x80, 0x86, 0x07, 0x2b,
> > + 0xa1, 0xe7, 0xcc, 0x23, 0x58, 0xba, 0xec, 0xa1, 0x34, 0xc8, 0x25,
> > +0xa7};
> > +
> > static const uint8_t test_result_accum_sha256[] = {
> > 0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40,
> 0xde,
> > 0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17,
> > 0x7a, 0x9c, @@ -195,6 +231,40 @@ void aspeed_test_sha256(const char
> *machine, const uint32_t base,
> > qtest_quit(s);
> > }
> >
> > +void aspeed_test_sha384(const char *machine, const uint32_t base,
> > + const uint32_t src_addr) {
> > + QTestState *s = qtest_init(machine);
> > +
> > + const uint32_t digest_addr = src_addr + 0x10000;
> > + uint8_t digest[32] = {0};
> > +
> > + /* Check engine is idle, no busy or irq bits set */
> > + g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
> > +
> > + /* Write test vector into memory */
> > + qtest_memwrite(s, src_addr, test_vector, sizeof(test_vector));
> > +
> > + write_regs(s, base, src_addr, sizeof(test_vector), digest_addr,
> > + HACE_ALGO_SHA384);
> > +
> > + /* Check hash IRQ status is asserted */
> > + g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
> > +
> > + /* Clear IRQ status and check status is deasserted */
> > + qtest_writel(s, base + HACE_STS, 0x00000200);
> > + g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
> > +
> > + /* Read computed digest from memory */
> > + qtest_memread(s, digest_addr, digest, sizeof(digest));
> > +
> > + /* Check result of computation */
> > + g_assert_cmpmem(digest, sizeof(digest),
> > + test_result_sha384, sizeof(digest));
> > +
> > + qtest_quit(s);
> > +}
> > +
> > void aspeed_test_sha512(const char *machine, const uint32_t base,
> > const uint32_t src_addr)
> > {
> > @@ -280,6 +350,57 @@ void aspeed_test_sha256_sg(const char *machine,
> const uint32_t base,
> > qtest_quit(s);
> > }
> >
> > +void aspeed_test_sha384_sg(const char *machine, const uint32_t base,
> > + const uint32_t src_addr) {
> > + QTestState *s = qtest_init(machine);
> > +
> > + const uint32_t src_addr_1 = src_addr + 0x10000;
> > + const uint32_t src_addr_2 = src_addr + 0x20000;
> > + const uint32_t src_addr_3 = src_addr + 0x30000;
> > + const uint32_t digest_addr = src_addr + 0x40000;
> > + uint8_t digest[64] = {0};
>
> This does not compile (gcc version 14.2.1)
>
>
> ../tests/qtest/aspeed-hace-utils.c: In function ‘aspeed_test_sha384_sg’:
> /usr/include/glib-2.0/glib/gtestutils.h:93:84: error: ‘__builtin_memcmp_eq’
> specified bound 64 exceeds the size 48 of unterminated array
> [-Werror=stringop-overread]
> 93 | else if
> (__l1 != 0 && __m2 != NULL && memcmp (__m1, __m2, __l1) != 0) \
> |
> ^~~~~~~~~~~~~~~~~~~~~~~~~
> ../tests/qtest/aspeed-hace-utils.c:399:5: note: in expansion of macro
> ‘g_assert_cmpmem’
> 399 | g_assert_cmpmem(digest, sizeof(digest),
> | ^~~~~~~~~~~~~~~
> ../tests/qtest/aspeed-hace-utils.c:69:22: note: referenced argument declared
> here
> 69 | static const uint8_t test_result_sg_sha384[] = {
> | ^~~~~~~~~~~~~~~~~~~~~
>
I didn't explicitly set the array size for "test_result_sg_sha384", and the
compiler seems to have inferred a size of 64 bytes—likely due to 32-byte
alignment.
However, the actual SHA384 digest is 48 bytes, which is why your compiler
reported an error: it detected a possible buffer overread.
There are two possible ways to fix this issue:
Solution 1:
Define a constant MAX_SIZE_LENGTH (e.g., 64 bytes), and use it for all test
result arrays:
test_result_sha512[MAX_SIZE_LENGTH];
test_result_sha384[MAX_SIZE_LENGTH];
However, in this case, we cannot use "sizeof(test_result_sha384)" or
"sizeof(test_result_sha256)" in the test code, because the compiler will
evaluate them as 128 bytes.
Therefore, we should use hardcoded digest lengths (48 for SHA384, 32 for
SHA256) when comparing results in the tests.
Solution 2:
Set the actual digest size explicitly in the array definition:
test_result_sha512[64];
test_result_sha384[48];
This ensures "sizeof()" will return the correct digest size, and we can safely
use it in assertions.
I prefer solution 2.
Let me know which solution you'd prefer to go with.
Thanks-Jamin
>
>
>
> > + struct AspeedSgList array[] = {
> > + { cpu_to_le32(sizeof(test_vector_sg1)),
> > + cpu_to_le32(src_addr_1) },
> > + { cpu_to_le32(sizeof(test_vector_sg2)),
> > + cpu_to_le32(src_addr_2) },
> > + { cpu_to_le32(sizeof(test_vector_sg3) | SG_LIST_LEN_LAST),
> > + cpu_to_le32(src_addr_3) },
> > + };
> > +
> > + /* Check engine is idle, no busy or irq bits set */
> > + g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
> > +
> > + /* Write test vector into memory */
> > + qtest_memwrite(s, src_addr_1, test_vector_sg1,
> sizeof(test_vector_sg1));
> > + qtest_memwrite(s, src_addr_2, test_vector_sg2,
> sizeof(test_vector_sg2));
> > + qtest_memwrite(s, src_addr_3, test_vector_sg3,
> sizeof(test_vector_sg3));
> > + qtest_memwrite(s, src_addr, array, sizeof(array));
> > +
> > + write_regs(s, base, src_addr,
> > + (sizeof(test_vector_sg1)
> > + + sizeof(test_vector_sg2)
> > + + sizeof(test_vector_sg3)),
> > + digest_addr, HACE_ALGO_SHA384 | HACE_SG_EN);
> > +
> > + /* Check hash IRQ status is asserted */
> > + g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
> > +
> > + /* Clear IRQ status and check status is deasserted */
> > + qtest_writel(s, base + HACE_STS, 0x00000200);
> > + g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
> > +
> > + /* Read computed digest from memory */
> > + qtest_memread(s, digest_addr, digest, sizeof(digest));
> > +
> > + /* Check result of computation */
> > + g_assert_cmpmem(digest, sizeof(digest),
> > + test_result_sg_sha384, sizeof(digest));
> > +
> > + qtest_quit(s);
> > +}
> > +
> > void aspeed_test_sha512_sg(const char *machine, const uint32_t base,
> > const uint32_t src_addr)
> > {
> > @@ -372,6 +493,47 @@ void aspeed_test_sha256_accum(const char
> *machine, const uint32_t base,
> > qtest_quit(s);
> > }
> >
> > +void aspeed_test_sha384_accum(const char *machine, const uint32_t base,
> > + const uint32_t src_addr) {
> > + QTestState *s = qtest_init(machine);
> > +
> > + const uint32_t buffer_addr = src_addr + 0x10000;
> > + const uint32_t digest_addr = src_addr + 0x40000;
> > + uint8_t digest[64] = {0};
>
> ../tests/qtest/aspeed-hace-utils.c: In function ‘aspeed_test_sha384_accum’:
> /usr/include/glib-2.0/glib/gtestutils.h:93:84: error: ‘__builtin_memcmp_eq’
> specified bound 64 exceeds source size 48 [-Werror=stringop-overread]
> 93 | else if
> (__l1 != 0 && __m2 != NULL && memcmp (__m1, __m2, __l1) != 0) \
> |
> ^~~~~~~~~~~~~~~~~~~~~~~~~
> ../tests/qtest/aspeed-hace-utils.c:533:5: note: in expansion of macro
> ‘g_assert_cmpmem’
> 533 | g_assert_cmpmem(digest, sizeof(digest),
> | ^~~~~~~~~~~~~~~
> ../tests/qtest/aspeed-hace-utils.c:145:22: note: source object declared here
> 145 | static const uint8_t test_result_accum_sha384[] = {
> | ^~~~~~~~~~~~~~~~~~~~~~~~
>
>
> Thanks,
>
> C.
>
>
>
> > + struct AspeedSgList array[] = {
> > + { cpu_to_le32(sizeof(test_vector_accum_384) |
> SG_LIST_LEN_LAST),
> > + cpu_to_le32(buffer_addr) },
> > + };
> > +
> > + /* Check engine is idle, no busy or irq bits set */
> > + g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
> > +
> > + /* Write test vector into memory */
> > + qtest_memwrite(s, buffer_addr, test_vector_accum_384,
> > + sizeof(test_vector_accum_384));
> > + qtest_memwrite(s, src_addr, array, sizeof(array));
> > +
> > + write_regs(s, base, src_addr, sizeof(test_vector_accum_384),
> > + digest_addr, HACE_ALGO_SHA384 | HACE_SG_EN |
> > + HACE_ACCUM_EN);
> > +
> > + /* Check hash IRQ status is asserted */
> > + g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
> > +
> > + /* Clear IRQ status and check status is deasserted */
> > + qtest_writel(s, base + HACE_STS, 0x00000200);
> > + g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
> > +
> > + /* Read computed digest from memory */
> > + qtest_memread(s, digest_addr, digest, sizeof(digest));
> > +
> > + /* Check result of computation */
> > + g_assert_cmpmem(digest, sizeof(digest),
> > + test_result_accum_sha384, sizeof(digest));
> > +
> > + qtest_quit(s);
> > +}
> > +
> > void aspeed_test_sha512_accum(const char *machine, const uint32_t
> base,
> > const uint32_t src_addr)
> > {
