public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] crypto: arm64/aes-neonbs - Move key expansion off the stack
@ 2026-03-05 18:32 Cheng-Yang Chou
  2026-03-05 18:32 ` [PATCH 1/1] " Cheng-Yang Chou
  0 siblings, 1 reply; 5+ messages in thread
From: Cheng-Yang Chou @ 2026-03-05 18:32 UTC (permalink / raw)
  To: herbert, davem, catalin.marinas, will, linux-crypto,
	linux-arm-kernel, linux-kernel
  Cc: jserv, yphbchou0911

aesbs_setkey() and aesbs_cbc_ctr_setkey() trigger -Wframe-larger-than=
warnings because struct crypto_aes_ctx is allocated on the stack,
pushing the frame size to ~1040 bytes and exceeding the 1024-byte limit.

arch/arm64/crypto/aes-neonbs-glue.c: In function ‘aesbs_setkey’:
arch/arm64/crypto/aes-neonbs-glue.c:92:1: warning: the frame size of 1040 bytes is larger than 1024 bytes [-Wframe-larger-than=]
   92 | }
      | ^
arch/arm64/crypto/aes-neonbs-glue.c: In function ‘aesbs_cbc_ctr_setkey’:
arch/arm64/crypto/aes-neonbs-glue.c:152:1: warning: the frame size of 1040 bytes is larger than 1024 bytes [-Wframe-larger-than=]
  152 | }
      | ^

Tested on arm64. Confirmed the -Wframe-larger-than= warning is resolved.

Thanks,
Cheng-Yang

---

Cheng-Yang Chou (1):
  crypto: arm64/aes-neonbs - Move key expansion off the stack

 arch/arm64/crypto/aes-neonbs-glue.c | 39 ++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 14 deletions(-)

-- 
2.48.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/1] crypto: arm64/aes-neonbs - Move key expansion off the stack
  2026-03-05 18:32 [PATCH 0/1] crypto: arm64/aes-neonbs - Move key expansion off the stack Cheng-Yang Chou
@ 2026-03-05 18:32 ` Cheng-Yang Chou
  2026-03-05 19:38   ` Eric Biggers
  0 siblings, 1 reply; 5+ messages in thread
From: Cheng-Yang Chou @ 2026-03-05 18:32 UTC (permalink / raw)
  To: herbert, davem, catalin.marinas, will, linux-crypto,
	linux-arm-kernel, linux-kernel
  Cc: jserv, yphbchou0911

aesbs_setkey() and aesbs_cbc_ctr_setkey() trigger -Wframe-larger-than=
warnings due to struct crypto_aes_ctx being allocated on the stack,
causing the frame size to exceed 1024 bytes.

Allocate struct crypto_aes_ctx on the heap instead to reduce stack
usage. Use a goto-based cleanup path to ensure memzero_explicit() and
kfree() are always called.

Signed-off-by: Cheng-Yang Chou <yphbchou0911@gmail.com>
---
 arch/arm64/crypto/aes-neonbs-glue.c | 39 ++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/crypto/aes-neonbs-glue.c b/arch/arm64/crypto/aes-neonbs-glue.c
index cb87c8fc66b3..a24b66fd5cad 100644
--- a/arch/arm64/crypto/aes-neonbs-glue.c
+++ b/arch/arm64/crypto/aes-neonbs-glue.c
@@ -76,19 +76,25 @@ static int aesbs_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
 			unsigned int key_len)
 {
 	struct aesbs_ctx *ctx = crypto_skcipher_ctx(tfm);
-	struct crypto_aes_ctx rk;
+	struct crypto_aes_ctx *rk;
 	int err;
 
-	err = aes_expandkey(&rk, in_key, key_len);
+	rk = kmalloc(sizeof(*rk), GFP_KERNEL);
+	if (!rk)
+		return -ENOMEM;
+
+	err = aes_expandkey(rk, in_key, key_len);
 	if (err)
-		return err;
+		goto out;
 
 	ctx->rounds = 6 + key_len / 4;
 
 	scoped_ksimd()
-		aesbs_convert_key(ctx->rk, rk.key_enc, ctx->rounds);
-
-	return 0;
+		aesbs_convert_key(ctx->rk, rk->key_enc, ctx->rounds);
+out:
+	memzero_explicit(rk, sizeof(*rk));
+	kfree(rk);
+	return err;
 }
 
 static int __ecb_crypt(struct skcipher_request *req,
@@ -133,22 +139,27 @@ static int aesbs_cbc_ctr_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
 			    unsigned int key_len)
 {
 	struct aesbs_cbc_ctr_ctx *ctx = crypto_skcipher_ctx(tfm);
-	struct crypto_aes_ctx rk;
+	struct crypto_aes_ctx *rk;
 	int err;
 
-	err = aes_expandkey(&rk, in_key, key_len);
+	rk = kmalloc(sizeof(*rk), GFP_KERNEL);
+	if (!rk)
+		return -ENOMEM;
+
+	err = aes_expandkey(rk, in_key, key_len);
 	if (err)
-		return err;
+		goto out;
 
 	ctx->key.rounds = 6 + key_len / 4;
 
-	memcpy(ctx->enc, rk.key_enc, sizeof(ctx->enc));
+	memcpy(ctx->enc, rk->key_enc, sizeof(ctx->enc));
 
 	scoped_ksimd()
-		aesbs_convert_key(ctx->key.rk, rk.key_enc, ctx->key.rounds);
-	memzero_explicit(&rk, sizeof(rk));
-
-	return 0;
+		aesbs_convert_key(ctx->key.rk, rk->key_enc, ctx->key.rounds);
+out:
+	memzero_explicit(rk, sizeof(*rk));
+	kfree(rk);
+	return err;
 }
 
 static int cbc_encrypt(struct skcipher_request *req)
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] crypto: arm64/aes-neonbs - Move key expansion off the stack
  2026-03-05 18:32 ` [PATCH 1/1] " Cheng-Yang Chou
@ 2026-03-05 19:38   ` Eric Biggers
  2026-03-06  5:46     ` Cheng-Yang Chou
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Biggers @ 2026-03-05 19:38 UTC (permalink / raw)
  To: Cheng-Yang Chou
  Cc: herbert, davem, catalin.marinas, will, linux-crypto,
	linux-arm-kernel, linux-kernel, jserv

On Fri, Mar 06, 2026 at 02:32:24AM +0800, Cheng-Yang Chou wrote:
> aesbs_setkey() and aesbs_cbc_ctr_setkey() trigger -Wframe-larger-than=
> warnings due to struct crypto_aes_ctx being allocated on the stack,
> causing the frame size to exceed 1024 bytes.
> 
> Allocate struct crypto_aes_ctx on the heap instead to reduce stack
> usage. Use a goto-based cleanup path to ensure memzero_explicit() and
> kfree() are always called.
> 
> Signed-off-by: Cheng-Yang Chou <yphbchou0911@gmail.com>
> ---
>  arch/arm64/crypto/aes-neonbs-glue.c | 39 ++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm64/crypto/aes-neonbs-glue.c b/arch/arm64/crypto/aes-neonbs-glue.c
> index cb87c8fc66b3..a24b66fd5cad 100644
> --- a/arch/arm64/crypto/aes-neonbs-glue.c
> +++ b/arch/arm64/crypto/aes-neonbs-glue.c
> @@ -76,19 +76,25 @@ static int aesbs_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
>  			unsigned int key_len)
>  {
>  	struct aesbs_ctx *ctx = crypto_skcipher_ctx(tfm);
> -	struct crypto_aes_ctx rk;
> +	struct crypto_aes_ctx *rk;
>  	int err;
>  
> -	err = aes_expandkey(&rk, in_key, key_len);
> +	rk = kmalloc(sizeof(*rk), GFP_KERNEL);
> +	if (!rk)
> +		return -ENOMEM;
> +
> +	err = aes_expandkey(rk, in_key, key_len);
>  	if (err)
> -		return err;
> +		goto out;
>  
>  	ctx->rounds = 6 + key_len / 4;
>  
>  	scoped_ksimd()
> -		aesbs_convert_key(ctx->rk, rk.key_enc, ctx->rounds);
> -
> -	return 0;
> +		aesbs_convert_key(ctx->rk, rk->key_enc, ctx->rounds);
> +out:
> +	memzero_explicit(rk, sizeof(*rk));
> +	kfree(rk);
> +	return err;
>  }
>  
>  static int __ecb_crypt(struct skcipher_request *req,
> @@ -133,22 +139,27 @@ static int aesbs_cbc_ctr_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
>  			    unsigned int key_len)
>  {
>  	struct aesbs_cbc_ctr_ctx *ctx = crypto_skcipher_ctx(tfm);
> -	struct crypto_aes_ctx rk;
> +	struct crypto_aes_ctx *rk;
>  	int err;
>  
> -	err = aes_expandkey(&rk, in_key, key_len);
> +	rk = kmalloc(sizeof(*rk), GFP_KERNEL);
> +	if (!rk)
> +		return -ENOMEM;
> +
> +	err = aes_expandkey(rk, in_key, key_len);
>  	if (err)
> -		return err;
> +		goto out;
>  
>  	ctx->key.rounds = 6 + key_len / 4;
>  
> -	memcpy(ctx->enc, rk.key_enc, sizeof(ctx->enc));
> +	memcpy(ctx->enc, rk->key_enc, sizeof(ctx->enc));
>  
>  	scoped_ksimd()
> -		aesbs_convert_key(ctx->key.rk, rk.key_enc, ctx->key.rounds);
> -	memzero_explicit(&rk, sizeof(rk));
> -
> -	return 0;
> +		aesbs_convert_key(ctx->key.rk, rk->key_enc, ctx->key.rounds);
> +out:
> +	memzero_explicit(rk, sizeof(*rk));
> +	kfree(rk);
> +	return err;
>  }

