2022-01-11 12:41:23

by Fabio Estevam

[permalink] [raw]
Subject: [PATCH] crypto: caam - enable prediction resistance conditionally

From: Fabio Estevam <[email protected]>

Since commit 358ba762d9f1 ("crypto: caam - enable prediction resistance
in HRWNG") the following CAAM errors can be seen on i.MX6:

caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
hwrng: no data available
caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
hwrng: no data available
caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
hwrng: no data available
caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
hwrng: no data available

OP_ALG_PR_ON is enabled unconditionally, which may cause the problem
on i.MX devices.

Fix the problem by only enabling OP_ALG_PR_ON on platforms that have
Management Complex support.

Fixes: 358ba762d9f1 ("crypto: caam - enable prediction resistance in HRWNG")
Signed-off-by: Fabio Estevam <[email protected]>
---
drivers/crypto/caam/caamrng.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
index 77d048dfe5d0..3514fe5de2a5 100644
--- a/drivers/crypto/caam/caamrng.c
+++ b/drivers/crypto/caam/caamrng.c
@@ -63,12 +63,19 @@ static void caam_rng_done(struct device *jrdev, u32 *desc, u32 err,
complete(jctx->done);
}

-static u32 *caam_init_desc(u32 *desc, dma_addr_t dst_dma)
+static u32 *caam_init_desc(struct device *jrdev, u32 *desc, dma_addr_t dst_dma)
{
+ struct caam_drv_private *priv = dev_get_drvdata(jrdev->parent);
+
init_job_desc(desc, 0); /* + 1 cmd_sz */
/* Generate random bytes: + 1 cmd_sz */
- append_operation(desc, OP_ALG_ALGSEL_RNG | OP_TYPE_CLASS1_ALG |
- OP_ALG_PR_ON);
+
+ if (priv->mc_en)
+ append_operation(desc, OP_ALG_ALGSEL_RNG | OP_TYPE_CLASS1_ALG |
+ OP_ALG_PR_ON);
+ else
+ append_operation(desc, OP_ALG_ALGSEL_RNG | OP_TYPE_CLASS1_ALG);
+
/* Store bytes: + 1 cmd_sz + caam_ptr_sz */
append_fifo_store(desc, dst_dma,
CAAM_RNG_MAX_FIFO_STORE_SIZE, FIFOST_TYPE_RNGSTORE);
@@ -101,7 +108,7 @@ static int caam_rng_read_one(struct device *jrdev,

init_completion(done);
err = caam_jr_enqueue(jrdev,
- caam_init_desc(desc, dst_dma),
+ caam_init_desc(jrdev, desc, dst_dma),
caam_rng_done, &jctx);
if (err == -EINPROGRESS) {
wait_for_completion(done);
--
2.25.1



2022-01-11 18:21:19

by Andrey Smirnov

[permalink] [raw]
Subject: Re: [PATCH] crypto: caam - enable prediction resistance conditionally

On Tue, Jan 11, 2022 at 4:41 AM Fabio Estevam <[email protected]> wrote:
>
> From: Fabio Estevam <[email protected]>
>
> Since commit 358ba762d9f1 ("crypto: caam - enable prediction resistance
> in HRWNG") the following CAAM errors can be seen on i.MX6:
>
> caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
> hwrng: no data available
> caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
> hwrng: no data available
> caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
> hwrng: no data available
> caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
> hwrng: no data available
>
> OP_ALG_PR_ON is enabled unconditionally, which may cause the problem
> on i.MX devices.

Is this true for every i.MX device? I haven't worked with the
i.MX6Q/i.MX8 hardware I was enabling this feature for in a while, so
I'm not 100% up to date on all of the problems we've seen with those,
but last time enabling prediction resistance didn't seem to cause any
issues besides a noticeable slowdown of random data generation.

Can this be a Kconfig option or maybe a runtime flag so that it'd
still be possible for some i.MX users to keep PR enabled?

>
> Fix the problem by only enabling OP_ALG_PR_ON on platforms that have
> Management Complex support.
>
> Fixes: 358ba762d9f1 ("crypto: caam - enable prediction resistance in HRWNG")
> Signed-off-by: Fabio Estevam <[email protected]>
> ---
> drivers/crypto/caam/caamrng.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
> index 77d048dfe5d0..3514fe5de2a5 100644
> --- a/drivers/crypto/caam/caamrng.c
> +++ b/drivers/crypto/caam/caamrng.c
> @@ -63,12 +63,19 @@ static void caam_rng_done(struct device *jrdev, u32 *desc, u32 err,
> complete(jctx->done);
> }
>
> -static u32 *caam_init_desc(u32 *desc, dma_addr_t dst_dma)
> +static u32 *caam_init_desc(struct device *jrdev, u32 *desc, dma_addr_t dst_dma)
> {
> + struct caam_drv_private *priv = dev_get_drvdata(jrdev->parent);
> +
> init_job_desc(desc, 0); /* + 1 cmd_sz */
> /* Generate random bytes: + 1 cmd_sz */
> - append_operation(desc, OP_ALG_ALGSEL_RNG | OP_TYPE_CLASS1_ALG |
> - OP_ALG_PR_ON);
> +
> + if (priv->mc_en)
> + append_operation(desc, OP_ALG_ALGSEL_RNG | OP_TYPE_CLASS1_ALG |
> + OP_ALG_PR_ON);
> + else
> + append_operation(desc, OP_ALG_ALGSEL_RNG | OP_TYPE_CLASS1_ALG);
> +
> /* Store bytes: + 1 cmd_sz + caam_ptr_sz */
> append_fifo_store(desc, dst_dma,
> CAAM_RNG_MAX_FIFO_STORE_SIZE, FIFOST_TYPE_RNGSTORE);
> @@ -101,7 +108,7 @@ static int caam_rng_read_one(struct device *jrdev,
>
> init_completion(done);
> err = caam_jr_enqueue(jrdev,
> - caam_init_desc(desc, dst_dma),
> + caam_init_desc(jrdev, desc, dst_dma),
> caam_rng_done, &jctx);
> if (err == -EINPROGRESS) {
> wait_for_completion(done);
> --
> 2.25.1
>

2022-01-11 18:35:04

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] crypto: caam - enable prediction resistance conditionally

Hi Andrey,

On Tue, Jan 11, 2022 at 3:21 PM Andrey Smirnov <[email protected]> wrote:

> Is this true for every i.MX device? I haven't worked with the

I do see the problem on i.MX6SX.

This thread reports the same problem on i.MX6D:
https://www.spinics.net/lists/linux-crypto/msg52319.html

> i.MX6Q/i.MX8 hardware I was enabling this feature for in a while, so
> I'm not 100% up to date on all of the problems we've seen with those,
> but last time enabling prediction resistance didn't seem to cause any
> issues besides a noticeable slowdown of random data generation.
>
> Can this be a Kconfig option or maybe a runtime flag so that it'd
> still be possible for some i.MX users to keep PR enabled?

The problem is that I don't know when it is safe or not to enable PR.

What about introducing a boolean devicetree property that when
present, disables prediction resistance?

2022-01-11 19:05:50

by Andrey Smirnov

[permalink] [raw]
Subject: Re: [PATCH] crypto: caam - enable prediction resistance conditionally

On Tue, Jan 11, 2022 at 10:35 AM Fabio Estevam <[email protected]> wrote:
>
> Hi Andrey,
>
> On Tue, Jan 11, 2022 at 3:21 PM Andrey Smirnov <[email protected]> wrote:
>
> > Is this true for every i.MX device? I haven't worked with the
>
> I do see the problem on i.MX6SX.
>
> This thread reports the same problem on i.MX6D:
> https://www.spinics.net/lists/linux-crypto/msg52319.html
>
> > i.MX6Q/i.MX8 hardware I was enabling this feature for in a while, so
> > I'm not 100% up to date on all of the problems we've seen with those,
> > but last time enabling prediction resistance didn't seem to cause any
> > issues besides a noticeable slowdown of random data generation.
> >
> > Can this be a Kconfig option or maybe a runtime flag so that it'd
> > still be possible for some i.MX users to keep PR enabled?
>
> The problem is that I don't know when it is safe or not to enable PR.
>

Yeah, I hear you. It sounds like long term, we'll need some advice
from HW folks on this. I don't have any FAE contacts anymore, but
maybe you or Horia do have a venue to pursue this?

> What about introducing a boolean devicetree property that when
> present, disables prediction resistance?

That sounds fair.

2022-01-17 17:09:21

by Varun Sethi

[permalink] [raw]
Subject: Re:[PATCH] crypto: caam - enable prediction resistance conditionally

Hi Fabio,
Sure, we will copy you once the fix is submitted.


Regards
Varun

-----Original Message-----
From: Fabio Estevam <[email protected]>
Sent: Friday, January 14, 2022 10:47 PM
To: Varun Sethi <[email protected]>
Cc: [email protected]; [email protected]; Horia Geanta <[email protected]>; Gaurav Jain <[email protected]>; Pankaj Gupta <[email protected]>
Subject: [EXT] Re:

Caution: EXT Email

Hi Varun,

On Thu, Jan 13, 2022 at 2:53 PM Varun Sethi <[email protected]> wrote:
>
> Hi Fabio, Andrey,
> So far we have observed this issue on i.MX6 only. Disabling prediction resistance isn't the solution for the problem. We are working on identifying the proper fix for this issue and would post the patch for the same.

Please copy me when you submit a fix for this issue.

Thanks!

Fabio Estevam