linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] sunrpc: Don't use stack buffer with scatterlist
@ 2018-08-17 21:43 Laura Abbott
  2018-08-20 18:50 ` J. Bruce Fields
  0 siblings, 1 reply; 2+ messages in thread
From: Laura Abbott @ 2018-08-17 21:43 UTC (permalink / raw)
  To: J. Bruce Fields, Jeff Layton, Trond Myklebust, Anna Schumaker
  Cc: Laura Abbott, David S. Miller, linux-nfs, netdev, linux-kernel

Fedora got a bug report from NFS:

kernel BUG at include/linux/scatterlist.h:143!
...
RIP: 0010:sg_init_one+0x7d/0x90
..
  make_checksum+0x4e7/0x760 [rpcsec_gss_krb5]
  gss_get_mic_kerberos+0x26e/0x310 [rpcsec_gss_krb5]
  gss_marshal+0x126/0x1a0 [auth_rpcgss]
  ? __local_bh_enable_ip+0x80/0xe0
  ? call_transmit_status+0x1d0/0x1d0 [sunrpc]
  call_transmit+0x137/0x230 [sunrpc]
  __rpc_execute+0x9b/0x490 [sunrpc]
  rpc_run_task+0x119/0x150 [sunrpc]
  nfs4_run_exchange_id+0x1bd/0x250 [nfsv4]
  _nfs4_proc_exchange_id+0x2d/0x490 [nfsv4]
  nfs41_discover_server_trunking+0x1c/0xa0 [nfsv4]
  nfs4_discover_server_trunking+0x80/0x270 [nfsv4]
  nfs4_init_client+0x16e/0x240 [nfsv4]
  ? nfs_get_client+0x4c9/0x5d0 [nfs]
  ? _raw_spin_unlock+0x24/0x30
  ? nfs_get_client+0x4c9/0x5d0 [nfs]
  nfs4_set_client+0xb2/0x100 [nfsv4]
  nfs4_create_server+0xff/0x290 [nfsv4]
  nfs4_remote_mount+0x28/0x50 [nfsv4]
  mount_fs+0x3b/0x16a
  vfs_kern_mount.part.35+0x54/0x160
  nfs_do_root_mount+0x7f/0xc0 [nfsv4]
  nfs4_try_mount+0x43/0x70 [nfsv4]
  ? get_nfs_version+0x21/0x80 [nfs]
  nfs_fs_mount+0x789/0xbf0 [nfs]
  ? pcpu_alloc+0x6ca/0x7e0
  ? nfs_clone_super+0x70/0x70 [nfs]
  ? nfs_parse_mount_options+0xb40/0xb40 [nfs]
  mount_fs+0x3b/0x16a
  vfs_kern_mount.part.35+0x54/0x160
  do_mount+0x1fd/0xd50
  ksys_mount+0xba/0xd0
  __x64_sys_mount+0x21/0x30
  do_syscall_64+0x60/0x1f0
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

This is BUG_ON(!virt_addr_valid(buf)) triggered by using a stack
allocated buffer with a scatterlist. Convert the buffer for
rc4salt to be dynamically allocated instead.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1615258
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
Compile tested only.
---
 net/sunrpc/auth_gss/gss_krb5_crypto.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/sunrpc/auth_gss/gss_krb5_crypto.c b/net/sunrpc/auth_gss/gss_krb5_crypto.c
index 8654494b4d0a..834eb2b9e41b 100644
--- a/net/sunrpc/auth_gss/gss_krb5_crypto.c
+++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c
@@ -169,7 +169,7 @@ make_checksum_hmac_md5(struct krb5_ctx *kctx, char *header, int hdrlen,
 	struct scatterlist              sg[1];
 	int err = -1;
 	u8 *checksumdata;
-	u8 rc4salt[4];
+	u8 *rc4salt;
 	struct crypto_ahash *md5;
 	struct crypto_ahash *hmac_md5;
 	struct ahash_request *req;
@@ -183,14 +183,18 @@ make_checksum_hmac_md5(struct krb5_ctx *kctx, char *header, int hdrlen,
 		return GSS_S_FAILURE;
 	}
 
+	rc4salt = kmalloc_array(4, sizeof(*rc4salt), GFP_NOFS);
+	if (!rc4salt)
+		return GSS_S_FAILURE;
+
 	if (arcfour_hmac_md5_usage_to_salt(usage, rc4salt)) {
 		dprintk("%s: invalid usage value %u\n", __func__, usage);
-		return GSS_S_FAILURE;
+		goto out_free_rc4salt;
 	}
 
 	checksumdata = kmalloc(GSS_KRB5_MAX_CKSUM_LEN, GFP_NOFS);
 	if (!checksumdata)
-		return GSS_S_FAILURE;
+		goto out_free_rc4salt;
 
 	md5 = crypto_alloc_ahash("md5", 0, CRYPTO_ALG_ASYNC);
 	if (IS_ERR(md5))
@@ -258,6 +262,8 @@ make_checksum_hmac_md5(struct krb5_ctx *kctx, char *header, int hdrlen,
 	crypto_free_ahash(md5);
 out_free_cksum:
 	kfree(checksumdata);
+out_free_rc4salt:
+	kfree(rc4salt);
 	return err ? GSS_S_FAILURE : 0;
 }
 
-- 
2.17.1


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

* Re: [PATCH] sunrpc: Don't use stack buffer with scatterlist
  2018-08-17 21:43 [PATCH] sunrpc: Don't use stack buffer with scatterlist Laura Abbott
@ 2018-08-20 18:50 ` J. Bruce Fields
  0 siblings, 0 replies; 2+ messages in thread
