diff mbox

[Xenial,SRU] crypto: vmx - fix null dereference in p8_aes_xts_crypt

Message ID 1474730255-21600-1-git-send-email-tim.gardner@canonical.com
State New
Headers show

Commit Message

Tim Gardner Sept. 24, 2016, 3:17 p.m. UTC
From: Li Zhong <zhong@linux.vnet.ibm.com>

BugLink: http://bugs.launchpad.net/bugs/1613295

walk.iv is not assigned a value in blkcipher_walk_init. It makes iv uninitialized.
It is possibly a null value(as shown below), which is then used by aes_p8_encrypt.

This patch moves iv = walk.iv after blkcipher_walk_virt, in which walk.iv is set.

[17856.268050] Unable to handle kernel paging request for data at address 0x00000000
[17856.268212] Faulting instruction address: 0xd000000002ff04bc
7:mon> t
[link register   ] d000000002ff47b8 p8_aes_xts_crypt+0x168/0x2a0 [vmx_crypto]   (938)
[c000000013b77960] d000000002ff4794 p8_aes_xts_crypt+0x144/0x2a0 [vmx_crypto] (unreliable)
[c000000013b77a70] c000000000544d64 skcipher_decrypt_blkcipher+0x64/0x80
[c000000013b77ac0] d000000003c0175c crypt_convert+0x53c/0x620 [dm_crypt]
[c000000013b77ba0] d000000003c043fc kcryptd_crypt+0x3cc/0x440 [dm_crypt]
[c000000013b77c50] c0000000000f3070 process_one_work+0x1e0/0x590
[c000000013b77ce0] c0000000000f34c8 worker_thread+0xa8/0x660
[c000000013b77d80] c0000000000fc0b0 kthread+0x110/0x130
[c000000013b77e30] c0000000000098f0 ret_from_kernel_thread+0x5c/0x6c

Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
(cherry picked from commit 901d3d4fee83e9407d91e7178048e2fed6c91f6b)
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 drivers/crypto/vmx/aes_xts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Chris J Arges Sept. 25, 2016, 1:02 a.m. UTC | #1
On Sat, Sep 24, 2016 at 09:17:35AM -0600, Tim Gardner wrote:
> From: Li Zhong <zhong@linux.vnet.ibm.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1613295
> 
> walk.iv is not assigned a value in blkcipher_walk_init. It makes iv uninitialized.
> It is possibly a null value(as shown below), which is then used by aes_p8_encrypt.
> 
> This patch moves iv = walk.iv after blkcipher_walk_virt, in which walk.iv is set.
> 
> [17856.268050] Unable to handle kernel paging request for data at address 0x00000000
> [17856.268212] Faulting instruction address: 0xd000000002ff04bc
> 7:mon> t
> [link register   ] d000000002ff47b8 p8_aes_xts_crypt+0x168/0x2a0 [vmx_crypto]   (938)
> [c000000013b77960] d000000002ff4794 p8_aes_xts_crypt+0x144/0x2a0 [vmx_crypto] (unreliable)
> [c000000013b77a70] c000000000544d64 skcipher_decrypt_blkcipher+0x64/0x80
> [c000000013b77ac0] d000000003c0175c crypt_convert+0x53c/0x620 [dm_crypt]
> [c000000013b77ba0] d000000003c043fc kcryptd_crypt+0x3cc/0x440 [dm_crypt]
> [c000000013b77c50] c0000000000f3070 process_one_work+0x1e0/0x590
> [c000000013b77ce0] c0000000000f34c8 worker_thread+0xa8/0x660
> [c000000013b77d80] c0000000000fc0b0 kthread+0x110/0x130
> [c000000013b77e30] c0000000000098f0 ret_from_kernel_thread+0x5c/0x6c
> 
> Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> (cherry picked from commit 901d3d4fee83e9407d91e7178048e2fed6c91f6b)
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  drivers/crypto/vmx/aes_xts.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/vmx/aes_xts.c b/drivers/crypto/vmx/aes_xts.c
> index cfb2541..24353ec3 100644
> --- a/drivers/crypto/vmx/aes_xts.c
> +++ b/drivers/crypto/vmx/aes_xts.c
> @@ -129,8 +129,8 @@ static int p8_aes_xts_crypt(struct blkcipher_desc *desc,
>  
>  		blkcipher_walk_init(&walk, dst, src, nbytes);
>  
> -		iv = (u8 *)walk.iv;
>  		ret = blkcipher_walk_virt(desc, &walk);
> +		iv = walk.iv;
>  		memset(tweak, 0, AES_BLOCK_SIZE);
>  		aes_p8_encrypt(iv, tweak, &ctx->tweak_key);
>  
> -- 
> 2.7.4
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Colin Ian King Sept. 25, 2016, 1:06 a.m. UTC | #2
On 24/09/16 08:17, Tim Gardner wrote:
> From: Li Zhong <zhong@linux.vnet.ibm.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1613295
> 
> walk.iv is not assigned a value in blkcipher_walk_init. It makes iv uninitialized.
> It is possibly a null value(as shown below), which is then used by aes_p8_encrypt.
> 
> This patch moves iv = walk.iv after blkcipher_walk_virt, in which walk.iv is set.
> 
> [17856.268050] Unable to handle kernel paging request for data at address 0x00000000
> [17856.268212] Faulting instruction address: 0xd000000002ff04bc
> 7:mon> t
> [link register   ] d000000002ff47b8 p8_aes_xts_crypt+0x168/0x2a0 [vmx_crypto]   (938)
> [c000000013b77960] d000000002ff4794 p8_aes_xts_crypt+0x144/0x2a0 [vmx_crypto] (unreliable)
> [c000000013b77a70] c000000000544d64 skcipher_decrypt_blkcipher+0x64/0x80
> [c000000013b77ac0] d000000003c0175c crypt_convert+0x53c/0x620 [dm_crypt]
> [c000000013b77ba0] d000000003c043fc kcryptd_crypt+0x3cc/0x440 [dm_crypt]
> [c000000013b77c50] c0000000000f3070 process_one_work+0x1e0/0x590
> [c000000013b77ce0] c0000000000f34c8 worker_thread+0xa8/0x660
> [c000000013b77d80] c0000000000fc0b0 kthread+0x110/0x130
> [c000000013b77e30] c0000000000098f0 ret_from_kernel_thread+0x5c/0x6c
> 
> Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> (cherry picked from commit 901d3d4fee83e9407d91e7178048e2fed6c91f6b)
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  drivers/crypto/vmx/aes_xts.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/vmx/aes_xts.c b/drivers/crypto/vmx/aes_xts.c
> index cfb2541..24353ec3 100644
> --- a/drivers/crypto/vmx/aes_xts.c
> +++ b/drivers/crypto/vmx/aes_xts.c
> @@ -129,8 +129,8 @@ static int p8_aes_xts_crypt(struct blkcipher_desc *desc,
>  
>  		blkcipher_walk_init(&walk, dst, src, nbytes);
>  
> -		iv = (u8 *)walk.iv;
>  		ret = blkcipher_walk_virt(desc, &walk);
> +		iv = walk.iv;
>  		memset(tweak, 0, AES_BLOCK_SIZE);
>  		aes_p8_encrypt(iv, tweak, &ctx->tweak_key);
>  
> 
Upstream cherry pick and seems to cleanly solve the issue.

Acked-by: Colin Ian King <colin.king@canonical.com>
Tim Gardner Sept. 30, 2016, 4:20 p.m. UTC | #3

diff mbox

Patch

diff --git a/drivers/crypto/vmx/aes_xts.c b/drivers/crypto/vmx/aes_xts.c
index cfb2541..24353ec3 100644
--- a/drivers/crypto/vmx/aes_xts.c
+++ b/drivers/crypto/vmx/aes_xts.c
@@ -129,8 +129,8 @@  static int p8_aes_xts_crypt(struct blkcipher_desc *desc,
 
 		blkcipher_walk_init(&walk, dst, src, nbytes);
 
-		iv = (u8 *)walk.iv;
 		ret = blkcipher_walk_virt(desc, &walk);
+		iv = walk.iv;
 		memset(tweak, 0, AES_BLOCK_SIZE);
 		aes_p8_encrypt(iv, tweak, &ctx->tweak_key);