Skip to content

Commit

Permalink
crypto: caam - fix MDHA key derivation for certain user key lengths
Browse files Browse the repository at this point in the history 
Fuzz testing uncovered an issue when |user key| > |derived key|.
Derived key generation has to be fixed in two cases:

1. Era >= 6 (DKP is available)
DKP cannot be used with immediate input key if |user key| > |derived key|,
since the resulting descriptor (after DKP execution) would be invalid -
having a few bytes from user key left in descriptor buffer
as incorrect opcodes.

Fix DKP usage both in standalone hmac and in authenc algorithms.
For authenc the logic is simplified, by always storing both virtual
and dma key addresses.

2. Era < 6
The same case (|user key| > |derived key|) fails when DKP
is not available.
Make sure gen_split_key() dma maps max(|user key|, |derived key|),
since this is an in-place (bidirectional) operation.

Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
  • Loading branch information
Horia Geantă authored and Herbert Xu committed Aug 9, 2019
1 parent a2fb864 commit e9b4913
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 105 deletions.
42 changes: 12 additions & 30 deletions drivers/crypto/caam/caamalg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,18 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ctx->cdata.keylen - CTR_RFC3686_NONCE_SIZE);
}

/*
* In case |user key| > |derived key|, using DKP<imm,imm>
* would result in invalid opcodes (last bytes of user key) in
* the resulting descriptor. Use DKP<ptr,imm> instead => both
* virtual and dma key addresses are needed.
*/
ctx->adata.key_virt = ctx->key;
ctx->adata.key_dma = ctx->key_dma;

ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;

data_len[0] = ctx->adata.keylen_pad;
data_len[1] = ctx->cdata.keylen;

Expand All @@ -221,16 +233,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;

if (inl_mask & 1)
ctx->adata.key_virt = ctx->key;
else
ctx->adata.key_dma = ctx->key_dma;

if (inl_mask & 2)
ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
else
ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;

ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);

Expand All @@ -253,16 +255,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;

if (inl_mask & 1)
ctx->adata.key_virt = ctx->key;
else
ctx->adata.key_dma = ctx->key_dma;

if (inl_mask & 2)
ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
else
ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;

ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);

Expand All @@ -287,16 +279,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;

if (inl_mask & 1)
ctx->adata.key_virt = ctx->key;
else
ctx->adata.key_dma = ctx->key_dma;

if (inl_mask & 2)
ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
else
ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;

ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);

Expand Down
42 changes: 12 additions & 30 deletions drivers/crypto/caam/caamalg_qi.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,18 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ctx->cdata.keylen - CTR_RFC3686_NONCE_SIZE);
}

/*
* In case |user key| > |derived key|, using DKP<imm,imm> would result
* in invalid opcodes (last bytes of user key) in the resulting
* descriptor. Use DKP<ptr,imm> instead => both virtual and dma key
* addresses are needed.
*/
ctx->adata.key_virt = ctx->key;
ctx->adata.key_dma = ctx->key_dma;

ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;

data_len[0] = ctx->adata.keylen_pad;
data_len[1] = ctx->cdata.keylen;

Expand All @@ -118,16 +130,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;

if (inl_mask & 1)
ctx->adata.key_virt = ctx->key;
else
ctx->adata.key_dma = ctx->key_dma;

if (inl_mask & 2)
ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
else
ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;

ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);

Expand All @@ -143,16 +145,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;

if (inl_mask & 1)
ctx->adata.key_virt = ctx->key;
else
ctx->adata.key_dma = ctx->key_dma;

if (inl_mask & 2)
ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
else
ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;

ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);

Expand All @@ -171,16 +163,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;

if (inl_mask & 1)
ctx->adata.key_virt = ctx->key;
else
ctx->adata.key_dma = ctx->key_dma;

if (inl_mask & 2)
ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
else
ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;

ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);

Expand Down
67 changes: 47 additions & 20 deletions drivers/crypto/caam/caamalg_qi2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,18 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ctx->cdata.keylen - CTR_RFC3686_NONCE_SIZE);
}

/*
* In case |user key| > |derived key|, using DKP<imm,imm> would result
* in invalid opcodes (last bytes of user key) in the resulting
* descriptor. Use DKP<ptr,imm> instead => both virtual and dma key
* addresses are needed.
*/
ctx->adata.key_virt = ctx->key;
ctx->adata.key_dma = ctx->key_dma;

ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;

data_len[0] = ctx->adata.keylen_pad;
data_len[1] = ctx->cdata.keylen;

Expand All @@ -210,16 +222,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;

if (inl_mask & 1)
ctx->adata.key_virt = ctx->key;
else
ctx->adata.key_dma = ctx->key_dma;

