Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] crypto: sun4i-ss - reduce stack usage
@ 2019-06-17 13:25 Arnd Bergmann
  2019-06-18 13:12 ` Corentin Labbe
  2019-06-28  4:18 ` Herbert Xu
  0 siblings, 2 replies; 3+ messages in thread
From: Arnd Bergmann @ 2019-06-17 13:25 UTC (permalink / raw)
  To: Corentin Labbe, Herbert Xu, David S. Miller
  Cc: Arnd Bergmann, Eric Biggers, Maxime Ripard, linux-kernel,
	Chen-Yu Tsai, linux-crypto, linux-arm-kernel

After the latest addition, the stack usage of sun4i_ss_cipher_poll
grew beyond the warning limit when KASAN is enabled:

drivers/crypto/sunxi-ss/sun4i-ss-cipher.c:118:12: error: stack frame size of 1152 bytes in function 'sun4i_ss_cipher_poll' [-Werror,-Wframe-larger-than=]
static int sun4i_ss_cipher_poll(struct skcipher_request *areq)

Reduce it in three ways:

- split out the new code into a separate function so its stack
  usage can overlap that of the sun4i_ss_opti_poll() code path
- mark both special cases as noinline_for_stack, which should
  ideally result in a tail call that frees the rest of the
  stack
- move the buf and obuf variables into the code blocks in
  which they are used.

The three separate functions now use 144, 640 and 304 bytes of kernel
stack, respectively.

Fixes: 0ae1f46c55f8 ("crypto: sun4i-ss - fallback when length is not multiple of blocksize")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/crypto/sunxi-ss/sun4i-ss-cipher.c | 47 +++++++++++++++--------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c b/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
index 7b0c42882830..4ab14d58e85b 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
@@ -12,7 +12,7 @@
  */
 #include "sun4i-ss.h"
 
-static int sun4i_ss_opti_poll(struct skcipher_request *areq)
+static int noinline_for_stack sun4i_ss_opti_poll(struct skcipher_request *areq)
 {
 	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(areq);
 	struct sun4i_tfm_ctx *op = crypto_skcipher_ctx(tfm);
@@ -114,6 +114,29 @@ static int sun4i_ss_opti_poll(struct skcipher_request *areq)
 	return err;
 }
 