Instead of memzero_explicit() followed by kfree(), just use
kfree_sensitive().

Also, single patches should not have a cover letter.  Just send a single
patch email with all the details in the patch itself.

As for the actual change, I guess it's okay for now.  Ideally we'd
refactor the aes-bs key preparation to not need temporary space.

- Eric

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] crypto: arm64/aes-neonbs - Move key expansion off the stack
  2026-03-05 19:38   ` Eric Biggers
@ 2026-03-06  5:46     ` Cheng-Yang Chou
  2026-03-06  5:54       ` Eric Biggers
  0 siblings, 1 reply; 5+ messages in thread
From: Cheng-Yang Chou @ 2026-03-06  5:46 UTC (permalink / raw)
  To: Eric Biggers
  Cc: herbert, davem, catalin.marinas, will, linux-crypto,
	linux-arm-kernel, linux-kernel, jserv

Hi Eric,

On Thu, Mar 05, 2026 at 11:38:47AM -0800, Eric Biggers wrote:
> Instead of memzero_explicit() followed by kfree(), just use
> kfree_sensitive().
> 
> Also, single patches should not have a cover letter.  Just send a single
> patch email with all the details in the patch itself.
> 
> As for the actual change, I guess it's okay for now.  Ideally we'd
> refactor the aes-bs key preparation to not need temporary space.

Thanks for the feedback.
I'll send a v2 to address your comments.

The arm implementation also allocates struct crypto_aes_ctx on the
stack in aesbs_setkey(). Should I include a fix for it as well?
Note that I can only test on arm64.

Also, I'd be happy to help with the refactoring if you can point me
in the right direction.

-- 
Thanks,
Cheng-Yang

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] crypto: arm64/aes-neonbs - Move key expansion off the stack
  2026-03-06  5:46     ` Cheng-Yang Chou
@ 2026-03-06  5:54       ` Eric Biggers
  0 siblings, 0 replies; 5+ messages in thread
From: Eric Biggers @ 2026-03-06  5:54 UTC (permalink / raw)
  To: Cheng-Yang Chou
  Cc: herbert, davem, catalin.marinas, will, linux-crypto,
	linux-arm-kernel, linux-kernel, jserv

On Fri, Mar 06, 2026 at 01:46:33PM +0800, Cheng-Yang Chou wrote:
> Hi Eric,
> 
> On Thu, Mar 05, 2026 at 11:38:47AM -0800, Eric Biggers wrote:
> > Instead of memzero_explicit() followed by kfree(), just use
> > kfree_sensitive().
> > 
> > Also, single patches should not have a cover letter.  Just send a single
> > patch email with all the details in the patch itself.
> > 
> > As for the actual change, I guess it's okay for now.  Ideally we'd
> > refactor the aes-bs key preparation to not need temporary space.
> 
> Thanks for the feedback.
> I'll send a v2 to address your comments.
> 
> The arm implementation also allocates struct crypto_aes_ctx on the
> stack in aesbs_setkey(). Should I include a fix for it as well?
> Note that I can only test on arm64.
> 
> Also, I'd be happy to help with the refactoring if you can point me
> in the right direction.

arm doesn't store the kernel-mode NEON context on the stack, so a
similar change shouldn't be needed there.  This issue showed up only
because arm64 started doing that, which made the stack memory used by
aesbs_setkey() exceed ~1000 bytes due to the crypto_aes_ctx and the
kernel-mode NEON context each using about 500.

- Eric

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-03-06  5:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-05 18:32 [PATCH 0/1] crypto: arm64/aes-neonbs - Move key expansion off the stack Cheng-Yang Chou
2026-03-05 18:32 ` [PATCH 1/1] " Cheng-Yang Chou
2026-03-05 19:38   ` Eric Biggers
2026-03-06  5:46     ` Cheng-Yang Chou
2026-03-06  5:54       ` Eric Biggers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox