diff mbox series

[v2,1/7] crypto: sun4i-ss: linearize buffers content must be kept

Message ID 1600627038-40000-2-git-send-email-clabbe@baylibre.com
State New
Headers show
Series crypto: sun4i-ss: prevent always fallback for ciphers | expand

Commit Message

Corentin Labbe Sept. 20, 2020, 6:37 p.m. UTC
When running the non-optimized cipher function, SS produce partial random
output.
This is due to linearize buffers being reseted after each loop.

Fixes: 8d3bcb9900ca ("crypto: sun4i-ss - reduce stack usage")
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Herbert Xu Sept. 25, 2020, 7:30 a.m. UTC | #1
On Sun, Sep 20, 2020 at 06:37:12PM +0000, Corentin Labbe wrote:
> When running the non-optimized cipher function, SS produce partial random

> output.

> This is due to linearize buffers being reseted after each loop.

> 

> Fixes: 8d3bcb9900ca ("crypto: sun4i-ss - reduce stack usage")

> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>

> ---

>  drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c | 6 ++----

>  1 file changed, 2 insertions(+), 4 deletions(-)

> 

> diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c

> index b72de8939497..b92d175b5d2a 100644

> --- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c

> +++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c

> @@ -163,6 +163,8 @@ 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 */

> @@ -233,8 +235,6 @@ 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


Ouch.  So this is obviously correct but I wonder if the stack
usage would be an issue again?

How about moving this code into another function so that it sits
at the same level as the fallback function, which would mean that
the buffers do not double up with the one for the fallback?

Cheers,
-- 
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
diff mbox series

Patch

diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
index b72de8939497..b92d175b5d2a 100644
--- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
+++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
@@ -163,6 +163,8 @@  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 */
@@ -233,8 +235,6 @@  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
@@ -295,8 +295,6 @@  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