Re: [PATCH] crypto: ccp - Fix AES XTS error for request sizes above 4096

[Tom Lendacky]

On 05/20/2016 06:35 PM, Herbert Xu wrote:
> On Fri, May 20, 2016 at 05:33:03PM -0500, Tom Lendacky wrote:
>> The ccp-crypto module for AES XTS support has a bug that can allow requests
>> greater than 4096 bytes in size to be passed to the CCP hardware. The CCP
>> hardware does not support request sizes larger than 4096, resulting in
>> incorrect output. The request should actually be handled by the fallback
>> mechanism instantiated by the ccp-crypto module.
>>
>> Add a check to insure the request size is less than or equal to the maximum
>> supported size and use the fallback mechanism if it is not.
>>
>> Cc:  # 3.14.x-
>> Signed-off-by: Tom Lendacky 
> 
> I'm OK with this patch but I think it doesn't always need to go into
> the fallback.  I made a test vector split as 4064 bytes + 48 bytes
> and ccp handled it just fine.  It appears that the bug is actually
> in the handling of a single SG entry that's longer than a page,
> presumably because sg_next is used unconditionally instead of
> checking whether there is more in the current SG entry.

I'll take a closer look at this. Something obviously isn't right but
the code doesn't do anything related to PAGE size checks and works
on the length specified in the SG entry.

> 
> But I'll merge your fix as it fixes a real problem.

Thanks Herbert.

Tom

> 
> Thanks,
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: ccp - Fix AES XTS error for request sizes above 4096

[Herbert Xu]

On Fri, May 20, 2016 at 05:33:03PM -0500, Tom Lendacky wrote:
> The ccp-crypto module for AES XTS support has a bug that can allow requests
> greater than 4096 bytes in size to be passed to the CCP hardware. The CCP
> hardware does not support request sizes larger than 4096, resulting in
> incorrect output. The request should actually be handled by the fallback
> mechanism instantiated by the ccp-crypto module.
> 
> Add a check to insure the request size is less than or equal to the maximum
> supported size and use the fallback mechanism if it is not.
> 
> Cc:  # 3.14.x-
> Signed-off-by: Tom Lendacky 

I'm OK with this patch but I think it doesn't always need to go into
the fallback.  I made a test vector split as 4064 bytes + 48 bytes
and ccp handled it just fine.  It appears that the bug is actually
in the handling of a single SG entry that's longer than a page,
presumably because sg_next is used unconditionally instead of
checking whether there is more in the current SG entry.

But I'll merge your fix as it fixes a real problem.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] crypto: ccp - Fix AES XTS error for request sizes above 4096

[Tom Lendacky]

The ccp-crypto module for AES XTS support has a bug that can allow requests
greater than 4096 bytes in size to be passed to the CCP hardware. The CCP
hardware does not support request sizes larger than 4096, resulting in
incorrect output. The request should actually be handled by the fallback
mechanism instantiated by the ccp-crypto module.

Add a check to insure the request size is less than or equal to the maximum
supported size and use the fallback mechanism if it is not.

Cc:  # 3.14.x-
Signed-off-by: Tom Lendacky 
---
 drivers/crypto/ccp/ccp-crypto-aes-xts.c |   17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c 
b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 52c7395..0d0d452 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -122,6 +122,7 @@ 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 unit;
+   u32 unit_size;
int ret;
 
if (!ctx->u.aes.key_len)
@@ -133,11 +134,17 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request 
*req,
if (!req->info)
return -EINVAL;
 
-   for (unit = 0; unit < ARRAY_SIZE(unit_size_map); unit++)
-   if (!(req->nbytes & (unit_size_map[unit].size - 1)))
-   break;
+   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;
+   }
+   }
+   }
 
-   if ((unit_size_map[unit].value == CCP_XTS_AES_UNIT_SIZE__LAST) ||
+   if ((unit_size == CCP_XTS_AES_UNIT_SIZE__LAST) ||
(ctx->u.aes.key_len != AES_KEYSIZE_128)) {
/* Use the fallback to process the request for any
 * unsupported unit sizes or key sizes
@@ -158,7 +165,7 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req,
rctx->cmd.engine = CCP_ENGINE_XTS_AES_128;
rctx->cmd.u.xts.action = (encrypt) ? CCP_AES_ACTION_ENCRYPT
   : CCP_AES_ACTION_DECRYPT;
-   rctx->cmd.u.xts.unit_size = unit_size_map[unit].value;
+   rctx->cmd.u.xts.unit_size = unit_size;
rctx->cmd.u.xts.key = >u.aes.key_sg;
rctx->cmd.u.xts.key_len = ctx->u.aes.key_len;
rctx->cmd.u.xts.iv = >iv_sg;

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html