On Fri, Oct 16, 2015 at 04:19:33PM -0700, Victoria Milhoan wrote:
> @@ -319,7 +319,7 @@ static int ahash_set_sh_desc(struct crypto_ahash *ahash)
> have_key = OP_ALG_AAI_HMAC_PRECOMP;
>
> /* ahash_update shared descriptor */
> - desc = ctx->sh_desc_update;
> + desc = kmalloc(DESC_HASH_MAX_USED_BYTES, GFP_KERNEL | GFP_DMA);
What if kmalloc() fails? Should this really oops the kernel? Ditto
for every other kmalloc you've added below.
> @@ -1557,6 +1575,20 @@ static int ahash_export(struct ahash_request *req,
> void *out)
> struct caam_hash_ctx *ctx = crypto_ahash_ctx(ahash);
> struct caam_hash_state *state = ahash_request_ctx(req);
>
> + /*
> + * Do not export the data buffers. New buffers are
> + * allocated during import.
> + */
> + kfree(state->buf_0);
> + kfree(state->buf_1);
> + state->buf_0 = NULL;
> + state->buf_1 = NULL;
> +
> + state->current_buf = 0;
> + state->buf_dma = 0;
> + state->buflen_0 = 0;
> + state->buflen_1 = 0;
> +
So what if I export, and then continue using _this_ context later?
> @@ -1605,6 +1641,8 @@ static struct caam_hash_template driver_hash[] = {
> .setkey = ahash_setkey,
> .halg = {
> .digestsize = SHA1_DIGEST_SIZE,
> + .statesize = sizeof(struct caam_hash_ctx) +
> + sizeof(struct caam_hash_state),
Much prefer to have a 'struct caam_hash_export_state' thing rather than
litter the code with the knowledge that these two go together.
> },
> },
> .alg_type = OP_ALG_ALGSEL_SHA1,
> @@ -1626,6 +1664,8 @@ static struct caam_hash_template driver_hash[] = {
> .setkey = ahash_setkey,
> .halg = {
> .digestsize = SHA224_DIGEST_SIZE,
> + .statesize = sizeof(struct caam_hash_ctx) +
> + sizeof(struct caam_hash_state),
> },
> },
> .alg_type = OP_ALG_ALGSEL_SHA224,
> @@ -1647,6 +1687,8 @@ static struct caam_hash_template driver_hash[] = {
> .setkey = ahash_setkey,
> .halg = {
> .digestsize = SHA256_DIGEST_SIZE,
> + .statesize = sizeof(struct caam_hash_ctx) +
> + sizeof(struct caam_hash_state),
> },
> },
> .alg_type = OP_ALG_ALGSEL_SHA256,
> @@ -1668,6 +1710,8 @@ static struct caam_hash_template driver_hash[] = {
> .setkey = ahash_setkey,
> .halg = {
> .digestsize = SHA384_DIGEST_SIZE,
> + .statesize = sizeof(struct caam_hash_ctx) +
> + sizeof(struct caam_hash_state),
> },
> },
> .alg_type = OP_ALG_ALGSEL_SHA384,
> @@ -1689,6 +1733,8 @@ static struct caam_hash_template driver_hash[] = {
> .setkey = ahash_setkey,
> .halg = {
> .digestsize = SHA512_DIGEST_SIZE,
> + .statesize = sizeof(struct caam_hash_ctx) +
> + sizeof(struct caam_hash_state),
> },
> },
> .alg_type = OP_ALG_ALGSEL_SHA512,
> @@ -1710,6 +1756,8 @@ static struct caam_hash_template driver_hash[] = {
> .setkey = ahash_setkey,
> .halg = {
> .digestsize = MD5_DIGEST_SIZE,
> + .statesize = sizeof(struct caam_hash_ctx) +
> + sizeof(struct caam_hash_state),
> },
> },
> .alg_type = OP_ALG_ALGSEL_MD5,
> @@ -1796,6 +1844,12 @@ static void caam_hash_cra_exit(struct crypto_tfm *tfm)
> dma_unmap_single(ctx->jrdev, ctx->sh_desc_finup_dma,
> desc_bytes(ctx->sh_desc_finup), DMA_TO_DEVICE);
>
> + kfree(ctx->sh_desc_update);
> + kfree(ctx->sh_desc_update_first);
> + kfree(ctx->sh_desc_fin);
> + kfree(ctx->sh_desc_digest);
> + kfree(ctx->sh_desc_finup);
> +
What happens to these when ahash_import() overwrites all the context
state? Doesn't this mean we end up with double-kfree()s ?
Also, did you test this DMA debug enabled?
--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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
