Re: [PATCH v2 3/4] crypto: ccp - Rework the unit-size check for XTS-AES

[Tom Lendacky]

On 7/21/2017 2:05 PM, Gary R Hook wrote:

The CCP supports a limited set of unit-size values. Change the check
for this parameter such that acceptable values match the enumeration.
Then clarify the conditions under which we must use the fallback
implementation.

Signed-off-by: Gary R Hook 
---
  drivers/crypto/ccp/ccp-crypto-aes-xts.c |   75 ++-
  1 file changed, 35 insertions(+), 40 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c 
b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 4a313f62dbea..3c37794ffe2d 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -1,8 +1,9 @@
  /*
   * 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: Gary R Hook 
   * Author: Tom Lendacky 
   *
   * This program is free software; you can redistribute it and/or modify
@@ -38,46 +39,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,
},
  };
  
@@ -124,7 +105,9 @@ 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 fallback = 0;
unsigned int unit;
+   u32 block_size;


I don't see this variable used anywhere. It should be deleted.


u32 unit_size;
int ret;
  
@@ -137,18 +120,30 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req,

if (!req->info)
return -EINVAL;
  
+	/* Check conditions under which the CCP can fulfill a request. The

+* device can handle input plaintext of a length that is a multiple
+* of the unit_size, bug the crypto implementation only supports
+* the unit_size being equal to the input length. This limits the
+* number of scenarios we can handle. Also validate the key length.


Remove the "Also validate the key length." since that happens below and
is covered by a different comment.


+*/
+   block_size = 0;
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 = 0; unit < ARRAY_SIZE(xts_unit_sizes); unit++) {
+   if (req->nbytes == xts_unit_sizes[unit].size) {
+   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 restrictions on block sizes. Also, a version 3 device
+* only supports AES-128 operations; version 5 CCPs support both
+* AES-128 and -256 operations.


The 256-bit XTS support isn't here yet, so shouldn't mention it now.


+*/
+   if (unit_size == CCP_XTS_AES_UNIT_SIZE__LAST)
+   fallback = 1;
+   if (ctx->u.aes.key_len != AES_KEYSIZE_128)
+   fallback = 1;
+

[PATCH v2 3/4] crypto: ccp - Rework the unit-size check for XTS-AES

[Gary R Hook]

The CCP supports a limited set of unit-size values. Change the check
for this parameter such that acceptable values match the enumeration.
Then clarify the conditions under which we must use the fallback
implementation.

Signed-off-by: Gary R Hook 
---
 drivers/crypto/ccp/ccp-crypto-aes-xts.c |   75 ++-
 1 file changed, 35 insertions(+), 40 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c 
b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 4a313f62dbea..3c37794ffe2d 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -1,8 +1,9 @@
 /*
  * 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: Gary R Hook 
  * Author: Tom Lendacky 
  *
  * This program is free software; you can redistribute it and/or modify
@@ -38,46 +39,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,
},
 };
 
@@ -124,7 +105,9 @@ 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 fallback = 0;
unsigned int unit;
+   u32 block_size;
u32 unit_size;
int ret;
 
@@ -137,18 +120,30 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request 
*req,
if (!req->info)
return -EINVAL;
 
+   /* Check conditions under which the CCP can fulfill a request. The
+* device can handle input plaintext of a length that is a multiple
+* of the unit_size, bug the crypto implementation only supports
+* the unit_size being equal to the input length. This limits the
+* number of scenarios we can handle. Also validate the key length.
+*/
+   block_size = 0;
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 = 0; unit < ARRAY_SIZE(xts_unit_sizes); unit++) {
+   if (req->nbytes == xts_unit_sizes[unit].size) {
+   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 restrictions on block sizes. Also, a version 3 device
+* only supports AES-128 operations; version 5 CCPs support both
+* AES-128 and -256 operations.
+*/
+   if (unit_size == CCP_XTS_AES_UNIT_SIZE__LAST)
+   fallback = 1;
+   if (ctx->u.aes.key_len != AES_KEYSIZE_128)
+   fallback = 1;
+   if (fallback) {
SKCIPHER_REQUEST_ON_STACK(subreq, ctx->u.aes.tfm_skcipher);
 
/* Use the fallback to process the request for any