Re: [PATCH] crypto: ccp - Fix XTS-AES support on a version 5 CCP

2017-08-18 Thread Gary R Hook 

On 08/18/2017 11:19 AM, Gary R Hook wrote:

On 07/17/2017 04:48 PM, Lendacky, Thomas wrote:

On 7/17/2017 3:08 PM, Gary R Hook wrote:

Version 5 CCPs have differing requirements for XTS-AES: key components
are stored in a 512-bit vector. The context must be little-endian
justified. AES-256 is supported now, so propagate the cipher size to
the command descriptor.

Signed-off-by: Gary R Hook 


Herbert,

I see that this patch (and others that add function or fix bugs) has
been added to
cryptodev. I've not seen anything CCP-related pushed to Linux in a
while, however.

Is there something more I need to do to for these recent patches, to get
them promoted
to the mainline kernel..?


Please ignore. I realized (too late) that the referenced items missed 
the merge window

for 4.13.




Thanks much!
Gary



---
  drivers/crypto/ccp/ccp-crypto-aes-xts.c |   79 ---
  drivers/crypto/ccp/ccp-dev-v5.c |2 +
  drivers/crypto/ccp/ccp-dev.h|2 +
  drivers/crypto/ccp/ccp-ops.c|   56 ++
  4 files changed, 89 insertions(+), 50 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c 
b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 58a4244b4752..8d248b198e22 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -1,7 +1,7 @@
  /*
   * AMD Cryptographic Coprocessor (CCP) AES XTS crypto API support
   *
- * Copyright (C) 2013 Advanced Micro Devices, Inc.
+ * Copyright (C) 2013,2017 Advanced Micro Devices, Inc.
   *
   * Author: Tom Lendacky 
   *
@@ -37,46 +37,26 @@ struct ccp_unit_size_map {
   u32 value;
  };

-static struct ccp_unit_size_map unit_size_map[] = {
+static struct ccp_unit_size_map xts_unit_sizes[] = {
   {
- .size   = 4096,
- .value  = CCP_XTS_AES_UNIT_SIZE_4096,
- },
- {
- .size   = 2048,
- .value  = CCP_XTS_AES_UNIT_SIZE_2048,
- },
- {
- .size   = 1024,
- .value  = CCP_XTS_AES_UNIT_SIZE_1024,
+ .size   = 16,
+ .value  = CCP_XTS_AES_UNIT_SIZE_16,
   },
   {
- .size   = 512,
+ .size   = 512,
   .value  = CCP_XTS_AES_UNIT_SIZE_512,
   },
   {
- .size   = 256,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
- },
- {
- .size   = 128,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
- },
- {
- .size   = 64,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
- },
- {
- .size   = 32,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
+ .size   = 1024,
+ .value  = CCP_XTS_AES_UNIT_SIZE_1024,
   },
   {
- .size   = 16,
- .value  = CCP_XTS_AES_UNIT_SIZE_16,
+ .size   = 2048,
+ .value  = CCP_XTS_AES_UNIT_SIZE_2048,
   },
   {
- .size   = 1,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
+ .size   = 4096,
+ .value  = CCP_XTS_AES_UNIT_SIZE_4096,
   },
  };


Because of the way the unit size check is performed, you can't delete
the intermediate size checks.  Those must remain so that unit sizes
that aren't supported by the CCP are sent to the fallback mechanism.

Also, re-arranging the order should be a separate patch if that doesn't
really fix anything.



@@ -97,14 +77,20 @@ static int ccp_aes_xts_setkey(struct crypto_ablkcipher 
*tfm, const u8 *key,
 unsigned int key_len)
  {
   struct ccp_ctx *ctx = crypto_tfm_ctx(crypto_ablkcipher_tfm(tfm));
+ unsigned int ccpversion = ccp_version();

   /* Only support 128-bit AES key with a 128-bit Tweak key,
* otherwise use the fallback
*/
+


Remove the addition of the blank line and update the above comment to
indicate the new supported key size added below.


   switch (key_len) {
   case AES_KEYSIZE_128 * 2:
   memcpy(ctx->u.aes.key, key, key_len);
   break;
+ case AES_KEYSIZE_256 * 2:
+ if (ccpversion > CCP_VERSION(3, 0))
+ memcpy(ctx->u.aes.key, key, key_len);
+ break;


Isn't u.aes.key defined with a maximum buffer size of AES_MAX_KEY_SIZE
(which is 32)?  I think this will cause a buffer overrun on memcpy.


   }
   ctx->u.aes.key_len = key_len / 2;
   sg_init_one(>u.aes.key_sg, ctx->u.aes.key, key_len);
