Hi, Romain,
On 10/18/2017 04:32 PM, Romain Izard wrote:
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 29e20c37f3a6..f3eabe1f1490 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -80,6 +80,7 @@
#define AES_FLAGS_BUSY BIT(3)
#define AES_FLAGS_DUMP_REG BIT(4)
#define AES_FLAGS_OWN_SHA BIT(5)
+#define AES_FLAGS_CIPHERTAIL BIT(6)
not really needed, see below.
#define AES_FLAGS_PERSISTENT (AES_FLAGS_INIT | AES_FLAGS_BUSY)
@@ -156,6 +157,7 @@ struct atmel_aes_authenc_ctx {
struct atmel_aes_reqctx {
unsigned long mode;
+ u32 ciphertail[AES_BLOCK_SIZE / sizeof(u32)];
How about renaming this variable to "lastc"? Ci, i=1..n is the usual
notation for the ciphertext blocks.
The assumption that the last ciphertext block is of AES_BLOCK_SIZE size
is correct, this driver is meant just for AES.
};
#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
@@ -496,6 +498,11 @@ static void atmel_aes_authenc_complete(struct
atmel_aes_dev *dd, int err);
static inline int atmel_aes_complete(struct atmel_aes_dev *dd, int err)
{
+ struct ablkcipher_request *req = ablkcipher_request_cast(dd->areq);
+ struct crypto_ablkcipher *ablkcipher = crypto_ablkcipher_reqtfm(req);
+ struct atmel_aes_reqctx *rctx = ablkcipher_request_ctx(req);
+ int ivsize = crypto_ablkcipher_ivsize(ablkcipher);
+
#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
atmel_aes_authenc_complete(dd, err);
#endif
@@ -503,6 +510,17 @@ static inline int atmel_aes_complete(struct
atmel_aes_dev *dd, int err)
clk_disable(dd->iclk);
dd->flags &= ~AES_FLAGS_BUSY;
+ if (rctx->mode & AES_FLAGS_CIPHERTAIL) {
+ if (rctx->mode & AES_FLAGS_ENCRYPT) {
+ scatterwalk_map_and_copy(req->info, req->dst,
+ req->nbytes - ivsize,
+ ivsize, 0);
+ } else {
+ memcpy(req->info, rctx->ciphertail, ivsize);
why don't we make the same assumption and replace ivsize with
AES_BLOCK_SIZE? Is there any chance that req->info was allocated with
less than AES_BLOCK_SIZE size?
+ memset(rctx->ciphertail, 0, ivsize);
memset to zero is not necessary.
+ }
+ }
+
if (dd->is_async)
dd->areq->complete(dd->areq, err);
@@ -1071,11 +1089,13 @@ static int atmel_aes_ctr_start(struct atmel_aes_dev *dd)
static int atmel_aes_crypt(struct ablkcipher_request *req, unsigned long mode)
{
+ struct crypto_ablkcipher *ablkcipher;
struct atmel_aes_base_ctx *ctx;
struct atmel_aes_reqctx *rctx;
struct atmel_aes_dev *dd;
- ctx = crypto_ablkcipher_ctx(crypto_ablkcipher_reqtfm(req));
+ ablkcipher = crypto_ablkcipher_reqtfm(req);
+ ctx = crypto_ablkcipher_ctx(ablkcipher);
you can initialize these variables at declaration.
switch (mode & AES_FLAGS_OPMODE_MASK) {
case AES_FLAGS_CFB8:
ctx->block_size = CFB8_BLOCK_SIZE;
@@ -1103,7 +1123,15 @@ static int atmel_aes_crypt(struct
ablkcipher_request *req, unsigned long mode)
return -ENODEV;
rctx = ablkcipher_request_ctx(req);
while here, please initialize this at declaration.
Also, this one:
'''
dd = atmel_aes_find_dev(ctx);
if (!dd)
return -ENODEV;
'''
should be moved at the beginning of the function. If an initialization
might fail, let's check it as soon as we can, so that we don't waste
resources.
- rctx->mode = mode;
+
+ if (!(mode & AES_FLAGS_ENCRYPT)) {
+ int ivsize = crypto_ablkcipher_ivsize(ablkcipher);
+
+ scatterwalk_map_and_copy(rctx->ciphertail, req->src,
+ (req->nbytes - ivsize), ivsize, 0);
+ }
+ rctx->mode = mode | AES_FLAGS_CIPHERTAIL;
saving the last ciphertext block is a requirement for skcipher. This
flag is redundant, there's no need to use it.
Thank you, Romain!
ta
