2022-09-06 03:02:40

by Peter Harliman Liem

[permalink] [raw]
Subject: [PATCH v2 1/2] crypto: inside_secure - Avoid dma map if size is zero

From commit d03c54419274 ("dma-mapping: disallow .map_sg
operations from returning zero on error"), dma_map_sg()
produces warning if size is 0. This results in visible
warnings if crypto length is zero.
To avoid that, we avoid calling dma_map_sg if size is zero.

Fixes: d03c54419274 ("dma-mapping: disallow .map_sg operations from returning zero on error")
Signed-off-by: Peter Harliman Liem <[email protected]>
---
v2:
Add fixes tag

drivers/crypto/inside-secure/safexcel_cipher.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
index d68ef16650d4..3775497775e0 100644
--- a/drivers/crypto/inside-secure/safexcel_cipher.c
+++ b/drivers/crypto/inside-secure/safexcel_cipher.c
@@ -737,14 +737,17 @@ static int safexcel_send_req(struct crypto_async_request *base, int ring,
max(totlen_src, totlen_dst));
return -EINVAL;
}
- dma_map_sg(priv->dev, src, sreq->nr_src, DMA_BIDIRECTIONAL);
+ if (sreq->nr_src > 0)
+ dma_map_sg(priv->dev, src, sreq->nr_src, DMA_BIDIRECTIONAL);
} else {
if (unlikely(totlen_src && (sreq->nr_src <= 0))) {
dev_err(priv->dev, "Source buffer not large enough (need %d bytes)!",
totlen_src);
return -EINVAL;
}
- dma_map_sg(priv->dev, src, sreq->nr_src, DMA_TO_DEVICE);
+
+ if (sreq->nr_src > 0)
+ dma_map_sg(priv->dev, src, sreq->nr_src, DMA_TO_DEVICE);

if (unlikely(totlen_dst && (sreq->nr_dst <= 0))) {
dev_err(priv->dev, "Dest buffer not large enough (need %d bytes)!",
@@ -753,7 +756,9 @@ static int safexcel_send_req(struct crypto_async_request *base, int ring,
DMA_TO_DEVICE);
return -EINVAL;
}
- dma_map_sg(priv->dev, dst, sreq->nr_dst, DMA_FROM_DEVICE);
+
+ if (sreq->nr_dst > 0)
+ dma_map_sg(priv->dev, dst, sreq->nr_dst, DMA_FROM_DEVICE);
}

memcpy(ctx->base.ctxr->data, ctx->key, ctx->key_len);
--
2.17.1


2022-09-06 15:20:32

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] crypto: inside_secure - Avoid dma map if size is zero

Quoting Peter Harliman Liem (2022-09-06 04:51:49)
> From commit d03c54419274 ("dma-mapping: disallow .map_sg
> operations from returning zero on error"), dma_map_sg()
> produces warning if size is 0. This results in visible
> warnings if crypto length is zero.
> To avoid that, we avoid calling dma_map_sg if size is zero.
>
> Fixes: d03c54419274 ("dma-mapping: disallow .map_sg operations from returning zero on error")

You can't reference the commit above, it's not introducing the issue but
the warning itself. The actual commit introducing the below logic should
be referenced.

Alternatively since the warning was introduced latter than the logic and
this is not a huge issue, you might resend it w/o the Fixes tag as well.

> Signed-off-by: Peter Harliman Liem <[email protected]>

With the Fixes: tag fixed,

Acked-by: Antoine Tenart <[email protected]>

Thanks!

> ---
> v2:
> Add fixes tag
>
> drivers/crypto/inside-secure/safexcel_cipher.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
> index d68ef16650d4..3775497775e0 100644
> --- a/drivers/crypto/inside-secure/safexcel_cipher.c
> +++ b/drivers/crypto/inside-secure/safexcel_cipher.c
> @@ -737,14 +737,17 @@ static int safexcel_send_req(struct crypto_async_request *base, int ring,
> max(totlen_src, totlen_dst));
> return -EINVAL;
> }
> - dma_map_sg(priv->dev, src, sreq->nr_src, DMA_BIDIRECTIONAL);
> + if (sreq->nr_src > 0)
> + dma_map_sg(priv->dev, src, sreq->nr_src, DMA_BIDIRECTIONAL);
> } else {
> if (unlikely(totlen_src && (sreq->nr_src <= 0))) {
> dev_err(priv->dev, "Source buffer not large enough (need %d bytes)!",
> totlen_src);
> return -EINVAL;
> }
> - dma_map_sg(priv->dev, src, sreq->nr_src, DMA_TO_DEVICE);
> +
> + if (sreq->nr_src > 0)
> + dma_map_sg(priv->dev, src, sreq->nr_src, DMA_TO_DEVICE);
>
> if (unlikely(totlen_dst && (sreq->nr_dst <= 0))) {
> dev_err(priv->dev, "Dest buffer not large enough (need %d bytes)!",
> @@ -753,7 +756,9 @@ static int safexcel_send_req(struct crypto_async_request *base, int ring,
> DMA_TO_DEVICE);
> return -EINVAL;
> }
> - dma_map_sg(priv->dev, dst, sreq->nr_dst, DMA_FROM_DEVICE);
> +
> + if (sreq->nr_dst > 0)
> + dma_map_sg(priv->dev, dst, sreq->nr_dst, DMA_FROM_DEVICE);
> }
>
> memcpy(ctx->base.ctxr->data, ctx->key, ctx->key_len);
> --
> 2.17.1
>

2022-09-07 06:50:20

by Peter Harliman Liem

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] crypto: inside_secure - Avoid dma map if size is zero

On 6/9/2022 10:01 pm, Antoine Tenart wrote:
> Quoting Peter Harliman Liem (2022-09-06 04:51:49)
>> From commit d03c54419274 ("dma-mapping: disallow .map_sg
>> operations from returning zero on error"), dma_map_sg()
>> produces warning if size is 0. This results in visible
>> warnings if crypto length is zero.
>> To avoid that, we avoid calling dma_map_sg if size is zero.
>>
>> Fixes: d03c54419274 ("dma-mapping: disallow .map_sg operations from returning zero on error")
>
> You can't reference the commit above, it's not introducing the issue but
> the warning itself. The actual commit introducing the below logic should
> be referenced.
>
> Alternatively since the warning was introduced latter than the logic and
> this is not a huge issue, you might resend it w/o the Fixes tag as well.
Noted.
I will remove the tag in v3.

Thanks!



2022-09-07 07:01:30

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] crypto: inside_secure - Avoid dma map if size is zero

On Tue, Sep 06, 2022 at 10:51:49AM +0800, Peter Harliman Liem wrote:
>
> diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
> index d68ef16650d4..3775497775e0 100644
> --- a/drivers/crypto/inside-secure/safexcel_cipher.c
> +++ b/drivers/crypto/inside-secure/safexcel_cipher.c
> @@ -737,14 +737,17 @@ static int safexcel_send_req(struct crypto_async_request *base, int ring,
> max(totlen_src, totlen_dst));
> return -EINVAL;
> }
> - dma_map_sg(priv->dev, src, sreq->nr_src, DMA_BIDIRECTIONAL);
> + if (sreq->nr_src > 0)
> + dma_map_sg(priv->dev, src, sreq->nr_src, DMA_BIDIRECTIONAL);

Where is the corresponding check on unmap?

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2022-09-23 02:44:54

by Peter Harliman Liem

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] crypto: inside_secure - Avoid dma map if size is zero

On 7/9/2022 2:52 pm, Herbert Xu wrote:
> On Tue, Sep 06, 2022 at 10:51:49AM +0800, Peter Harliman Liem wrote:
> >
> > diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c
> b/drivers/crypto/inside-secure/safexcel_cipher.c
> > index d68ef16650d4..3775497775e0 100644
> > --- a/drivers/crypto/inside-secure/safexcel_cipher.c
> > +++ b/drivers/crypto/inside-secure/safexcel_cipher.c
> > @@ -737,14 +737,17 @@ static int safexcel_send_req(struct
> crypto_async_request *base, int ring,
> > max(totlen_src, totlen_dst));
> > return -EINVAL;
> > }
> > - dma_map_sg(priv->dev, src, sreq->nr_src, DMA_BIDIRECTIONAL);
> > + if (sreq->nr_src > 0)
> > + dma_map_sg(priv->dev, src, sreq->nr_src, DMA_BIDIRECTIONAL);
>
> Where is the corresponding check on unmap?

Added in v3.

Thanks!