if (inl_mask & 2)
ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
else
ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;

ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);

Expand Down Expand Up @@ -248,16 +250,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;

if (inl_mask & 1)
ctx->adata.key_virt = ctx->key;
else
ctx->adata.key_dma = ctx->key_dma;

if (inl_mask & 2)
ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
else
ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;

ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);

Expand Down Expand Up @@ -2999,13 +2991,15 @@ enum hash_optype {
/**
* caam_hash_ctx - ahash per-session context
* @flc: Flow Contexts array
* @key: authentication key
* @flc_dma: I/O virtual addresses of the Flow Contexts
* @dev: dpseci device
* @ctx_len: size of Context Register
* @adata: hashing algorithm details
*/
struct caam_hash_ctx {
struct caam_flc flc[HASH_NUM_OP];
u8 key[CAAM_MAX_HASH_BLOCK_SIZE] ____cacheline_aligned;
dma_addr_t flc_dma[HASH_NUM_OP];
struct device *dev;
int ctx_len;
Expand Down Expand Up @@ -3306,6 +3300,19 @@ static int ahash_setkey(struct crypto_ahash *ahash, const u8 *key,
ctx->adata.key_virt = key;
ctx->adata.key_inline = true;

/*
* In case |user key| > |derived key|, using DKP<imm,imm> would result
* in invalid opcodes (last bytes of user key) in the resulting
* descriptor. Use DKP<ptr,imm> instead => both virtual and dma key
* addresses are needed.
*/
if (keylen > ctx->adata.keylen_pad) {
memcpy(ctx->key, key, keylen);
dma_sync_single_for_device(ctx->dev, ctx->adata.key_dma,
ctx->adata.keylen_pad,
DMA_TO_DEVICE);
}

ret = ahash_set_sh_desc(ahash);
kfree(hashed_key);
return ret;
Expand Down Expand Up @@ -4536,11 +4543,27 @@ static int caam_hash_cra_init(struct crypto_tfm *tfm)

ctx->dev = caam_hash->dev;

if (alg->setkey) {
ctx->adata.key_dma = dma_map_single_attrs(ctx->dev, ctx->key,
ARRAY_SIZE(ctx->key),
DMA_TO_DEVICE,
DMA_ATTR_SKIP_CPU_SYNC);
if (dma_mapping_error(ctx->dev, ctx->adata.key_dma)) {
dev_err(ctx->dev, "unable to map key\n");
return -ENOMEM;
}
}

dma_addr = dma_map_single_attrs(ctx->dev, ctx->flc, sizeof(ctx->flc),
DMA_BIDIRECTIONAL,
DMA_ATTR_SKIP_CPU_SYNC);
if (dma_mapping_error(ctx->dev, dma_addr)) {
dev_err(ctx->dev, "unable to map shared descriptors\n");
if (ctx->adata.key_dma)
dma_unmap_single_attrs(ctx->dev, ctx->adata.key_dma,
ARRAY_SIZE(ctx->key),
DMA_TO_DEVICE,
DMA_ATTR_SKIP_CPU_SYNC);
return -ENOMEM;
}

Expand All @@ -4566,6 +4589,10 @@ static void caam_hash_cra_exit(struct crypto_tfm *tfm)

dma_unmap_single_attrs(ctx->dev, ctx->flc_dma[0], sizeof(ctx->flc),
DMA_BIDIRECTIONAL, DMA_ATTR_SKIP_CPU_SYNC);
if (ctx->adata.key_dma)
dma_unmap_single_attrs(ctx->dev, ctx->adata.key_dma,
ARRAY_SIZE(ctx->key), DMA_TO_DEVICE,
DMA_ATTR_SKIP_CPU_SYNC);
}

static struct caam_hash_alg *caam_hash_alloc(struct device *dev,
Expand Down
53 changes: 38 additions & 15 deletions drivers/crypto/caam/caamhash.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ struct caam_hash_ctx {
dma_addr_t sh_desc_fin_dma;
dma_addr_t sh_desc_digest_dma;
enum dma_data_direction dir;
enum dma_data_direction key_dir;
struct device *jrdev;
int ctx_len;
struct alginfo adata;
Expand Down Expand Up @@ -476,6 +477,18 @@ static int ahash_setkey(struct crypto_ahash *ahash,
goto bad_free_key;

memcpy(ctx->key, key, keylen);

/*
* In case |user key| > |derived key|, using DKP<imm,imm>
* would result in invalid opcodes (last bytes of user key) in
* the resulting descriptor. Use DKP<ptr,imm> instead => both
* virtual and dma key addresses are needed.
*/
if (keylen > ctx->adata.keylen_pad)
dma_sync_single_for_device(ctx->jrdev,
ctx->adata.key_dma,
ctx->adata.keylen_pad,
DMA_TO_DEVICE);
} else {
ret = gen_split_key(ctx->jrdev, ctx->key, &ctx->adata, key,
keylen, CAAM_MAX_HASH_KEY_SIZE);
Expand Down Expand Up @@ -1825,40 +1838,50 @@ static int caam_hash_cra_init(struct crypto_tfm *tfm)

if (is_xcbc_aes(caam_hash->alg_type)) {
ctx->dir = DMA_TO_DEVICE;
ctx->key_dir = DMA_BIDIRECTIONAL;
ctx->adata.algtype = OP_TYPE_CLASS1_ALG | caam_hash->alg_type;
ctx->ctx_len = 48;

ctx->adata.key_dma = dma_map_single_attrs(ctx->jrdev, ctx->key,
ARRAY_SIZE(ctx->key),
DMA_BIDIRECTIONAL,
DMA_ATTR_SKIP_CPU_SYNC);
if (dma_mapping_error(ctx->jrdev, ctx->adata.key_dma)) {
dev_err(ctx->jrdev, "unable to map key\n");
caam_jr_free(ctx->jrdev);
return -ENOMEM;
}
} else if (is_cmac_aes(caam_hash->alg_type)) {
ctx->dir = DMA_TO_DEVICE;
ctx->key_dir = DMA_NONE;
ctx->adata.algtype = OP_TYPE_CLASS1_ALG | caam_hash->alg_type;
ctx->ctx_len = 32;
} else {
ctx->dir = priv->era >= 6 ? DMA_BIDIRECTIONAL : DMA_TO_DEVICE;
if (priv->era >= 6) {
ctx->dir = DMA_BIDIRECTIONAL;
ctx->key_dir = alg->setkey ? DMA_TO_DEVICE : DMA_NONE;
} else {
ctx->dir = DMA_TO_DEVICE;
ctx->key_dir = DMA_NONE;
}
ctx->adata.algtype = OP_TYPE_CLASS2_ALG | caam_hash->alg_type;
ctx->ctx_len = runninglen[(ctx->adata.algtype &
OP_ALG_ALGSEL_SUBMASK) >>
OP_ALG_ALGSEL_SHIFT];
}

if (ctx->key_dir != DMA_NONE) {
ctx->adata.key_dma = dma_map_single_attrs(ctx->jrdev, ctx->key,
ARRAY_SIZE(ctx->key),
ctx->key_dir,
DMA_ATTR_SKIP_CPU_SYNC);
if (dma_mapping_error(ctx->jrdev, ctx->adata.key_dma)) {
dev_err(ctx->jrdev, "unable to map key\n");
caam_jr_free(ctx->jrdev);
return -ENOMEM;
}
}

dma_addr = dma_map_single_attrs(ctx->jrdev, ctx->sh_desc_update,
offsetof(struct caam_hash_ctx, key),
ctx->dir, DMA_ATTR_SKIP_CPU_SYNC);
if (dma_mapping_error(ctx->jrdev, dma_addr)) {
dev_err(ctx->jrdev, "unable to map shared descriptors\n");

if (is_xcbc_aes(caam_hash->alg_type))
if (ctx->key_dir != DMA_NONE)
dma_unmap_single_attrs(ctx->jrdev, ctx->adata.key_dma,
ARRAY_SIZE(ctx->key),
DMA_BIDIRECTIONAL,
ctx->key_dir,
DMA_ATTR_SKIP_CPU_SYNC);

caam_jr_free(ctx->jrdev);
Expand Down Expand Up @@ -1891,9 +1914,9 @@ static void caam_hash_cra_exit(struct crypto_tfm *tfm)
dma_unmap_single_attrs(ctx->jrdev, ctx->sh_desc_update_dma,
offsetof(struct caam_hash_ctx, key),
ctx->dir, DMA_ATTR_SKIP_CPU_SYNC);
if (is_xcbc_aes(ctx->adata.algtype))
if (ctx->key_dir != DMA_NONE)
dma_unmap_single_attrs(ctx->jrdev, ctx->adata.key_dma,
ARRAY_SIZE(ctx->key), DMA_BIDIRECTIONAL,
ARRAY_SIZE(ctx->key), ctx->key_dir,
DMA_ATTR_SKIP_CPU_SYNC);
caam_jr_free(ctx->jrdev);
}
Expand Down
Loading

0 comments on commit e9b4913

Please sign in to comment.