+
+static int noinline_for_stack sun4i_ss_cipher_poll_fallback(struct skcipher_request *areq)
+{
+	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(areq);
+	struct sun4i_tfm_ctx *op = crypto_skcipher_ctx(tfm);
+	struct sun4i_cipher_req_ctx *ctx = skcipher_request_ctx(areq);
+	SYNC_SKCIPHER_REQUEST_ON_STACK(subreq, op->fallback_tfm);
+	int err;
+
+	skcipher_request_set_sync_tfm(subreq, op->fallback_tfm);
+	skcipher_request_set_callback(subreq, areq->base.flags, NULL,
+				      NULL);
+	skcipher_request_set_crypt(subreq, areq->src, areq->dst,
+				   areq->cryptlen, areq->iv);
+	if (ctx->mode & SS_DECRYPTION)
+		err = crypto_skcipher_decrypt(subreq);
+	else
+		err = crypto_skcipher_encrypt(subreq);
+	skcipher_request_zero(subreq);
+
+	return err;
+}
+
 /* Generic function that support SG with size not multiple of 4 */
 static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
 {
@@ -140,8 +163,6 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
 	unsigned int todo;
 	struct sg_mapping_iter mi, mo;
 	unsigned int oi, oo;	/* offset for in and out */
-	char buf[4 * SS_RX_MAX];/* buffer for linearize SG src */
-	char bufo[4 * SS_TX_MAX]; /* buffer for linearize SG dst */
 	unsigned int ob = 0;	/* offset in buf */
 	unsigned int obo = 0;	/* offset in bufo*/
 	unsigned int obl = 0;	/* length of data in bufo */
@@ -178,20 +199,8 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
 	if (no_chunk == 1 && !need_fallback)
 		return sun4i_ss_opti_poll(areq);
 
-	if (need_fallback) {
-		SYNC_SKCIPHER_REQUEST_ON_STACK(subreq, op->fallback_tfm);
-		skcipher_request_set_sync_tfm(subreq, op->fallback_tfm);
-		skcipher_request_set_callback(subreq, areq->base.flags, NULL,
-					      NULL);
-		skcipher_request_set_crypt(subreq, areq->src, areq->dst,
-					   areq->cryptlen, areq->iv);
-		if (ctx->mode & SS_DECRYPTION)
-			err = crypto_skcipher_decrypt(subreq);
-		else
-			err = crypto_skcipher_encrypt(subreq);
-		skcipher_request_zero(subreq);
-		return err;
-	}
+	if (need_fallback)
+		return sun4i_ss_cipher_poll_fallback(areq);
 
 	spin_lock_irqsave(&ss->slock, flags);
 
@@ -224,6 +233,8 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
 
 	while (oleft) {
 		if (ileft) {
+			char buf[4 * SS_RX_MAX];/* buffer for linearize SG src */
+
 			/*
 			 * todo is the number of consecutive 4byte word that we
 			 * can read from current SG
@@ -281,6 +292,8 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
 				oo = 0;
 			}
 		} else {
+			char bufo[4 * SS_TX_MAX]; /* buffer for linearize SG dst */
+
 			/*
 			 * read obl bytes in bufo, we read at maximum for
 			 * emptying the device
-- 
2.20.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] crypto: sun4i-ss - reduce stack usage
  2019-06-17 13:25 [PATCH] crypto: sun4i-ss - reduce stack usage Arnd Bergmann
@ 2019-06-18 13:12 ` Corentin Labbe
  2019-06-28  4:18 ` Herbert Xu
  1 sibling, 0 replies; 3+ messages in thread
From: Corentin Labbe @ 2019-06-18 13:12 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Herbert Xu, Eric Biggers, Maxime Ripard, linux-kernel,
	Chen-Yu Tsai, linux-crypto, David S. Miller, linux-arm-kernel

On Mon, Jun 17, 2019 at 03:25:17PM +0200, Arnd Bergmann wrote:
> After the latest addition, the stack usage of sun4i_ss_cipher_poll
> grew beyond the warning limit when KASAN is enabled:
> 
> drivers/crypto/sunxi-ss/sun4i-ss-cipher.c:118:12: error: stack frame size of 1152 bytes in function 'sun4i_ss_cipher_poll' [-Werror,-Wframe-larger-than=]
> static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
> 
> Reduce it in three ways:
> 
> - split out the new code into a separate function so its stack
>   usage can overlap that of the sun4i_ss_opti_poll() code path
> - mark both special cases as noinline_for_stack, which should
>   ideally result in a tail call that frees the rest of the
>   stack
> - move the buf and obuf variables into the code blocks in
>   which they are used.
> 
> The three separate functions now use 144, 640 and 304 bytes of kernel
> stack, respectively.
> 
> Fixes: 0ae1f46c55f8 ("crypto: sun4i-ss - fallback when length is not multiple of blocksize")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/crypto/sunxi-ss/sun4i-ss-cipher.c | 47 +++++++++++++++--------
>  1 file changed, 30 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c b/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
> index 7b0c42882830..4ab14d58e85b 100644
> --- a/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
> +++ b/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
> @@ -12,7 +12,7 @@
>   */
>  #include "sun4i-ss.h"
>  
> -static int sun4i_ss_opti_poll(struct skcipher_request *areq)
> +static int noinline_for_stack sun4i_ss_opti_poll(struct skcipher_request *areq)
>  {
>  	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(areq);
>  	struct sun4i_tfm_ctx *op = crypto_skcipher_ctx(tfm);
> @@ -114,6 +114,29 @@ static int sun4i_ss_opti_poll(struct skcipher_request *areq)
>  	return err;
>  }
>  
> +
> +static int noinline_for_stack sun4i_ss_cipher_poll_fallback(struct skcipher_request *areq)
> +{
> +	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(areq);
> +	struct sun4i_tfm_ctx *op = crypto_skcipher_ctx(tfm);
> +	struct sun4i_cipher_req_ctx *ctx = skcipher_request_ctx(areq);
> +	SYNC_SKCIPHER_REQUEST_ON_STACK(subreq, op->fallback_tfm);
> +	int err;
> +
> +	skcipher_request_set_sync_tfm(subreq, op->fallback_tfm);
> +	skcipher_request_set_callback(subreq, areq->base.flags, NULL,
> +				      NULL);
> +	skcipher_request_set_crypt(subreq, areq->src, areq->dst,
> +				   areq->cryptlen, areq->iv);
> +	if (ctx->mode & SS_DECRYPTION)
> +		err = crypto_skcipher_decrypt(subreq);
> +	else
> +		err = crypto_skcipher_encrypt(subreq);
> +	skcipher_request_zero(subreq);
> +
> +	return err;
> +}
> +
>  /* Generic function that support SG with size not multiple of 4 */
>  static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
>  {
> @@ -140,8 +163,6 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
>  	unsigned int todo;
>  	struct sg_mapping_iter mi, mo;
>  	unsigned int oi, oo;	/* offset for in and out */
> -	char buf[4 * SS_RX_MAX];/* buffer for linearize SG src */
> -	char bufo[4 * SS_TX_MAX]; /* buffer for linearize SG dst */
>  	unsigned int ob = 0;	/* offset in buf */
>  	unsigned int obo = 0;	/* offset in bufo*/
>  	unsigned int obl = 0;	/* length of data in bufo */
> @@ -178,20 +199,8 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
>  	if (no_chunk == 1 && !need_fallback)
>  		return sun4i_ss_opti_poll(areq);
>  
> -	if (need_fallback) {
> -		SYNC_SKCIPHER_REQUEST_ON_STACK(subreq, op->fallback_tfm);
> -		skcipher_request_set_sync_tfm(subreq, op->fallback_tfm);
> -		skcipher_request_set_callback(subreq, areq->base.flags, NULL,
> -					      NULL);
> -		skcipher_request_set_crypt(subreq, areq->src, areq->dst,
> -					   areq->cryptlen, areq->iv);
> -		if (ctx->mode & SS_DECRYPTION)
> -			err = crypto_skcipher_decrypt(subreq);
> -		else
> -			err = crypto_skcipher_encrypt(subreq);
> -		skcipher_request_zero(subreq);
> -		return err;
> -	}
> +	if (need_fallback)
> +		return sun4i_ss_cipher_poll_fallback(areq);
>  
>  	spin_lock_irqsave(&ss->slock, flags);
>  
> @@ -224,6 +233,8 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
>  
>  	while (oleft) {
>  		if (ileft) {
> +			char buf[4 * SS_RX_MAX];/* buffer for linearize SG src */
> +
>  			/*
>  			 * todo is the number of consecutive 4byte word that we
>  			 * can read from current SG
> @@ -281,6 +292,8 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
>  				oo = 0;
>  			}
>  		} else {
> +			char bufo[4 * SS_TX_MAX]; /* buffer for linearize SG dst */
> +
>  			/*
>  			 * read obl bytes in bufo, we read at maximum for
>  			 * emptying the device
> -- 
> 2.20.0
> 

Tested-by: Corentin LABBE <clabbe.montjoie@gmail.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] crypto: sun4i-ss - reduce stack usage
  2019-06-17 13:25 [PATCH] crypto: sun4i-ss - reduce stack usage Arnd Bergmann
  2019-06-18 13:12 ` Corentin Labbe
@ 2019-06-28  4:18 ` Herbert Xu
  1 sibling, 0 replies; 3+ messages in thread
From: Herbert Xu @ 2019-06-28  4:18 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Eric Biggers, Maxime Ripard, linux-kernel, Chen-Yu Tsai,
	Corentin Labbe, linux-crypto, David S. Miller, linux-arm-kernel

On Mon, Jun 17, 2019 at 03:25:17PM +0200, Arnd Bergmann wrote:
> After the latest addition, the stack usage of sun4i_ss_cipher_poll
> grew beyond the warning limit when KASAN is enabled:
> 
> drivers/crypto/sunxi-ss/sun4i-ss-cipher.c:118:12: error: stack frame size of 1152 bytes in function 'sun4i_ss_cipher_poll' [-Werror,-Wframe-larger-than=]
> static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
> 
> Reduce it in three ways:
> 
> - split out the new code into a separate function so its stack
>   usage can overlap that of the sun4i_ss_opti_poll() code path
> - mark both special cases as noinline_for_stack, which should
>   ideally result in a tail call that frees the rest of the
>   stack
> - move the buf and obuf variables into the code blocks in
>   which they are used.
> 
> The three separate functions now use 144, 640 and 304 bytes of kernel
> stack, respectively.
> 
> Fixes: 0ae1f46c55f8 ("crypto: sun4i-ss - fallback when length is not multiple of blocksize")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/crypto/sunxi-ss/sun4i-ss-cipher.c | 47 +++++++++++++++--------
>  1 file changed, 30 insertions(+), 17 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-17 13:25 [PATCH] crypto: sun4i-ss - reduce stack usage Arnd Bergmann
2019-06-18 13:12 ` Corentin Labbe
2019-06-28  4:18 ` Herbert Xu

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org infradead-linux-arm-kernel@archiver.kernel.org
	public-inbox-index linux-arm-kernel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox