| Message ID | 150066390692.49973.8816619065422606698.stgit@sosxen.amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
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 <gary.hook@amd.com> > --- > 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 <gary.hook@amd.com> > * Author: Tom Lendacky <thomas.lendacky@amd.com> > * > * 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; > + if (fallback) { It appears these changes are not necessary... It does the same thing as the previous check. Just make the changes when the XTS-256 support is added in the next patch. Thanks, Tom > SKCIPHER_REQUEST_ON_STACK(subreq, ctx->u.aes.tfm_skcipher); > > /* Use the fallback to process the request for any >
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 <gary.hook@amd.com> * Author: Tom Lendacky <thomas.lendacky@amd.com> * * 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
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 <gary.hook@amd.com> --- drivers/crypto/ccp/ccp-crypto-aes-xts.c | 75 ++++++++++++++----------------- 1 file changed, 35 insertions(+), 40 deletions(-)