All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 4.19] sunrpc: fix 4 more call sites that were using stack memory with a scatterlist
@ 2019-02-19 19:36 Scott Mayhew
  2019-02-19 21:38 ` J. Bruce Fields
  2019-02-21 12:23 ` Greg KH
  0 siblings, 2 replies; 3+ messages in thread
From: Scott Mayhew @ 2019-02-19 19:36 UTC (permalink / raw)
  To: stable; +Cc: bfields

(cherry picked from commit e7afe6c1d486b516ed586dcc10b3e7e3e85a9c2b)

While trying to reproduce a reported kernel panic on arm64, I discovered
that AUTH_GSS basically doesn't work at all with older enctypes on arm64
systems with CONFIG_VMAP_STACK enabled.  It turns out there still a few
places using stack memory with scatterlists, causing krb5_encrypt() and
krb5_decrypt() to produce incorrect results (or a BUG if CONFIG_DEBUG_SG
is enabled).

Tested with cthon on v4.0/v4.1/v4.2 with krb5/krb5i/krb5p using
des3-cbc-sha1 and arcfour-hmac-md5.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: J. Bruce Fields <bfields@redhat.com>

Conflicts:
	net/sunrpc/auth_gss/gss_krb5_seqnum.c
---
 net/sunrpc/auth_gss/gss_krb5_seqnum.c | 49 +++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 11 deletions(-)

diff --git a/net/sunrpc/auth_gss/gss_krb5_seqnum.c b/net/sunrpc/auth_gss/gss_krb5_seqnum.c
index c8b9082..2d2ed67 100644
--- a/net/sunrpc/auth_gss/gss_krb5_seqnum.c
+++ b/net/sunrpc/auth_gss/gss_krb5_seqnum.c
@@ -44,7 +44,7 @@
 		      unsigned char *cksum, unsigned char *buf)
 {
 	struct crypto_skcipher *cipher;
-	unsigned char plain[8];
+	unsigned char *plain;
 	s32 code;
 
 	dprintk("RPC:       %s:\n", __func__);
@@ -53,6 +53,10 @@
 	if (IS_ERR(cipher))
 		return PTR_ERR(cipher);
 
+	plain = kmalloc(8, GFP_NOFS);
+	if (!plain)
+		return -ENOMEM;
+
 	plain[0] = (unsigned char) ((seqnum >> 24) & 0xff);
 	plain[1] = (unsigned char) ((seqnum >> 16) & 0xff);
 	plain[2] = (unsigned char) ((seqnum >> 8) & 0xff);
@@ -69,6 +73,7 @@
 	code = krb5_encrypt(cipher, cksum, plain, buf, 8);
 out:
 	crypto_free_skcipher(cipher);
+	kfree(plain);
 	return code;
 }
 s32
@@ -78,12 +83,17 @@
 		u32 seqnum,
 		unsigned char *cksum, unsigned char *buf)
 {
-	unsigned char plain[8];
+	unsigned char *plain;
+	s32 code;
 
 	if (kctx->enctype == ENCTYPE_ARCFOUR_HMAC)
 		return krb5_make_rc4_seq_num(kctx, direction, seqnum,
 					     cksum, buf);
 
+	plain = kmalloc(8, GFP_NOFS);
+	if (!plain)
+		return -ENOMEM;
+
 	plain[0] = (unsigned char) (seqnum & 0xff);
 	plain[1] = (unsigned char) ((seqnum >> 8) & 0xff);
 	plain[2] = (unsigned char) ((seqnum >> 16) & 0xff);
@@ -94,7 +104,9 @@
 	plain[6] = direction;
 	plain[7] = direction;
 
-	return krb5_encrypt(key, cksum, plain, buf, 8);
+	code = krb5_encrypt(key, cksum, plain, buf, 8);
+	kfree(plain);
+	return code;
 }
 
 static s32