From: J. Bruce Fields @ 2018-08-20 18:50 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Jeff Layton, Trond Myklebust, Anna Schumaker, David S. Miller,
	linux-nfs, netdev, linux-kernel

On Fri, Aug 17, 2018 at 02:43:54PM -0700, Laura Abbott wrote:
> Fedora got a bug report from NFS:

Thanks, applying (unless Trond or Anna gets it).

--b.

> 
> kernel BUG at include/linux/scatterlist.h:143!
> ...
> RIP: 0010:sg_init_one+0x7d/0x90
> ..
>   make_checksum+0x4e7/0x760 [rpcsec_gss_krb5]
>   gss_get_mic_kerberos+0x26e/0x310 [rpcsec_gss_krb5]
>   gss_marshal+0x126/0x1a0 [auth_rpcgss]
>   ? __local_bh_enable_ip+0x80/0xe0
>   ? call_transmit_status+0x1d0/0x1d0 [sunrpc]
>   call_transmit+0x137/0x230 [sunrpc]
>   __rpc_execute+0x9b/0x490 [sunrpc]
>   rpc_run_task+0x119/0x150 [sunrpc]
>   nfs4_run_exchange_id+0x1bd/0x250 [nfsv4]
>   _nfs4_proc_exchange_id+0x2d/0x490 [nfsv4]
>   nfs41_discover_server_trunking+0x1c/0xa0 [nfsv4]
>   nfs4_discover_server_trunking+0x80/0x270 [nfsv4]
>   nfs4_init_client+0x16e/0x240 [nfsv4]
>   ? nfs_get_client+0x4c9/0x5d0 [nfs]
>   ? _raw_spin_unlock+0x24/0x30
>   ? nfs_get_client+0x4c9/0x5d0 [nfs]
>   nfs4_set_client+0xb2/0x100 [nfsv4]
>   nfs4_create_server+0xff/0x290 [nfsv4]
>   nfs4_remote_mount+0x28/0x50 [nfsv4]
>   mount_fs+0x3b/0x16a
>   vfs_kern_mount.part.35+0x54/0x160
>   nfs_do_root_mount+0x7f/0xc0 [nfsv4]
>   nfs4_try_mount+0x43/0x70 [nfsv4]
>   ? get_nfs_version+0x21/0x80 [nfs]
>   nfs_fs_mount+0x789/0xbf0 [nfs]
>   ? pcpu_alloc+0x6ca/0x7e0
>   ? nfs_clone_super+0x70/0x70 [nfs]
>   ? nfs_parse_mount_options+0xb40/0xb40 [nfs]
>   mount_fs+0x3b/0x16a
>   vfs_kern_mount.part.35+0x54/0x160
>   do_mount+0x1fd/0xd50
>   ksys_mount+0xba/0xd0
>   __x64_sys_mount+0x21/0x30
>   do_syscall_64+0x60/0x1f0
>   entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> This is BUG_ON(!virt_addr_valid(buf)) triggered by using a stack
> allocated buffer with a scatterlist. Convert the buffer for
> rc4salt to be dynamically allocated instead.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1615258
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> Compile tested only.
> ---
>  net/sunrpc/auth_gss/gss_krb5_crypto.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/net/sunrpc/auth_gss/gss_krb5_crypto.c b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> index 8654494b4d0a..834eb2b9e41b 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_crypto.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> @@ -169,7 +169,7 @@ make_checksum_hmac_md5(struct krb5_ctx *kctx, char *header, int hdrlen,
>  	struct scatterlist              sg[1];
>  	int err = -1;
>  	u8 *checksumdata;
> -	u8 rc4salt[4];
> +	u8 *rc4salt;
>  	struct crypto_ahash *md5;
>  	struct crypto_ahash *hmac_md5;
>  	struct ahash_request *req;
> @@ -183,14 +183,18 @@ make_checksum_hmac_md5(struct krb5_ctx *kctx, char *header, int hdrlen,
>  		return GSS_S_FAILURE;
>  	}
>  
> +	rc4salt = kmalloc_array(4, sizeof(*rc4salt), GFP_NOFS);
> +	if (!rc4salt)
> +		return GSS_S_FAILURE;
> +
>  	if (arcfour_hmac_md5_usage_to_salt(usage, rc4salt)) {
>  		dprintk("%s: invalid usage value %u\n", __func__, usage);
> -		return GSS_S_FAILURE;
> +		goto out_free_rc4salt;
>  	}
>  
>  	checksumdata = kmalloc(GSS_KRB5_MAX_CKSUM_LEN, GFP_NOFS);
>  	if (!checksumdata)
> -		return GSS_S_FAILURE;
> +		goto out_free_rc4salt;
>  
>  	md5 = crypto_alloc_ahash("md5", 0, CRYPTO_ALG_ASYNC);
>  	if (IS_ERR(md5))
> @@ -258,6 +262,8 @@ make_checksum_hmac_md5(struct krb5_ctx *kctx, char *header, int hdrlen,
>  	crypto_free_ahash(md5);
>  out_free_cksum:
>  	kfree(checksumdata);
> +out_free_rc4salt:
> +	kfree(rc4salt);
>  	return err ? GSS_S_FAILURE : 0;
>  }
>  
> -- 
> 2.17.1

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

end of thread, other threads:[~2018-08-20 18:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-17 21:43 [PATCH] sunrpc: Don't use stack buffer with scatterlist Laura Abbott
2018-08-20 18:50 ` J. Bruce Fields

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).