[PATCH] Fix BUG() in calc_seckey()

[PATCH] Fix BUG() in calc_seckey()

[Sachin Prabhu]

Andy Lutromirski's new virtually mapped kernel stack allocations moves
kernel stacks the vmalloc area. This triggers the bug
 kernel BUG at ./include/linux/scatterlist.h:140!
at calc_seckey()->sg_init()

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsencrypt.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index 8347c90..5eb0412 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -808,7 +808,11 @@ calc_seckey(struct cifs_ses *ses)
 	struct crypto_skcipher *tfm_arc4;
 	struct scatterlist sgin, sgout;
 	struct skcipher_request *req;
-	unsigned char sec_key[CIFS_SESS_KEY_SIZE]; /* a nonce */
+	unsigned char *sec_key;
+
+	sec_key = kmalloc(CIFS_SESS_KEY_SIZE, GFP_KERNEL);
+	if (sec_key == NULL)
+		return -ENOMEM;
 
 	get_random_bytes(sec_key, CIFS_SESS_KEY_SIZE);
 
@@ -816,7 +820,7 @@ calc_seckey(struct cifs_ses *ses)
 	if (IS_ERR(tfm_arc4)) {
 		rc = PTR_ERR(tfm_arc4);
 		cifs_dbg(VFS, "could not allocate crypto API arc4\n");
-		return rc;
+		goto out;
 	}
 
 	rc = crypto_skcipher_setkey(tfm_arc4, ses->auth_key.response,
@@ -854,7 +858,8 @@ calc_seckey(struct cifs_ses *ses)
 
 out_free_cipher:
 	crypto_free_skcipher(tfm_arc4);
-
+out:
+	kfree(sec_key);
 	return rc;
 }
 
-- 
2.7.4

Re: [PATCH] Fix BUG() in calc_seckey()

[Sachin Prabhu]

A bit more reference for this patch

Information about the patch series which seems to have caused this
problem is available at

http://lwn.net/Articles/692208/

For the reasons described in the article, the kernel now allocates the
stack from the vmalloc region. The buffers used in calc_seckey() are
therefore now situated in the vmalloc-ed region. This triggers the
check in 

static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
                              unsigned int buflen)
{
#ifdef CONFIG_DEBUG_SG
        BUG_ON(!virt_addr_valid(buf));
#endif
..
}

This issue was also addressed in Linus's release notes at
http://lwn.net/Articles/703664/

"
The virtual stack mapping also happens to mean that people who try to
do DMA from temporary buffers on the stack ("Don't do it!") now really
need to change their evil ways. So there is some fallout from this,
and I expect a couple of drivers to need minor fixes. But it's all for
a good cause, really (and it isn't all that common, because doing DMA
from the stack really has never been a good idea, and is generally not
even workable in most situations).
"

Sachin Prabhu

On Mon, 2016-10-17 at 16:40 -0400, Sachin Prabhu wrote:
> Andy Lutromirski's new virtually mapped kernel stack allocations
> moves
> kernel stacks the vmalloc area. This triggers the bug
>  kernel BUG at ./include/linux/scatterlist.h:140!
> at calc_seckey()->sg_init()
> 
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsencrypt.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index 8347c90..5eb0412 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -808,7 +808,11 @@ calc_seckey(struct cifs_ses *ses)
>  	struct crypto_skcipher *tfm_arc4;
>  	struct scatterlist sgin, sgout;
>  	struct skcipher_request *req;
> -	unsigned char sec_key[CIFS_SESS_KEY_SIZE]; /* a nonce */
> +	unsigned char *sec_key;
> +
> +	sec_key = kmalloc(CIFS_SESS_KEY_SIZE, GFP_KERNEL);
> +	if (sec_key == NULL)
> +		return -ENOMEM;
>  
>  	get_random_bytes(sec_key, CIFS_SESS_KEY_SIZE);
>  
> @@ -816,7 +820,7 @@ calc_seckey(struct cifs_ses *ses)
>  	if (IS_ERR(tfm_arc4)) {
>  		rc = PTR_ERR(tfm_arc4);
>  		cifs_dbg(VFS, "could not allocate crypto API
> arc4\n");
> -		return rc;
> +		goto out;
>  	}
>  
>  	rc = crypto_skcipher_setkey(tfm_arc4, ses-
> >auth_key.response,
> @@ -854,7 +858,8 @@ calc_seckey(struct cifs_ses *ses)
>  
>  out_free_cipher:
>  	crypto_free_skcipher(tfm_arc4);
> -
> +out:
> +	kfree(sec_key);
>  	return rc;
>  }
>  

Re: [PATCH] Fix BUG() in calc_seckey()

[Jeff Layton]

On Mon, 2016-10-17 at 16:40 -0400, Sachin Prabhu wrote:
> Andy Lutromirski's new virtually mapped kernel stack allocations moves
> kernel stacks the vmalloc area. This triggers the bug
>  kernel BUG at ./include/linux/scatterlist.h:140!
> at calc_seckey()->sg_init()
> 
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsencrypt.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index 8347c90..5eb0412 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -808,7 +808,11 @@ calc_seckey(struct cifs_ses *ses)
>  	struct crypto_skcipher *tfm_arc4;
>  	struct scatterlist sgin, sgout;
>  	struct skcipher_request *req;
> -	unsigned char sec_key[CIFS_SESS_KEY_SIZE]; /* a nonce */
> +	unsigned char *sec_key;
> +
> +	sec_key = kmalloc(CIFS_SESS_KEY_SIZE, GFP_KERNEL);
> +	if (sec_key == NULL)
> +		return -ENOMEM;
>  
>  	get_random_bytes(sec_key, CIFS_SESS_KEY_SIZE);
>  
> @@ -816,7 +820,7 @@ calc_seckey(struct cifs_ses *ses)
>  	if (IS_ERR(tfm_arc4)) {
>  		rc = PTR_ERR(tfm_arc4);
>  		cifs_dbg(VFS, "could not allocate crypto API arc4\n");
> -		return rc;
> +		goto out;
>  	}
>  
>  	rc = crypto_skcipher_setkey(tfm_arc4, ses->auth_key.response,
> @@ -854,7 +858,8 @@ calc_seckey(struct cifs_ses *ses)
>  
>  out_free_cipher:
>  	crypto_free_skcipher(tfm_arc4);
> -
> +out:
> +	kfree(sec_key);
>  	return rc;
>  }
>  

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>