@@ -102,7 +114,7 @@
 		     unsigned char *buf, int *direction, s32 *seqnum)
 {
 	struct crypto_skcipher *cipher;
-	unsigned char plain[8];
+	unsigned char *plain;
 	s32 code;
 
 	dprintk("RPC:       %s:\n", __func__);
@@ -115,20 +127,28 @@
 	if (code)
 		goto out;
 
+	plain = kmalloc(8, GFP_NOFS);
+	if (!plain) {
+		code = -ENOMEM;
+		goto out;
+	}
+
 	code = krb5_decrypt(cipher, cksum, buf, plain, 8);
 	if (code)
-		goto out;
+		goto out_plain;
 
 	if ((plain[4] != plain[5]) || (plain[4] != plain[6])
 				   || (plain[4] != plain[7])) {
 		code = (s32)KG_BAD_SEQ;
-		goto out;
+		goto out_plain;
 	}
 
 	*direction = plain[4];
 
 	*seqnum = ((plain[0] << 24) | (plain[1] << 16) |
 					(plain[2] << 8) | (plain[3]));
+out_plain:
+	kfree(plain);
 out:
 	crypto_free_skcipher(cipher);
 	return code;
@@ -141,26 +161,33 @@
 	       int *direction, u32 *seqnum)
 {
 	s32 code;
-	unsigned char plain[8];
 	struct crypto_skcipher *key = kctx->seq;
+	unsigned char *plain;
 
 	dprintk("RPC:       krb5_get_seq_num:\n");
 
 	if (kctx->enctype == ENCTYPE_ARCFOUR_HMAC)
 		return krb5_get_rc4_seq_num(kctx, cksum, buf,
 					    direction, seqnum);
+	plain = kmalloc(8, GFP_NOFS);
+	if (!plain)
+		return -ENOMEM;
 
 	if ((code = krb5_decrypt(key, cksum, buf, plain, 8)))
-		return code;
+		goto out;
 
 	if ((plain[4] != plain[5]) || (plain[4] != plain[6]) ||
-	    (plain[4] != plain[7]))
-		return (s32)KG_BAD_SEQ;
+	    (plain[4] != plain[7])) {
+		code = (s32)KG_BAD_SEQ;
+		goto out;
+	}
 
 	*direction = plain[4];
 
 	*seqnum = ((plain[0]) |
 		   (plain[1] << 8) | (plain[2] << 16) | (plain[3] << 24));
 
-	return 0;
+out:
+	kfree(plain);
+	return code;
 }
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH 4.19] sunrpc: fix 4 more call sites that were using stack memory with a scatterlist
  2019-02-19 19:36 [PATCH 4.19] sunrpc: fix 4 more call sites that were using stack memory with a scatterlist Scott Mayhew
@ 2019-02-19 21:38 ` J. Bruce Fields
  2019-02-21 12:23 ` Greg KH
  1 sibling, 0 replies; 3+ messages in thread
From: J. Bruce Fields @ 2019-02-19 21:38 UTC (permalink / raw)
  To: Scott Mayhew; +Cc: stable

ACK.--b.

