The extra run-time crypto self tests hit some corner cases that were
not handled in the drivers. Fix some corner cases. Propose some cleaning
patches.
Tudor Ambarus (9):
crypto: atmel-tdes: Clarify how tdes dev gets allocated to the tfm
crypto: atmel-tdes: Handle error messages
crypto: atmel-aes: Add blocksize constraint for ECB and CBC modes
crypto: atmel-aes: Add XTS input length constraint
crypto: atmel-aes: Add NIST 800-38A's zero length cryptlen constraint
crypto: atmel-tdes: Add FIPS81's zero length cryptlen constraint
crypto: atmel-{aes, tdes}: Set OFB's blocksize to 1
crypto: atmel-aes: Add fallback to XTS software implementation
crypto: atmel-aes: Allocate aes dev at tfm init time
drivers/crypto/atmel-aes.c | 146 +++++++++++++++++++++++++++---------
drivers/crypto/atmel-tdes.c | 66 +++++++---------
2 files changed, 138 insertions(+), 74 deletions(-)
--
2.25.1
NIST 800-38A requires for the ECB and CBC modes that the total number
of bits in the plaintext to be a multiple of the block cipher.
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/crypto/atmel-aes.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index b1d286004295..9c6d80d1d7a0 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -1089,6 +1089,11 @@ static int atmel_aes_crypt(struct skcipher_request *req, unsigned long mode)
struct atmel_aes_base_ctx *ctx = crypto_skcipher_ctx(skcipher);
struct atmel_aes_reqctx *rctx;
struct atmel_aes_dev *dd;
+ u32 opmode = mode & AES_FLAGS_OPMODE_MASK;
+
+ if ((opmode == AES_FLAGS_ECB || opmode == AES_FLAGS_CBC) &&
+ !IS_ALIGNED(req->cryptlen, crypto_skcipher_blocksize(skcipher)))
+ return -EINVAL;
switch (mode & AES_FLAGS_OPMODE_MASK) {
case AES_FLAGS_CFB8:
@@ -1120,7 +1125,7 @@ static int atmel_aes_crypt(struct skcipher_request *req, unsigned long mode)
rctx = skcipher_request_ctx(req);
rctx->mode = mode;
- if ((mode & AES_FLAGS_OPMODE_MASK) != AES_FLAGS_ECB &&
+ if (opmode != AES_FLAGS_ECB &&
!(mode & AES_FLAGS_ENCRYPT) && req->src == req->dst) {
unsigned int ivsize = crypto_skcipher_ivsize(skcipher);
--
2.25.1
XTS is supported just for input lengths with data units of 128-bit blocks.
Add a fallback to software implementation when the last block is shorter
than 128 bits.
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/crypto/atmel-aes.c | 55 +++++++++++++++++++++++++++++++++++---
1 file changed, 51 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 9ec007b4f8fc..e74fcaac551e 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -143,6 +143,7 @@ struct atmel_aes_xts_ctx {
struct atmel_aes_base_ctx base;
u32 key2[AES_KEYSIZE_256 / sizeof(u32)];
+ struct crypto_skcipher *fallback_tfm;
};
#if IS_ENABLED(CONFIG_CRYPTO_DEV_ATMEL_AUTHENC)
@@ -155,6 +156,7 @@ struct atmel_aes_authenc_ctx {
struct atmel_aes_reqctx {
unsigned long mode;
u8 lastc[AES_BLOCK_SIZE];
+ struct skcipher_request fallback_req;
};
#if IS_ENABLED(CONFIG_CRYPTO_DEV_ATMEL_AUTHENC)
@@ -1083,6 +1085,22 @@ static int atmel_aes_ctr_start(struct atmel_aes_dev *dd)
return atmel_aes_ctr_transfer(dd);
}
+static int atmel_aes_xts_fallback(struct skcipher_request *req, bool enc)
+{
+ struct atmel_aes_reqctx *rctx = skcipher_request_ctx(req);
+ struct atmel_aes_xts_ctx *ctx = crypto_skcipher_ctx(
+ crypto_skcipher_reqtfm(req));
+
+ skcipher_request_set_tfm(&rctx->fallback_req, ctx->fallback_tfm);
+ skcipher_request_set_callback(&rctx->fallback_req, req->base.flags,
+ req->base.complete, req->base.data);
+ skcipher_request_set_crypt(&rctx->fallback_req, req->src, req->dst,
+ req->cryptlen, req->iv);
+
+ return enc ? crypto_skcipher_encrypt(&rctx->fallback_req) :
+ crypto_skcipher_decrypt(&rctx->fallback_req);
+}
+
static int atmel_aes_crypt(struct skcipher_request *req, unsigned long mode)
{
struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
@@ -1091,8 +1109,14 @@ static int atmel_aes_crypt(struct skcipher_request *req, unsigned long mode)
struct atmel_aes_dev *dd;
u32 opmode = mode & AES_FLAGS_OPMODE_MASK;
- if (opmode == AES_FLAGS_XTS && req->cryptlen < XTS_BLOCK_SIZE)
- return -EINVAL;
+ if (opmode == AES_FLAGS_XTS) {
+ if (req->cryptlen < XTS_BLOCK_SIZE)
+ return -EINVAL;
+
+ if (!IS_ALIGNED(req->cryptlen, XTS_BLOCK_SIZE))
+ return atmel_aes_xts_fallback(req,
+ mode & AES_FLAGS_ENCRYPT);
+ }
/*
* ECB, CBC, CFB, OFB or CTR mode require the plaintext and ciphertext
@@ -1864,6 +1888,13 @@ static int atmel_aes_xts_setkey(struct crypto_skcipher *tfm, const u8 *key,
if (err)
return err;
+ crypto_skcipher_clear_flags(ctx->fallback_tfm, CRYPTO_TFM_REQ_MASK);
+ crypto_skcipher_set_flags(ctx->fallback_tfm, tfm->base.crt_flags &
+ CRYPTO_TFM_REQ_MASK);
+ err = crypto_skcipher_setkey(ctx->fallback_tfm, key, keylen);
+ if (err)
+ return err;
+
memcpy(ctx->base.key, key, keylen/2);
memcpy(ctx->key2, key + keylen/2, keylen/2);
ctx->base.keylen = keylen/2;
@@ -1884,18 +1915,33 @@ static int atmel_aes_xts_decrypt(struct skcipher_request *req)
static int atmel_aes_xts_init_tfm(struct crypto_skcipher *tfm)
{
struct atmel_aes_xts_ctx *ctx = crypto_skcipher_ctx(tfm);
+ const char *tfm_name = crypto_tfm_alg_name(&tfm->base);
- crypto_skcipher_set_reqsize(tfm, sizeof(struct atmel_aes_reqctx));
+ ctx->fallback_tfm = crypto_alloc_skcipher(tfm_name, 0,
+ CRYPTO_ALG_NEED_FALLBACK);
+ if (IS_ERR(ctx->fallback_tfm))
+ return PTR_ERR(ctx->fallback_tfm);
+
+ crypto_skcipher_set_reqsize(tfm, sizeof(struct atmel_aes_reqctx) +
+ crypto_skcipher_reqsize(ctx->fallback_tfm));
ctx->base.start = atmel_aes_xts_start;
return 0;
}
+static void atmel_aes_xts_exit_tfm(struct crypto_skcipher *tfm)
+{
+ struct atmel_aes_xts_ctx *ctx = crypto_skcipher_ctx(tfm);
+
+ crypto_free_skcipher(ctx->fallback_tfm);
+}
+
static struct skcipher_alg aes_xts_alg = {
.base.cra_name = "xts(aes)",
.base.cra_driver_name = "atmel-xts-aes",
.base.cra_blocksize = AES_BLOCK_SIZE,
.base.cra_ctxsize = sizeof(struct atmel_aes_xts_ctx),
+ .base.cra_flags = CRYPTO_ALG_NEED_FALLBACK,
.min_keysize = 2 * AES_MIN_KEY_SIZE,
.max_keysize = 2 * AES_MAX_KEY_SIZE,
@@ -1904,6 +1950,7 @@ static struct skcipher_alg aes_xts_alg = {
.encrypt = atmel_aes_xts_encrypt,
.decrypt = atmel_aes_xts_decrypt,
.init = atmel_aes_xts_init_tfm,
+ .exit = atmel_aes_xts_exit_tfm,
};
#if IS_ENABLED(CONFIG_CRYPTO_DEV_ATMEL_AUTHENC)
@@ -2373,7 +2420,7 @@ static void atmel_aes_unregister_algs(struct atmel_aes_dev *dd)
static void atmel_aes_crypto_alg_init(struct crypto_alg *alg)
{
- alg->cra_flags = CRYPTO_ALG_ASYNC;
+ alg->cra_flags |= CRYPTO_ALG_ASYNC;
alg->cra_alignmask = 0xf;
alg->cra_priority = ATMEL_AES_PRIORITY;
alg->cra_module = THIS_MODULE;
--
2.25.1
Downgrade all runtime error messages to dev_dbg so that we don't
pollute the console. All probe error messages are kept with dev_err.
Get rid of pr_err and use dev_dbg instead, so that we know from which
device the error comes.
dma_mapping_error() return code was overwritten, use the error code
that the function returns.
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/crypto/atmel-tdes.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/crypto/atmel-tdes.c b/drivers/crypto/atmel-tdes.c
index dda70dbe0838..abbf1b7a75ab 100644
--- a/drivers/crypto/atmel-tdes.c
+++ b/drivers/crypto/atmel-tdes.c
@@ -312,7 +312,7 @@ static int atmel_tdes_crypt_pdc_stop(struct atmel_tdes_dev *dd)
dd->buf_out, dd->buflen, dd->dma_size, 1);
if (count != dd->dma_size) {
err = -EINVAL;
- pr_err("not all data converted: %zu\n", count);
+ dev_dbg(dd->dev, "not all data converted: %zu\n", count);
}
}
@@ -329,24 +329,24 @@ static int atmel_tdes_buff_init(struct atmel_tdes_dev *dd)
dd->buflen &= ~(DES_BLOCK_SIZE - 1);
if (!dd->buf_in || !dd->buf_out) {
- dev_err(dd->dev, "unable to alloc pages.\n");
+ dev_dbg(dd->dev, "unable to alloc pages.\n");
goto err_alloc;
}
/* MAP here */
dd->dma_addr_in = dma_map_single(dd->dev, dd->buf_in,
dd->buflen, DMA_TO_DEVICE);
- if (dma_mapping_error(dd->dev, dd->dma_addr_in)) {
- dev_err(dd->dev, "dma %zd bytes error\n", dd->buflen);
- err = -EINVAL;
+ err = dma_mapping_error(dd->dev, dd->dma_addr_in);
+ if (err) {
+ dev_dbg(dd->dev, "dma %zd bytes error\n", dd->buflen);
goto err_map_in;
}
dd->dma_addr_out = dma_map_single(dd->dev, dd->buf_out,
dd->buflen, DMA_FROM_DEVICE);
- if (dma_mapping_error(dd->dev, dd->dma_addr_out)) {
- dev_err(dd->dev, "dma %zd bytes error\n", dd->buflen);
- err = -EINVAL;
+ err = dma_mapping_error(dd->dev, dd->dma_addr_out);
+ if (err) {
+ dev_dbg(dd->dev, "dma %zd bytes error\n", dd->buflen);
goto err_map_out;
}
@@ -359,8 +359,6 @@ static int atmel_tdes_buff_init(struct atmel_tdes_dev *dd)
err_alloc:
free_page((unsigned long)dd->buf_out);
free_page((unsigned long)dd->buf_in);
- if (err)
- pr_err("error: %d\n", err);
return err;
}
@@ -512,14 +510,14 @@ static int atmel_tdes_crypt_start(struct atmel_tdes_dev *dd)
err = dma_map_sg(dd->dev, dd->in_sg, 1, DMA_TO_DEVICE);
if (!err) {
- dev_err(dd->dev, "dma_map_sg() error\n");
+ dev_dbg(dd->dev, "dma_map_sg() error\n");
return -EINVAL;
}
err = dma_map_sg(dd->dev, dd->out_sg, 1,
DMA_FROM_DEVICE);
if (!err) {
- dev_err(dd->dev, "dma_map_sg() error\n");
+ dev_dbg(dd->dev, "dma_map_sg() error\n");
dma_unmap_sg(dd->dev, dd->in_sg, 1,
DMA_TO_DEVICE);
return -EINVAL;
@@ -670,7 +668,7 @@ static int atmel_tdes_crypt_dma_stop(struct atmel_tdes_dev *dd)
dd->buf_out, dd->buflen, dd->dma_size, 1);
if (count != dd->dma_size) {
err = -EINVAL;
- pr_err("not all data converted: %zu\n", count);
+ dev_dbg(dd->dev, "not all data converted: %zu\n", count);
}
}
}
@@ -682,11 +680,12 @@ static int atmel_tdes_crypt(struct skcipher_request *req, unsigned long mode)
struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
struct atmel_tdes_ctx *ctx = crypto_skcipher_ctx(skcipher);
struct atmel_tdes_reqctx *rctx = skcipher_request_ctx(req);
+ struct device *dev = ctx->dd->dev;
switch (mode & TDES_FLAGS_OPMODE_MASK) {
case TDES_FLAGS_CFB8:
if (!IS_ALIGNED(req->cryptlen, CFB8_BLOCK_SIZE)) {
- pr_err("request size is not exact amount of CFB8 blocks\n");
+ dev_dbg(dev, "request size is not exact amount of CFB8 blocks\n");
return -EINVAL;
}
ctx->block_size = CFB8_BLOCK_SIZE;
@@ -694,7 +693,7 @@ static int atmel_tdes_crypt(struct skcipher_request *req, unsigned long mode)
case TDES_FLAGS_CFB16:
if (!IS_ALIGNED(req->cryptlen, CFB16_BLOCK_SIZE)) {
- pr_err("request size is not exact amount of CFB16 blocks\n");
+ dev_dbg(dev, "request size is not exact amount of CFB16 blocks\n");
return -EINVAL;
}
ctx->block_size = CFB16_BLOCK_SIZE;
@@ -702,7 +701,7 @@ static int atmel_tdes_crypt(struct skcipher_request *req, unsigned long mode)
case TDES_FLAGS_CFB32:
if (!IS_ALIGNED(req->cryptlen, CFB32_BLOCK_SIZE)) {
- pr_err("request size is not exact amount of CFB32 blocks\n");
+ dev_dbg(dev, "request size is not exact amount of CFB32 blocks\n");
return -EINVAL;
}
ctx->block_size = CFB32_BLOCK_SIZE;
@@ -710,7 +709,7 @@ static int atmel_tdes_crypt(struct skcipher_request *req, unsigned long mode)
default:
if (!IS_ALIGNED(req->cryptlen, DES_BLOCK_SIZE)) {
- pr_err("request size is not exact amount of DES blocks\n");
+ dev_dbg(dev, "request size is not exact amount of DES blocks\n");
return -EINVAL;
}
ctx->block_size = DES_BLOCK_SIZE;
--
2.25.1
NIST 800-38A requires for the ECB, CBC, CFB, OFB and CTR modes that
the plaintext and ciphertext to have a positive integer length.
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/crypto/atmel-aes.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 4e9515e8dd25..8ea873bf6b86 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -1094,6 +1094,13 @@ static int atmel_aes_crypt(struct skcipher_request *req, unsigned long mode)
if (opmode == AES_FLAGS_XTS && req->cryptlen < XTS_BLOCK_SIZE)
return -EINVAL;
+ /*
+ * ECB, CBC, CFB, OFB or CTR mode require the plaintext and ciphertext
+ * to have a positve integer length.
+ */
+ if (!req->cryptlen && opmode != AES_FLAGS_XTS)
+ return 0;
+
if ((opmode == AES_FLAGS_ECB || opmode == AES_FLAGS_CBC) &&
!IS_ALIGNED(req->cryptlen, crypto_skcipher_blocksize(skcipher)))
return -EINVAL;
--
2.25.1
On Tue, Jul 20, 2021 at 11:55:26AM +0300, Tudor Ambarus wrote:
> The extra run-time crypto self tests hit some corner cases that were
> not handled in the drivers. Fix some corner cases. Propose some cleaning
> patches.
>
> Tudor Ambarus (9):
> crypto: atmel-tdes: Clarify how tdes dev gets allocated to the tfm
> crypto: atmel-tdes: Handle error messages
> crypto: atmel-aes: Add blocksize constraint for ECB and CBC modes
> crypto: atmel-aes: Add XTS input length constraint
> crypto: atmel-aes: Add NIST 800-38A's zero length cryptlen constraint
> crypto: atmel-tdes: Add FIPS81's zero length cryptlen constraint
> crypto: atmel-{aes, tdes}: Set OFB's blocksize to 1
> crypto: atmel-aes: Add fallback to XTS software implementation
> crypto: atmel-aes: Allocate aes dev at tfm init time
>
> drivers/crypto/atmel-aes.c | 146 +++++++++++++++++++++++++++---------
> drivers/crypto/atmel-tdes.c | 66 +++++++---------
> 2 files changed, 138 insertions(+), 74 deletions(-)
All applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt