All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Horia Geantă" <horia.geanta@freescale.com>
To: Russell King <rmk+kernel@arm.linux.org.uk>,
	Fabio Estevam <fabio.estevam@freescale.com>,
	Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>, <linux-crypto@vger.kernel.org>
Subject: Re: [PATCH RFC 07/11] crypto: caam: check and use dma_map_sg() return code
Date: Wed, 9 Dec 2015 17:20:45 +0200	[thread overview]
Message-ID: <566846CD.1020607@freescale.com> (raw)
In-Reply-To: <E1a61D4-0007ms-UR@rmk-PC.arm.linux.org.uk>

On 12/7/2015 9:12 PM, Russell King wrote:
> Strictly, dma_map_sg() may coalesce SG entries, but in practise on iMX
> hardware, this will never happen.  However, dma_map_sg() can fail, and
> we completely fail to check its return value.  So, fix this properly.
> 
> Arrange the code to map the scatterlist early, so we know how many
> scatter table entries to allocate, and then fill them in.  This allows
> us to keep relatively simple error cleanup paths.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Some tcrypt tests fail - looks like those with zero plaintext:
caam_jr ffe301000.jr: unable to map source for DMA
alg: hash: digest failed on test 1 for sha1-caam: ret=12
[...]

Need to be careful, dma_map_sg() returning zero is an error only if ptxt
is not null (alternatively src_nents returned by sg_nents_for_len()
could be checked).

> @@ -1091,7 +1102,7 @@ static int ahash_digest(struct ahash_request *req)
>  	u32 *sh_desc = ctx->sh_desc_digest, *desc;
>  	dma_addr_t ptr = ctx->sh_desc_digest_dma;
>  	int digestsize = crypto_ahash_digestsize(ahash);
> -	int src_nents, sec4_sg_bytes;
> +	int src_nents, mapped_nents, sec4_sg_bytes;
>  	dma_addr_t src_dma;
>  	struct ahash_edesc *edesc;
>  	int ret = 0;
> @@ -1099,9 +1110,14 @@ static int ahash_digest(struct ahash_request *req)
>  	int sh_len;
>  
>  	src_nents = sg_nents_for_len(req->src, req->nbytes);
> -	dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
> -	if (src_nents > 1)
> -		sec4_sg_bytes = src_nents * sizeof(struct sec4_sg_entry);
> +	mapped_nents = dma_map_sg(jrdev, req->src, src_nents, DMA_TO_DEVICE);
> +	if (mapped_nents == 0) {
> +		dev_err(jrdev, "unable to map source for DMA\n");
> +		return -ENOMEM;
> +	}

This is at least one of the places where the error condition must change
to smth. like (src_nents != 0 && mapped_nents == 0).

Horia

  reply	other threads:[~2015-12-09 15:20 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-07 19:11 [PATCH RFC 00/13] Further iMX CAAM updates Russell King - ARM Linux
2015-12-07 19:12 ` [PATCH RFC 01/11] crypto: caam: fix DMA API mapping leak Russell King
2015-12-07 19:12 ` [PATCH RFC 02/11] crypto: caam: ensure descriptor buffers are cacheline aligned Russell King
2015-12-07 19:12 ` [PATCH RFC 03/11] crypto: caam: incorporate job descriptor into struct ahash_edesc Russell King
2015-12-07 19:12 ` [PATCH RFC 04/11] crypto: caam: mark the hardware descriptor as cache line aligned Russell King
2015-12-07 19:12 ` [PATCH RFC 05/11] crypto: caam: replace sec4_sg pointer with array Russell King
2015-12-07 19:12 ` [PATCH RFC 06/11] crypto: caam: ensure that we clean up after an error Russell King
2015-12-09 15:08   ` Horia Geantă
2015-12-09 18:30     ` Russell King - ARM Linux
2015-12-07 19:12 ` [PATCH RFC 07/11] crypto: caam: check and use dma_map_sg() return code Russell King
2015-12-09 15:20   ` Horia Geantă [this message]
2015-12-09 18:31     ` Russell King - ARM Linux
2015-12-07 19:12 ` [PATCH RFC 08/11] crypto: caam: add ahash_edesc_alloc() for descriptor allocation Russell King
2015-12-07 19:12 ` [PATCH RFC 09/11] crypto: caam: move job descriptor initialisation to ahash_edesc_alloc() Russell King
2015-12-07 19:12 ` [PATCH RFC 10/11] crypto: caam: add ahash_edesc_add_src() Russell King
2015-12-09 16:21   ` Horia Geantă
2015-12-07 19:12 ` [PATCH RFC 11/11] crypto: caam: get rid of tasklet Russell King
2015-12-07 20:59 ` [PATCH RFC 00/13] Further iMX CAAM updates Fabio Estevam
2015-12-09 15:06 ` Horia Geantă
2015-12-09 18:21   ` Russell King - ARM Linux

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=566846CD.1020607@freescale.com \
    --to=horia.geanta@freescale.com \
    --cc=davem@davemloft.net \
    --cc=fabio.estevam@freescale.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=rmk+kernel@arm.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.