@@ -117,7 +103,10 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request 
*req,
  {
   struct ccp_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
   struct ccp_aes_req_ctx *rctx = ablkcipher_request_ctx(req);
+ unsigned int ccpversion = ccp_version();
+ unsigned int fallback = 0;
   unsigned int unit;
+ u32 block_size = 0;
   u32 unit_size;
   int ret;

@@ -131,17 +120,29 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request

Re: [PATCH] crypto: ccp - Fix XTS-AES support on a version 5 CCP

2017-08-18 Thread Gary R Hook 

On 07/17/2017 04:48 PM, Lendacky, Thomas wrote:

On 7/17/2017 3:08 PM, Gary R Hook wrote:

Version 5 CCPs have differing requirements for XTS-AES: key components
are stored in a 512-bit vector. The context must be little-endian
justified. AES-256 is supported now, so propagate the cipher size to
the command descriptor.

Signed-off-by: Gary R Hook 


Herbert,

I see that this patch (and others that add function or fix bugs) has 
been added to
cryptodev. I've not seen anything CCP-related pushed to Linux in a 
while, however.


Is there something more I need to do to for these recent patches, to get 
them promoted

to the mainline kernel..?

Thanks much!
Gary



---
  drivers/crypto/ccp/ccp-crypto-aes-xts.c |   79 ---
  drivers/crypto/ccp/ccp-dev-v5.c |2 +
  drivers/crypto/ccp/ccp-dev.h|2 +
  drivers/crypto/ccp/ccp-ops.c|   56 ++
  4 files changed, 89 insertions(+), 50 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c 
b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 58a4244b4752..8d248b198e22 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -1,7 +1,7 @@
  /*
   * AMD Cryptographic Coprocessor (CCP) AES XTS crypto API support
   *
- * Copyright (C) 2013 Advanced Micro Devices, Inc.
+ * Copyright (C) 2013,2017 Advanced Micro Devices, Inc.
   *
   * Author: Tom Lendacky 
   *
@@ -37,46 +37,26 @@ struct ccp_unit_size_map {
   u32 value;
  };

-static struct ccp_unit_size_map unit_size_map[] = {
+static struct ccp_unit_size_map xts_unit_sizes[] = {
   {
- .size   = 4096,
- .value  = CCP_XTS_AES_UNIT_SIZE_4096,
- },
- {
- .size   = 2048,
- .value  = CCP_XTS_AES_UNIT_SIZE_2048,
- },
- {
- .size   = 1024,
- .value  = CCP_XTS_AES_UNIT_SIZE_1024,
+ .size   = 16,
+ .value  = CCP_XTS_AES_UNIT_SIZE_16,
   },
   {
- .size   = 512,
+ .size   = 512,
   .value  = CCP_XTS_AES_UNIT_SIZE_512,
   },
   {
- .size   = 256,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
- },
- {
- .size   = 128,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
- },
- {
- .size   = 64,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
- },
- {
- .size   = 32,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
+ .size   = 1024,
+ .value  = CCP_XTS_AES_UNIT_SIZE_1024,
   },
   {
- .size   = 16,
- .value  = CCP_XTS_AES_UNIT_SIZE_16,
+ .size   = 2048,
+ .value  = CCP_XTS_AES_UNIT_SIZE_2048,
   },
   {
- .size   = 1,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
+ .size   = 4096,
+ .value  = CCP_XTS_AES_UNIT_SIZE_4096,
   },
  };


Because of the way the unit size check is performed, you can't delete
the intermediate size checks.  Those must remain so that unit sizes
that aren't supported by the CCP are sent to the fallback mechanism.

Also, re-arranging the order should be a separate patch if that doesn't
really fix anything.



@@ -97,14 +77,20 @@ static int ccp_aes_xts_setkey(struct crypto_ablkcipher 
*tfm, const u8 *key,
 unsigned int key_len)
  {
   struct ccp_ctx *ctx = crypto_tfm_ctx(crypto_ablkcipher_tfm(tfm));
+ unsigned int ccpversion = ccp_version();

   /* Only support 128-bit AES key with a 128-bit Tweak key,
* otherwise use the fallback
*/
+


Remove the addition of the blank line and update the above comment to
indicate the new supported key size added below.


   switch (key_len) {
   case AES_KEYSIZE_128 * 2:
   memcpy(ctx->u.aes.key, key, key_len);
   break;
+ case AES_KEYSIZE_256 * 2:
+ if (ccpversion > CCP_VERSION(3, 0))
+ memcpy(ctx->u.aes.key, key, key_len);
+ break;


Isn't u.aes.key defined with a maximum buffer size of AES_MAX_KEY_SIZE
(which is 32)?  I think this will cause a buffer overrun on memcpy.


   }
   ctx->u.aes.key_len = key_len / 2;
   sg_init_one(>u.aes.key_sg, ctx->u.aes.key, key_len);
@@ -117,7 +103,10 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request 
*req,
  {
   struct ccp_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
   struct ccp_aes_req_ctx *rctx = ablkcipher_request_ctx(req);
+ unsigned int ccpversion = ccp_version();
+ unsigned int fallback = 0;
   unsigned int unit;
+ u32 block_size = 0;
   u32 unit_size;
   int ret;

@@ -131,17 +120,29 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request 
*req,
   return -EINVAL;

   unit_size = CCP_XTS_AES_UNIT_SIZE__LAST;
- if (req->nbytes <= unit_size_map[0].size) {


This

Re: [PATCH] crypto: ccp - Fix XTS-AES support on a version 5 CCP

2017-07-21 Thread Gary R Hook 

On 07/17/2017 04:48 PM, Lendacky, Thomas wrote:

On 7/17/2017 3:08 PM, Gary R Hook wrote:

Version 5 CCPs have differing requirements for XTS-AES: key components
are stored in a 512-bit vector. The context must be little-endian
justified. AES-256 is supported now, so propagate the cipher size to
the command descriptor.

Signed-off-by: Gary R Hook 



Look for a version 2


---
  drivers/crypto/ccp/ccp-crypto-aes-xts.c |   79 ---
  drivers/crypto/ccp/ccp-dev-v5.c |2 +
  drivers/crypto/ccp/ccp-dev.h|2 +
  drivers/crypto/ccp/ccp-ops.c|   56 ++
  4 files changed, 89 insertions(+), 50 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c 
b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 58a4244b4752..8d248b198e22 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -1,7 +1,7 @@
  /*
   * AMD Cryptographic Coprocessor (CCP) AES XTS crypto API support
   *
- * Copyright (C) 2013 Advanced Micro Devices, Inc.
+ * Copyright (C) 2013,2017 Advanced Micro Devices, Inc.
   *
   * Author: Tom Lendacky 
   *
@@ -37,46 +37,26 @@ struct ccp_unit_size_map {
   u32 value;
  };

-static struct ccp_unit_size_map unit_size_map[] = {
+static struct ccp_unit_size_map xts_unit_sizes[] = {
   {
- .size   = 4096,
- .value  = CCP_XTS_AES_UNIT_SIZE_4096,
- },
- {
- .size   = 2048,
- .value  = CCP_XTS_AES_UNIT_SIZE_2048,
- },
- {
- .size   = 1024,
- .value  = CCP_XTS_AES_UNIT_SIZE_1024,
+ .size   = 16,
+ .value  = CCP_XTS_AES_UNIT_SIZE_16,
   },
   {
- .size   = 512,
+ .size   = 512,
   .value  = CCP_XTS_AES_UNIT_SIZE_512,
   },
   {
- .size   = 256,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
- },
- {
- .size   = 128,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
- },
- {
- .size   = 64,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
- },
- {
- .size   = 32,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
+ .size   = 1024,
+ .value  = CCP_XTS_AES_UNIT_SIZE_1024,
   },
   {
- .size   = 16,
- .value  = CCP_XTS_AES_UNIT_SIZE_16,
+ .size   = 2048,
+ .value  = CCP_XTS_AES_UNIT_SIZE_2048,
   },
   {
- .size   = 1,
- .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
+ .size   = 4096,
+ .value  = CCP_XTS_AES_UNIT_SIZE_4096,
   },
  };


Because of the way the unit size check is performed, you can't delete
the intermediate size checks.  Those must remain so that unit sizes
that aren't supported by the CCP are sent to the fallback mechanism.


Given the limitations of the CCP (w.r.t. XTS-AES) I thought it more 
clear to look
for only those unit-sizes that are supported, and to use the enumerated 
value

as the index.


Also, re-arranging the order should be a separate patch if that doesn't
really fix anything.


Yes, agreed.



@@ -97,14 +77,20 @@ static int ccp_aes_xts_setkey(struct crypto_ablkcipher 
*tfm, const u8 *key,
 unsigned int key_len)
  {
   struct ccp_ctx *ctx = crypto_tfm_ctx(crypto_ablkcipher_tfm(tfm));
+ unsigned int ccpversion = ccp_version();

   /* Only support 128-bit AES key with a 128-bit Tweak key,
* otherwise use the fallback
*/
+


Remove the addition of the blank line and update the above comment to
indicate the new supported key size added below.


Yes.




   switch (key_len) {
   case AES_KEYSIZE_128 * 2:
   memcpy(ctx->u.aes.key, key, key_len);
   break;
+ case AES_KEYSIZE_256 * 2:
+ if (ccpversion > CCP_VERSION(3, 0))
+ memcpy(ctx->u.aes.key, key, key_len);
+ break;


Isn't u.aes.key defined with a maximum buffer size of AES_MAX_KEY_SIZE
(which is 32)?  I think this will cause a buffer overrun on memcpy.


Yes, the structure member needs to be made larger.




   }
   ctx->u.aes.key_len = key_len / 2;
   sg_init_one(>u.aes.key_sg, ctx->u.aes.key, key_len);
@@ -117,7 +103,10 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request 
*req,
  {
   struct ccp_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
   struct ccp_aes_req_ctx *rctx = ablkcipher_request_ctx(req);
+ unsigned int ccpversion = ccp_version();
+ unsigned int fallback = 0;
   unsigned int unit;
+ u32 block_size = 0;
   u32 unit_size;
   int ret;

@@ -131,17 +120,29 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request 
*req,
   return -EINVAL;

   unit_size = CCP_XTS_AES_UNIT_SIZE__LAST;
- if (req->nbytes <= unit_size_map[0].size) {


This check can't be deleted.  It was added

Re: [PATCH] crypto: ccp - Fix XTS-AES support on a version 5 CCP

2017-07-18 Thread Gary R Hook 

On 07/18/2017 01:28 AM, Stephan Müller wrote:

Am Montag, 17. Juli 2017, 22:08:27 CEST schrieb Gary R Hook:

Hi Gary,


Version 5 CCPs have differing requirements for XTS-AES: key components
are stored in a 512-bit vector. The context must be little-endian
justified. AES-256 is supported now, so propagate the cipher size to
the command descriptor.

Signed-off-by: Gary R Hook 
---
 drivers/crypto/ccp/ccp-crypto-aes-xts.c |   79


..


@@ -97,14 +77,20 @@ static int ccp_aes_xts_setkey(struct crypto_ablkcipher
*tfm, const u8 *key, unsigned int key_len)
 {
   struct ccp_ctx *ctx = crypto_tfm_ctx(crypto_ablkcipher_tfm(tfm));
+ unsigned int ccpversion = ccp_version();

   /* Only support 128-bit AES key with a 128-bit Tweak key,
* otherwise use the fallback
*/
+


Can you please add xts_check_key here?


Certainly!

Re: [PATCH] crypto: ccp - Fix XTS-AES support on a version 5 CCP

2017-07-18 Thread Stephan Müller 
Am Montag, 17. Juli 2017, 22:08:27 CEST schrieb Gary R Hook:

Hi Gary,

> Version 5 CCPs have differing requirements for XTS-AES: key components
> are stored in a 512-bit vector. The context must be little-endian
> justified. AES-256 is supported now, so propagate the cipher size to
> the command descriptor.
> 
> Signed-off-by: Gary R Hook 
> ---
>  drivers/crypto/ccp/ccp-crypto-aes-xts.c |   79
> --- drivers/crypto/ccp/ccp-dev-v5.c |  
>  2 +
>  drivers/crypto/ccp/ccp-dev.h|2 +
>  drivers/crypto/ccp/ccp-ops.c|   56 ++
>  4 files changed, 89 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
> b/drivers/crypto/ccp/ccp-crypto-aes-xts.c index 58a4244b4752..8d248b198e22
> 100644
> --- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
> +++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
> @@ -1,7 +1,7 @@
>  /*
>   * AMD Cryptographic Coprocessor (CCP) AES XTS crypto API support
>   *
> - * Copyright (C) 2013 Advanced Micro Devices, Inc.
> + * Copyright (C) 2013,2017 Advanced Micro Devices, Inc.
>   *
>   * Author: Tom Lendacky 
>   *
> @@ -37,46 +37,26 @@ struct ccp_unit_size_map {
>   u32 value;
>  };
> 
> -static struct ccp_unit_size_map unit_size_map[] = {
> +static struct ccp_unit_size_map xts_unit_sizes[] = {
>   {
> - .size   = 4096,
> - .value  = CCP_XTS_AES_UNIT_SIZE_4096,
> - },
> - {
> - .size   = 2048,
> - .value  = CCP_XTS_AES_UNIT_SIZE_2048,
> - },
> - {
> - .size   = 1024,
> - .value  = CCP_XTS_AES_UNIT_SIZE_1024,
> + .size   = 16,
> + .value  = CCP_XTS_AES_UNIT_SIZE_16,
>   },
>   {
> - .size   = 512,
> + .size   = 512,
>   .value  = CCP_XTS_AES_UNIT_SIZE_512,
>   },
>   {
> - .size   = 256,
> - .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
> - },
> - {
> - .size   = 128,
> - .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
> - },
> - {
> - .size   = 64,
> - .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
> - },
> - {
> - .size   = 32,
> - .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
> + .size   = 1024,
> + .value  = CCP_XTS_AES_UNIT_SIZE_1024,
>   },
>   {
> - .size   = 16,
> - .value  = CCP_XTS_AES_UNIT_SIZE_16,
> + .size   = 2048,
> + .value  = CCP_XTS_AES_UNIT_SIZE_2048,
>   },
>   {
> - .size   = 1,
> - .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
> + .size   = 4096,
> + .value  = CCP_XTS_AES_UNIT_SIZE_4096,
>   },
>  };
> 
> @@ -97,14 +77,20 @@ static int ccp_aes_xts_setkey(struct crypto_ablkcipher
> *tfm, const u8 *key, unsigned int key_len)
>  {
>   struct ccp_ctx *ctx = crypto_tfm_ctx(crypto_ablkcipher_tfm(tfm));
> + unsigned int ccpversion = ccp_version();
> 
>   /* Only support 128-bit AES key with a 128-bit Tweak key,
>* otherwise use the fallback
>*/
> +

Can you please add xts_check_key here?

>   switch (key_len) {
>   case AES_KEYSIZE_128 * 2:
>   memcpy(ctx->u.aes.key, key, key_len);
>   break;
> + case AES_KEYSIZE_256 * 2:
> + if (ccpversion > CCP_VERSION(3, 0))
> + memcpy(ctx->u.aes.key, key, key_len);
> + break;
>   }
>   ctx->u.aes.key_len = key_len / 2;
>   sg_init_one(>u.aes.key_sg, ctx->u.aes.key, key_len);
> @@ -117,7 +103,10 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request
> *req, {
>   struct ccp_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
>   struct ccp_aes_req_ctx *rctx = ablkcipher_request_ctx(req);
> + unsigned int ccpversion = ccp_version();
> + unsigned int fallback = 0;
>   unsigned int unit;
> + u32 block_size = 0;
>   u32 unit_size;
>   int ret;
> 
> @@ -131,17 +120,29 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request
> *req, return -EINVAL;
> 
>   unit_size = CCP_XTS_AES_UNIT_SIZE__LAST;
> - if (req->nbytes <= unit_size_map[0].size) {
> - for (unit = 0; unit < ARRAY_SIZE(unit_size_map); unit++) {
> - if (!(req->nbytes & (unit_size_map[unit].size - 1))) {
> - unit_size = unit_size_map[unit].value;
> - break;
> - }
> + for (unit = ARRAY_SIZE(xts_unit_sizes); unit-- > 0; ) {
> + if (!(req->nbytes & (xts_unit_sizes[unit].size - 1))) {
> + unit_size = unit;
> + block_size = xts_unit_sizes[unit].size;
> + break;
>   }
>   }
> 
> - if ((unit_size == CCP_XTS_AES_UNIT_SIZE__LAST) ||
> - (ctx->u.aes.key_len != AES_KEYSIZE_128)) {
> + /* The CCP has

Re: [PATCH] crypto: ccp - Fix XTS-AES support on a version 5 CCP

2017-07-17 Thread Tom Lendacky 

On 7/17/2017 3:08 PM, Gary R Hook wrote:

Version 5 CCPs have differing requirements for XTS-AES: key components
are stored in a 512-bit vector. The context must be little-endian
justified. AES-256 is supported now, so propagate the cipher size to
the command descriptor.

Signed-off-by: Gary R Hook 
---
  drivers/crypto/ccp/ccp-crypto-aes-xts.c |   79 ---
  drivers/crypto/ccp/ccp-dev-v5.c |2 +
  drivers/crypto/ccp/ccp-dev.h|2 +
  drivers/crypto/ccp/ccp-ops.c|   56 ++
  4 files changed, 89 insertions(+), 50 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c 
b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 58a4244b4752..8d248b198e22 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -1,7 +1,7 @@
  /*
   * AMD Cryptographic Coprocessor (CCP) AES XTS crypto API support
   *
- * Copyright (C) 2013 Advanced Micro Devices, Inc.
+ * Copyright (C) 2013,2017 Advanced Micro Devices, Inc.
   *
   * Author: Tom Lendacky 
   *
@@ -37,46 +37,26 @@ struct ccp_unit_size_map {
u32 value;
  };
  
-static struct ccp_unit_size_map unit_size_map[] = {

+static struct ccp_unit_size_map xts_unit_sizes[] = {
{
-   .size   = 4096,
-   .value  = CCP_XTS_AES_UNIT_SIZE_4096,
-   },
-   {
-   .size   = 2048,
-   .value  = CCP_XTS_AES_UNIT_SIZE_2048,
-   },
-   {
-   .size   = 1024,
-   .value  = CCP_XTS_AES_UNIT_SIZE_1024,
+   .size   = 16,
+   .value  = CCP_XTS_AES_UNIT_SIZE_16,
},
{
-   .size   = 512,
+   .size   = 512,
.value  = CCP_XTS_AES_UNIT_SIZE_512,
},
{
-   .size   = 256,
-   .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
-   },
-   {
-   .size   = 128,
-   .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
-   },
-   {
-   .size   = 64,
-   .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
-   },
-   {
-   .size   = 32,
-   .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
+   .size   = 1024,
+   .value  = CCP_XTS_AES_UNIT_SIZE_1024,
},
{
-   .size   = 16,
-   .value  = CCP_XTS_AES_UNIT_SIZE_16,
+   .size   = 2048,
+   .value  = CCP_XTS_AES_UNIT_SIZE_2048,
},
{
-   .size   = 1,
-   .value  = CCP_XTS_AES_UNIT_SIZE__LAST,
+   .size   = 4096,
+   .value  = CCP_XTS_AES_UNIT_SIZE_4096,
},
  };


Because of the way the unit size check is performed, you can't delete
the intermediate size checks.  Those must remain so that unit sizes
that aren't supported by the CCP are sent to the fallback mechanism.

Also, re-arranging the order should be a separate patch if that doesn't
really fix anything.

  
@@ -97,14 +77,20 @@ static int ccp_aes_xts_setkey(struct crypto_ablkcipher *tfm, const u8 *key,

  unsigned int key_len)
  {
struct ccp_ctx *ctx = crypto_tfm_ctx(crypto_ablkcipher_tfm(tfm));
+   unsigned int ccpversion = ccp_version();
  
  	/* Only support 128-bit AES key with a 128-bit Tweak key,

 * otherwise use the fallback
 */
+


Remove the addition of the blank line and update the above comment to
indicate the new supported key size added below.


switch (key_len) {
case AES_KEYSIZE_128 * 2:
memcpy(ctx->u.aes.key, key, key_len);
break;
+   case AES_KEYSIZE_256 * 2:
+   if (ccpversion > CCP_VERSION(3, 0))
+   memcpy(ctx->u.aes.key, key, key_len);
+   break;


Isn't u.aes.key defined with a maximum buffer size of AES_MAX_KEY_SIZE
(which is 32)?  I think this will cause a buffer overrun on memcpy.


}
ctx->u.aes.key_len = key_len / 2;
sg_init_one(>u.aes.key_sg, ctx->u.aes.key, key_len);
@@ -117,7 +103,10 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request 
*req,
  {
struct ccp_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
struct ccp_aes_req_ctx *rctx = ablkcipher_request_ctx(req);
+   unsigned int ccpversion = ccp_version();
+   unsigned int fallback = 0;
unsigned int unit;
+   u32 block_size = 0;
u32 unit_size;
int ret;
  
@@ -131,17 +120,29 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req,

return -EINVAL;
  
  	unit_size = CCP_XTS_AES_UNIT_SIZE__LAST;

-   if (req->nbytes <= unit_size_map[0].size) {


This check can't be deleted.  It was added specifically to catch cases
where the size was greater than 4096.


-   for (unit = 0; unit < ARRAY_SIZE(unit_size_map); unit++) {
-   if (!(req->nbytes &