On Tue, Feb 19, 2019 at 02:36:27PM -0500, Scott Mayhew wrote:
> (cherry picked from commit e7afe6c1d486b516ed586dcc10b3e7e3e85a9c2b)
> 
> While trying to reproduce a reported kernel panic on arm64, I discovered
> that AUTH_GSS basically doesn't work at all with older enctypes on arm64
> systems with CONFIG_VMAP_STACK enabled.  It turns out there still a few
> places using stack memory with scatterlists, causing krb5_encrypt() and
> krb5_decrypt() to produce incorrect results (or a BUG if CONFIG_DEBUG_SG
> is enabled).
> 
> Tested with cthon on v4.0/v4.1/v4.2 with krb5/krb5i/krb5p using
> des3-cbc-sha1 and arcfour-hmac-md5.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> 
> Conflicts:
> 	net/sunrpc/auth_gss/gss_krb5_seqnum.c
> ---
>  net/sunrpc/auth_gss/gss_krb5_seqnum.c | 49 +++++++++++++++++++++++++++--------
>  1 file changed, 38 insertions(+), 11 deletions(-)
> 
> diff --git a/net/sunrpc/auth_gss/gss_krb5_seqnum.c b/net/sunrpc/auth_gss/gss_krb5_seqnum.c
> index c8b9082..2d2ed67 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_seqnum.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_seqnum.c
> @@ -44,7 +44,7 @@
>  		      unsigned char *cksum, unsigned char *buf)
>  {
>  	struct crypto_skcipher *cipher;
> -	unsigned char plain[8];
> +	unsigned char *plain;
>  	s32 code;
>  
>  	dprintk("RPC:       %s:\n", __func__);
> @@ -53,6 +53,10 @@
>  	if (IS_ERR(cipher))
>  		return PTR_ERR(cipher);
>  
> +	plain = kmalloc(8, GFP_NOFS);
> +	if (!plain)
> +		return -ENOMEM;
> +
>  	plain[0] = (unsigned char) ((seqnum >> 24) & 0xff);
>  	plain[1] = (unsigned char) ((seqnum >> 16) & 0xff);
>  	plain[2] = (unsigned char) ((seqnum >> 8) & 0xff);
> @@ -69,6 +73,7 @@
>  	code = krb5_encrypt(cipher, cksum, plain, buf, 8);
>  out:
>  	crypto_free_skcipher(cipher);
> +	kfree(plain);
>  	return code;
>  }
>  s32
> @@ -78,12 +83,17 @@
>  		u32 seqnum,
>  		unsigned char *cksum, unsigned char *buf)
>  {
> -	unsigned char plain[8];
> +	unsigned char *plain;
> +	s32 code;
>  
>  	if (kctx->enctype == ENCTYPE_ARCFOUR_HMAC)
>  		return krb5_make_rc4_seq_num(kctx, direction, seqnum,
>  					     cksum, buf);
>  
> +	plain = kmalloc(8, GFP_NOFS);
> +	if (!plain)
> +		return -ENOMEM;
> +
>  	plain[0] = (unsigned char) (seqnum & 0xff);
>  	plain[1] = (unsigned char) ((seqnum >> 8) & 0xff);
>  	plain[2] = (unsigned char) ((seqnum >> 16) & 0xff);
> @@ -94,7 +104,9 @@
>  	plain[6] = direction;
>  	plain[7] = direction;
>  
> -	return krb5_encrypt(key, cksum, plain, buf, 8);
> +	code = krb5_encrypt(key, cksum, plain, buf, 8);
> +	kfree(plain);
> +	return code;
>  }
>  
>  static s32
> @@ -102,7 +114,7 @@
>  		     unsigned char *buf, int *direction, s32 *seqnum)
>  {
>  	struct crypto_skcipher *cipher;
> -	unsigned char plain[8];
> +	unsigned char *plain;
>  	s32 code;
>  
>  	dprintk("RPC:       %s:\n", __func__);
> @@ -115,20 +127,28 @@
>  	if (code)
>  		goto out;
>  
> +	plain = kmalloc(8, GFP_NOFS);
> +	if (!plain) {
> +		code = -ENOMEM;
> +		goto out;
> +	}
> +
>  	code = krb5_decrypt(cipher, cksum, buf, plain, 8);
>  	if (code)
> -		goto out;
> +		goto out_plain;
>  
>  	if ((plain[4] != plain[5]) || (plain[4] != plain[6])
>  				   || (plain[4] != plain[7])) {
>  		code = (s32)KG_BAD_SEQ;
> -		goto out;
> +		goto out_plain;
>  	}
>  
>  	*direction = plain[4];
>  
>  	*seqnum = ((plain[0] << 24) | (plain[1] << 16) |
>  					(plain[2] << 8) | (plain[3]));
> +out_plain:
> +	kfree(plain);
>  out:
>  	crypto_free_skcipher(cipher);
>  	return code;
> @@ -141,26 +161,33 @@
>  	       int *direction, u32 *seqnum)
>  {
>  	s32 code;
> -	unsigned char plain[8];
>  	struct crypto_skcipher *key = kctx->seq;
> +	unsigned char *plain;
>  
>  	dprintk("RPC:       krb5_get_seq_num:\n");
>  
>  	if (kctx->enctype == ENCTYPE_ARCFOUR_HMAC)
>  		return krb5_get_rc4_seq_num(kctx, cksum, buf,
>  					    direction, seqnum);
> +	plain = kmalloc(8, GFP_NOFS);
> +	if (!plain)
> +		return -ENOMEM;
>  
>  	if ((code = krb5_decrypt(key, cksum, buf, plain, 8)))
> -		return code;
> +		goto out;
>  
>  	if ((plain[4] != plain[5]) || (plain[4] != plain[6]) ||
> -	    (plain[4] != plain[7]))
> -		return (s32)KG_BAD_SEQ;
> +	    (plain[4] != plain[7])) {
> +		code = (s32)KG_BAD_SEQ;
> +		goto out;
> +	}
>  
>  	*direction = plain[4];
>  
>  	*seqnum = ((plain[0]) |
>  		   (plain[1] << 8) | (plain[2] << 16) | (plain[3] << 24));
>  
> -	return 0;
> +out:
> +	kfree(plain);
> +	return code;
>  }
> -- 
> 1.8.3.1
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH 4.19] sunrpc: fix 4 more call sites that were using stack memory with a scatterlist
  2019-02-19 19:36 [PATCH 4.19] sunrpc: fix 4 more call sites that were using stack memory with a scatterlist Scott Mayhew
  2019-02-19 21:38 ` J. Bruce Fields
@ 2019-02-21 12:23 ` Greg KH
  1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2019-02-21 12:23 UTC (permalink / raw)
  To: Scott Mayhew; +Cc: stable, bfields

On Tue, Feb 19, 2019 at 02:36:27PM -0500, Scott Mayhew wrote:
> (cherry picked from commit e7afe6c1d486b516ed586dcc10b3e7e3e85a9c2b)
> 
> While trying to reproduce a reported kernel panic on arm64, I discovered
> that AUTH_GSS basically doesn't work at all with older enctypes on arm64
> systems with CONFIG_VMAP_STACK enabled.  It turns out there still a few
> places using stack memory with scatterlists, causing krb5_encrypt() and
> krb5_decrypt() to produce incorrect results (or a BUG if CONFIG_DEBUG_SG
> is enabled).
> 
> Tested with cthon on v4.0/v4.1/v4.2 with krb5/krb5i/krb5p using
> des3-cbc-sha1 and arcfour-hmac-md5.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> 
> Conflicts:
> 	net/sunrpc/auth_gss/gss_krb5_seqnum.c

These two lines are not needed :)

Anyway, thanks for the backport, now queued up for both trees.

greg k-h

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-02-21 12:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-19 19:36 [PATCH 4.19] sunrpc: fix 4 more call sites that were using stack memory with a scatterlist Scott Mayhew
2019-02-19 21:38 ` J. Bruce Fields
2019-02-21 12:23 ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.