2022-12-20 20:40:06

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH] crypto: lib/blake2s - Split up test function to halve stack usage

On Sun, Dec 11, 2022 at 02:22:29PM +0800, Herbert Xu wrote:
> On Thu, Dec 08, 2022 at 02:37:45PM +0800, Chen, Rong A wrote:
> >
> > Thanks for the information, I checked the result of the fix commit,
> > it reduced the stack frame but the warning still exists:
> >
> > lib/crypto/blake2s-selftest.c:632:1: warning: the frame size of 1056 bytes
> > is larger than 1024 bytes [-Wframe-larger-than=]
>
> Sorry, I didn't realise that you could still reproduce it. I tried
> reproducing it on multiple architectures and failed (all were in
> the 900 range).
>
> Anyhow, this patch reduces it by half for me so hopefully it should
> put this to rest.
>
> ---8<---
> Reduce the stack usage further by splitting up the test function.
>
> Also squash blocks and unaligned_blocks into one array.
>
> Signed-off-by: Herbert Xu <[email protected]>
>
> diff --git a/lib/crypto/blake2s-selftest.c b/lib/crypto/blake2s-selftest.c
> index 7d77dea15587..484b89332c83 100644
> --- a/lib/crypto/blake2s-selftest.c
> +++ b/lib/crypto/blake2s-selftest.c
> @@ -545,7 +545,7 @@ static const u8 blake2s_testvecs[][BLAKE2S_HASH_SIZE] __initconst = {
> 0xd6, 0x98, 0x6b, 0x07, 0x10, 0x65, 0x52, 0x65, },
> };
>
> -bool __init blake2s_selftest(void)
> +static bool __init blake2s_digest_test(void)
> {
> u8 key[BLAKE2S_KEY_SIZE];
> u8 buf[ARRAY_SIZE(blake2s_testvecs)];
> @@ -589,11 +589,20 @@ bool __init blake2s_selftest(void)
> }
> }
>
> + return success;
> +}
> +
> +static bool __init blake2s_random_test(void)
> +{
> + struct blake2s_state state;
> + bool success = true;
> + int i, l;
> +

Shouldn't these functions have 'noinline_for_stack' added to them?

- Eric


2022-12-21 07:18:18

by Herbert Xu

[permalink] [raw]
Subject: [v2 PATCH] crypto: lib/blake2s - Split up test function to halve stack usage

On Tue, Dec 20, 2022 at 08:34:18PM +0000, Eric Biggers wrote:
>
> Shouldn't these functions have 'noinline_for_stack' added to them?

Indeed they should. Thanks!

---8<---
Reduce the stack usage further by splitting up the test function.

Also squash blocks and unaligned_blocks into one array.

Reported-by: kernel test robot <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>

diff --git a/lib/crypto/blake2s-selftest.c b/lib/crypto/blake2s-selftest.c
index 7d77dea15587..d0634ed6a937 100644
--- a/lib/crypto/blake2s-selftest.c
+++ b/lib/crypto/blake2s-selftest.c
@@ -545,7 +545,7 @@ static const u8 blake2s_testvecs[][BLAKE2S_HASH_SIZE] __initconst = {
0xd6, 0x98, 0x6b, 0x07, 0x10, 0x65, 0x52, 0x65, },
};

-bool __init blake2s_selftest(void)
+static bool __init noinline_for_stack blake2s_digest_test(void)
{
u8 key[BLAKE2S_KEY_SIZE];
u8 buf[ARRAY_SIZE(blake2s_testvecs)];
@@ -589,11 +589,20 @@ bool __init blake2s_selftest(void)
}
}

+ return success;
+}
+
+static bool __init noinline_for_stack blake2s_random_test(void)
+{
+ struct blake2s_state state;
+ bool success = true;
+ int i, l;
+
for (i = 0; i < 32; ++i) {
enum { TEST_ALIGNMENT = 16 };
- u8 unaligned_block[BLAKE2S_BLOCK_SIZE + TEST_ALIGNMENT - 1]
+ u8 blocks[BLAKE2S_BLOCK_SIZE * 2 + TEST_ALIGNMENT - 1]
__aligned(TEST_ALIGNMENT);
- u8 blocks[BLAKE2S_BLOCK_SIZE * 2];
+ u8 *unaligned_block = blocks + BLAKE2S_BLOCK_SIZE;
struct blake2s_state state1, state2;

get_random_bytes(blocks, sizeof(blocks));
@@ -630,3 +639,13 @@ bool __init blake2s_selftest(void)

return success;
}
+
+bool __init blake2s_selftest(void)
+{
+ bool success;
+
+ success = blake2s_digest_test();
+ success &= blake2s_random_test();
+
+ return success;
+}
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt