crypto: caam - enable prediction resistance conditionally

PATCH In review Fabio Estevam 5 participants 13 messages Jan 11, 2022 – Apr 16, 2022 1 message not in the archive

Subsystems (2)

Crypto API Freescale CAAM (Cryptographic Acceleration and Assurance Module) driver

lore

Share & tools

lore

Fabio Estevam

author

2022-01-11 12:41 UTC #0 lore raw

PATCH 1/1 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(-)

drivers/crypto/caam/caamrng.c +11 -4

index 77d048dfe5d0..3514fe5de2a5 100644
@@ -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);

Signature · 1 line

2.25.1

Andrey Smirnov

2022-01-11 18:21 UTC #1 lore raw

Quoted 15 lines · Fabio Estevam

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

Quoted 2 lines

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?

Quoted 49 lines

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(-)

drivers/crypto/caam/caamrng.c +11 -5

index 77d048dfe5d0..3514fe5de2a5 100644
@@ -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

Fabio Estevam

author

2022-01-11 18:35 UTC #2 lore raw

Hi Andrey,

Quoted 1 line · Andrey Smirnov

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

Quoted 7 lines

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?

Andrey Smirnov

2022-01-11 19:05 UTC #3 lore raw

Quoted 17 lines · Fabio Estevam

Hi Andrey,
Quoted 1 line · Andrey Smirnov
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
Quoted 5 lines
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.

Quoted 5 lines

Quoted 2 lines
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?

Quoted 2 lines

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

That sounds fair.

Herbert Xu

maintainer

2022-01-28 06:23 UTC #4 lore raw

In reply to Fabio Estevam

Quoted 17 lines · Fabio Estevam

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.

Quoted 8 lines

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(-)

Patch applied. Thanks.

Signature · 3 lines

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

Horia Geantă

maintainer

2022-01-28 07:44 UTC #5 lore raw

Quoted 15 lines · Herbert Xu

Quoted 14 lines · Fabio Estevam
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

Quoted 3 lines

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

What parts exactly?
Anything besides i.MX6 SX, S/DL?

Quoted 3 lines

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

This limitation doesn't make any sense, it's too general. Only a handful of Layerscape devices have MC, so all i.MX devices and most LS devices will no longer have prediction resistance enabled.

Quoted 7 lines

Quoted 5 lines
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(-)
Patch applied. Thanks.

We've been in contact with Fabio and we're working on a solution. Now I realize the list hasn't been Cc-ed - sorry for the confusion and for not providing an explicit Nack.

Herbert, could you please revert this patch?

It's doing more harm than good, since it's making the internal CAAM RNG work like a DRBG / PRNG (instead of TRNG) while the driver registers to hwrng as an entropy source.

Thanks,
Horia

Fabio Estevam

author

2022-01-28 11:35 UTC #6 lore raw

Hi Horia,

Quoted 2 lines · Horia Geantă

What parts exactly?
Anything besides i.MX6 SX, S/DL?

I see it on i.MX6SX, but in this report, it is mentioned i.MX6D:
https://www.spinics.net/lists/linux-crypto/msg52319.html

Lucas always saw it on i.MX6D:
https://linuxlists.cc/l/4/linux-crypto/t/3843436/caam_rng_trouble

Quoted 3 lines

We've been in contact with Fabio and we're working on a solution. Now I realize the list hasn't been Cc-ed - sorry for the confusion and for not providing an explicit Nack.

Varun on Cc said he would work on a proper solution as he was able to reproduce it.

Any progress, Varun?

Thanks

Herbert Xu

maintainer

2022-01-31 00:20 UTC #7 lore raw

In reply to Horia Geantă

Quoted 2 lines · Horia Geantă

Herbert, could you please revert this patch?

OK I will revert this.

Thanks,

Signature · 3 lines

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

Fabio Estevam

author

2022-03-22 13:20 UTC #8 lore raw

In reply to Horia Geantă

Hi Horia and Varun,

Quoted 9 lines · Horia Geantă

We've been in contact with Fabio and we're working on a solution. Now I realize the list hasn't been Cc-ed - sorry for the confusion and for not providing an explicit Nack.
Herbert, could you please revert this patch?
It's doing more harm than good, since it's making the internal CAAM RNG work like a DRBG / PRNG (instead of TRNG) while the driver registers to hwrng as an entropy source.

Any progress on the proper fix for this issue?

Thanks

Varun Sethi

2022-03-22 13:56 UTC #9 lore raw

Hi Fabio,

Quoted 21 lines

-----Original Message-----
From: Fabio Estevam <[email protected]>
Sent: Tuesday, March 22, 2022 6:50 PM
To: Horia Geanta <[email protected]>; Varun Sethi <[email protected]>
Cc: Herbert Xu <[email protected]>; Andrei Botila <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Fabio Estevam <[email protected]>
Subject: [EXT] Re: [PATCH] crypto: caam - enable prediction resistance conditionally
Caution: EXT Email
Hi Horia and Varun,
Quoted 4 lines · Horia Geantă
We've been in contact with Fabio and we're working on a solution. Now I realize the list hasn't been Cc-ed - sorry for the confusion and for not providing an explicit Nack.

Quoted 8 lines

Quoted 5 lines
Herbert, could you please revert this patch?
It's doing more harm than good, since it's making the internal CAAM RNG work like a DRBG / PRNG (instead of TRNG) while the driver registers to hwrng as an entropy source.
Any progress on the proper fix for this issue?

[Varun] Yes, we have made progress on the fix. Currently we are testing the fix and should be able to post the patch upstream pretty soon.

Quoted 1 line

Thanks

Regards
Varun

Fabio Estevam

author

2022-04-14 15:57 UTC #10 lore raw

Hi Varun,

Quoted 1 line · Varun Sethi

[Varun] Yes, we have made progress on the fix. Currently we are testing the fix and should be able to post the patch upstream pretty soon.

Any progress on the fix?

Thanks

A message that is not in the archive (sent off-list or not yet received): AM9PR04MB82119651D9FC652BD982646DE8EF9@AM9PR04MB8211.eurprd04.prod.outlook.com

Fabio Estevam

author

2022-04-16 12:24 UTC #12 lore raw

Hi Varun,

Quoted 2 lines · Varun Sethi

Yes Fabio, we will be posting patch by next week.

Is the kernel patch that you plan to send along the lines of the following U-Boot patch?
https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/

Thanks

Fabio Estevam

author

2022-04-16 13:28 UTC #13 lore raw

Hi Varun,

Quoted 3 lines · Fabio Estevam

Is the kernel patch that you plan to send along the lines of the following U-Boot patch?
https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/

Following the U-Boot fix, the kernel patch would look like this:

drivers/crypto/caam/ctrl.c +4 -0

@@ -648,6 +648,8 @@ static int caam_probe(struct platform_device *pdev)
                        return ret;
        }
 
+       if (of_machine_is_compatible("fsl,imx6sx"))
+               ent_delay = 12000;
 
        /* Get configuration properties from device tree */
        /* First, get register page */

@@ -871,6 +873,8 @@ static int caam_probe(struct platform_device *pdev)
                         */
                        ret = instantiate_rng(dev, inst_handles,
                                              gen_sk);
+                       if (of_machine_is_compatible("fsl,imx6sx"))
+                               break;
                        if (ret == -EAGAIN)
                                /*
                                 * if here, the loop will rerun,

What do you think?