* [PATCH] crypto: crypto4xx - Remove insecure and unused rng_alg
@ 2026-05-29 22:04 Eric Biggers
2026-05-30 10:20 ` Christian Lamparter
2026-05-30 15:05 ` Aleksander Jan Bajkowski
0 siblings, 2 replies; 6+ messages in thread
From: Eric Biggers @ 2026-05-29 22:04 UTC (permalink / raw)
To: linux-crypto, Herbert Xu
Cc: Christian Lamparter, linuxppc-dev, linux-kernel, Eric Biggers,
stable
Remove crypto4xx_rng, as it is insecure and unused:
- It has only a 64-bit security strength, which is highly inadequate.
This can be seen by the fact that crypto4xx_hw_init() seeds it with
only 64 bits of entropy, and the fact that the original commit
mentions that it implements ANSI X9.17 Annex C.
Another issue was that this driver didn't implement the crypto_rng API
correctly, as crypto4xx_prng_generate() didn't return 0 on success.
- No user of this code is known. It's usable only theoretically via the
"rng" algorithm type of AF_ALG. But userspace actually just uses the
actual Linux RNG (/dev/random etc) instead. And rng_algs don't
contribute entropy to the actual Linux RNG either. (This may have
been confused with hwrng, which does contribute entropy.)
Fixes: d072bfa48853 ("crypto: crypto4xx - add prng crypto support")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
drivers/crypto/Kconfig | 1 -
drivers/crypto/amcc/crypto4xx_core.c | 88 -------------------------
drivers/crypto/amcc/crypto4xx_core.h | 4 --
drivers/crypto/amcc/crypto4xx_reg_def.h | 11 ----
4 files changed, 104 deletions(-)
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 3449b3c9c6ad..5dab813a9f74 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -299,11 +299,10 @@ config CRYPTO_DEV_PPC4XX
select CRYPTO_AES
select CRYPTO_LIB_AES
select CRYPTO_CCM
select CRYPTO_CTR
select CRYPTO_GCM
- select CRYPTO_RNG
select CRYPTO_SKCIPHER
help
This option allows you to have support for AMCC crypto acceleration.
config HW_RANDOM_PPC4XX
diff --git a/drivers/crypto/amcc/crypto4xx_core.c b/drivers/crypto/amcc/crypto4xx_core.c
index b7b6c97d2147..68c5ff7a85b4 100644
--- a/drivers/crypto/amcc/crypto4xx_core.c
+++ b/drivers/crypto/amcc/crypto4xx_core.c
@@ -29,15 +29,13 @@
#include <crypto/aead.h>
#include <crypto/aes.h>
#include <crypto/ctr.h>
#include <crypto/gcm.h>
#include <crypto/sha1.h>
-#include <crypto/rng.h>
#include <crypto/scatterwalk.h>
#include <crypto/skcipher.h>
#include <crypto/internal/aead.h>
-#include <crypto/internal/rng.h>
#include <crypto/internal/skcipher.h>
#include "crypto4xx_reg_def.h"
#include "crypto4xx_core.h"
#include "crypto4xx_sa.h"
#include "crypto4xx_trng.h"
@@ -983,14 +981,10 @@ static int crypto4xx_register_alg(struct crypto4xx_device *sec_dev,
switch (alg->alg.type) {
case CRYPTO_ALG_TYPE_AEAD:
rc = crypto_register_aead(&alg->alg.u.aead);
break;
- case CRYPTO_ALG_TYPE_RNG:
- rc = crypto_register_rng(&alg->alg.u.rng);
- break;
-
default:
rc = crypto_register_skcipher(&alg->alg.u.cipher);
break;
}
@@ -1012,14 +1006,10 @@ static void crypto4xx_unregister_alg(struct crypto4xx_device *sec_dev)
switch (alg->alg.type) {
case CRYPTO_ALG_TYPE_AEAD:
crypto_unregister_aead(&alg->alg.u.aead);
break;
- case CRYPTO_ALG_TYPE_RNG:
- crypto_unregister_rng(&alg->alg.u.rng);
- break;
-
default:
crypto_unregister_skcipher(&alg->alg.u.cipher);
}
kfree(alg);
}
@@ -1074,73 +1064,10 @@ static irqreturn_t crypto4xx_ce_interrupt_handler_revb(int irq, void *data)
{
return crypto4xx_interrupt_handler(irq, data, PPC4XX_INTERRUPT_CLR |
PPC4XX_TMO_ERR_INT);
}
-static int ppc4xx_prng_data_read(struct crypto4xx_device *dev,
- u8 *data, unsigned int max)
-{
- unsigned int i, curr = 0;
- u32 val[2];
-
- do {
- /* trigger PRN generation */
- writel(PPC4XX_PRNG_CTRL_AUTO_EN,
- dev->ce_base + CRYPTO4XX_PRNG_CTRL);
-
- for (i = 0; i < 1024; i++) {
- /* usually 19 iterations are enough */
- if ((readl(dev->ce_base + CRYPTO4XX_PRNG_STAT) &
- CRYPTO4XX_PRNG_STAT_BUSY))
- continue;
-
- val[0] = readl_be(dev->ce_base + CRYPTO4XX_PRNG_RES_0);
- val[1] = readl_be(dev->ce_base + CRYPTO4XX_PRNG_RES_1);
- break;
- }
- if (i == 1024)
- return -ETIMEDOUT;
-
- if ((max - curr) >= 8) {
- memcpy(data, &val, 8);
- data += 8;
- curr += 8;
- } else {
- /* copy only remaining bytes */
- memcpy(data, &val, max - curr);
- break;
- }
- } while (curr < max);
-
- return curr;
-}
-
-static int crypto4xx_prng_generate(struct crypto_rng *tfm,
- const u8 *src, unsigned int slen,
- u8 *dstn, unsigned int dlen)
-{
- struct rng_alg *alg = crypto_rng_alg(tfm);
- struct crypto4xx_alg *amcc_alg;
- struct crypto4xx_device *dev;
- int ret;
-
- amcc_alg = container_of(alg, struct crypto4xx_alg, alg.u.rng);
- dev = amcc_alg->dev;
-
- mutex_lock(&dev->core_dev->rng_lock);
- ret = ppc4xx_prng_data_read(dev, dstn, dlen);
- mutex_unlock(&dev->core_dev->rng_lock);
- return ret;
-}
-
-
-static int crypto4xx_prng_seed(struct crypto_rng *tfm, const u8 *seed,
- unsigned int slen)
-{
- return 0;
-}
-
/*
* Supported Crypto Algorithms
*/
static struct crypto4xx_alg_common crypto4xx_alg[] = {
/* Crypto AES modes */
@@ -1266,22 +1193,10 @@ static struct crypto4xx_alg_common crypto4xx_alg[] = {
.cra_blocksize = 1,
.cra_ctxsize = sizeof(struct crypto4xx_ctx),
.cra_module = THIS_MODULE,
},
} },
- { .type = CRYPTO_ALG_TYPE_RNG, .u.rng = {
- .base = {
- .cra_name = "stdrng",
- .cra_driver_name = "crypto4xx_rng",
- .cra_priority = 300,
- .cra_ctxsize = 0,
- .cra_module = THIS_MODULE,
- },
- .generate = crypto4xx_prng_generate,
- .seed = crypto4xx_prng_seed,
- .seedsize = 0,
- } },
};
/*
* Module Initialization Routine
*/
@@ -1351,13 +1266,10 @@ static int crypto4xx_probe(struct platform_device *ofdev)
}
core_dev->dev->core_dev = core_dev;
core_dev->dev->is_revb = is_revb;
core_dev->device = dev;
- rc = devm_mutex_init(&ofdev->dev, &core_dev->rng_lock);
- if (rc)
- return rc;
spin_lock_init(&core_dev->lock);
INIT_LIST_HEAD(&core_dev->dev->alg_list);
ratelimit_default_init(&core_dev->dev->aead_ratelimit);
rc = crypto4xx_build_sdr(core_dev->dev);
if (rc)
diff --git a/drivers/crypto/amcc/crypto4xx_core.h b/drivers/crypto/amcc/crypto4xx_core.h
index ee36630c670f..3a028aec3f0c 100644
--- a/drivers/crypto/amcc/crypto4xx_core.h
+++ b/drivers/crypto/amcc/crypto4xx_core.h
@@ -12,14 +12,12 @@
#ifndef __CRYPTO4XX_CORE_H__
#define __CRYPTO4XX_CORE_H__
#include <linux/ratelimit.h>
-#include <linux/mutex.h>
#include <linux/scatterlist.h>
#include <crypto/internal/aead.h>
-#include <crypto/internal/rng.h>
#include <crypto/internal/skcipher.h>
#include "crypto4xx_reg_def.h"
#include "crypto4xx_sa.h"
#define PPC460SX_SDR0_SRST 0x201
@@ -109,11 +107,10 @@ struct crypto4xx_core_device {
struct hwrng *trng;
u32 int_status;
u32 irq;
struct tasklet_struct tasklet;
spinlock_t lock;
- struct mutex rng_lock;
};
struct crypto4xx_ctx {
struct crypto4xx_device *dev;
struct dynamic_sa_ctl *sa_in;
@@ -133,11 +130,10 @@ struct crypto4xx_aead_reqctx {
struct crypto4xx_alg_common {
u32 type;
union {
struct skcipher_alg cipher;
struct aead_alg aead;
- struct rng_alg rng;
} u;
};
struct crypto4xx_alg {
struct list_head entry;
diff --git a/drivers/crypto/amcc/crypto4xx_reg_def.h b/drivers/crypto/amcc/crypto4xx_reg_def.h
index 1038061224da..73d626308a84 100644
--- a/drivers/crypto/amcc/crypto4xx_reg_def.h
+++ b/drivers/crypto/amcc/crypto4xx_reg_def.h
@@ -88,24 +88,13 @@
#define CRYPTO4XX_DMA_CFG 0x000600d4
#define CRYPTO4XX_BYTE_ORDER_CFG 0x000600d8
#define CRYPTO4XX_ENDIAN_CFG 0x000600d8
-#define CRYPTO4XX_PRNG_STAT 0x00070000
-#define CRYPTO4XX_PRNG_STAT_BUSY 0x1
#define CRYPTO4XX_PRNG_CTRL 0x00070004
#define CRYPTO4XX_PRNG_SEED_L 0x00070008
#define CRYPTO4XX_PRNG_SEED_H 0x0007000c
-
-#define CRYPTO4XX_PRNG_RES_0 0x00070020
-#define CRYPTO4XX_PRNG_RES_1 0x00070024
-#define CRYPTO4XX_PRNG_RES_2 0x00070028
-#define CRYPTO4XX_PRNG_RES_3 0x0007002C
-
-#define CRYPTO4XX_PRNG_LFSR_L 0x00070030
-#define CRYPTO4XX_PRNG_LFSR_H 0x00070034
-
/*
* Initialize CRYPTO ENGINE registers, and memory bases.
*/
#define PPC4XX_PDR_POLL 0x3ff
#define PPC4XX_OUTPUT_THRESHOLD 2
base-commit: 49e05bb00f2e8168695f7af4d694c39e1423e8a2
--
2.54.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] crypto: crypto4xx - Remove insecure and unused rng_alg
2026-05-29 22:04 [PATCH] crypto: crypto4xx - Remove insecure and unused rng_alg Eric Biggers
@ 2026-05-30 10:20 ` Christian Lamparter
2026-05-30 19:12 ` Eric Biggers
2026-05-30 15:05 ` Aleksander Jan Bajkowski
1 sibling, 1 reply; 6+ messages in thread
From: Christian Lamparter @ 2026-05-30 10:20 UTC (permalink / raw)
To: Eric Biggers, linux-crypto, Herbert Xu; +Cc: linuxppc-dev, linux-kernel, stable
Hi!
On 5/30/26 12:04 AM, Eric Biggers wrote:
> Remove crypto4xx_rng, as it is insecure and unused:
>
> - It has only a 64-bit security strength, which is highly inadequate.
> This can be seen by the fact that crypto4xx_hw_init() seeds it with
> only 64 bits of entropy, and the fact that the original commit
> mentions that it implements ANSI X9.17 Annex C.
Yes, that "ANSI X9.17 Annex C" comes from the datasheet for the PRNG.
> Another issue was that this driver didn't implement the crypto_rng API
> correctly, as crypto4xx_prng_generate() didn't return 0 on success.
Oh! Hmm, I think I copied that "return amount;" from another driver that
had it implemented? But I'm not sure, this was sooo long ago. That said,
if this never worked...
> - No user of this code is known. It's usable only theoretically via the
> "rng" algorithm type of AF_ALG. But userspace actually just uses the
> actual Linux RNG (/dev/random etc) instead. And rng_algs don't
> contribute entropy to the actual Linux RNG either. (This may have
> been confused with hwrng, which does contribute entropy.)
... and it's completely redundant: Sure!
just in case, this counts for anything, but as the person that added it in the
first place:
Acked-by: Christian Lamparter <chunkeey@gmail.com>
> Fixes: d072bfa48853 ("crypto: crypto4xx - add prng crypto support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
> drivers/crypto/Kconfig | 1 -
> drivers/crypto/amcc/crypto4xx_core.c | 88 -------------------------
> drivers/crypto/amcc/crypto4xx_core.h | 4 --
> drivers/crypto/amcc/crypto4xx_reg_def.h | 11 ----
> 4 files changed, 104 deletions(-)
>
> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index 3449b3c9c6ad..5dab813a9f74 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -299,11 +299,10 @@ config CRYPTO_DEV_PPC4XX
> select CRYPTO_AES
> select CRYPTO_LIB_AES
> select CRYPTO_CCM
> select CRYPTO_CTR
> select CRYPTO_GCM
> - select CRYPTO_RNG
> select CRYPTO_SKCIPHER
> help
> This option allows you to have support for AMCC crypto acceleration.
>
> config HW_RANDOM_PPC4XX
> diff --git a/drivers/crypto/amcc/crypto4xx_core.c b/drivers/crypto/amcc/crypto4xx_core.c
> index b7b6c97d2147..68c5ff7a85b4 100644
> --- a/drivers/crypto/amcc/crypto4xx_core.c
> +++ b/drivers/crypto/amcc/crypto4xx_core.c
> @@ -29,15 +29,13 @@
> #include <crypto/aead.h>
> #include <crypto/aes.h>
> #include <crypto/ctr.h>
> #include <crypto/gcm.h>
> #include <crypto/sha1.h>
> -#include <crypto/rng.h>
> #include <crypto/scatterwalk.h>
> #include <crypto/skcipher.h>
> #include <crypto/internal/aead.h>
> -#include <crypto/internal/rng.h>
> #include <crypto/internal/skcipher.h>
> #include "crypto4xx_reg_def.h"
> #include "crypto4xx_core.h"
> #include "crypto4xx_sa.h"
> #include "crypto4xx_trng.h"
> @@ -983,14 +981,10 @@ static int crypto4xx_register_alg(struct crypto4xx_device *sec_dev,
> switch (alg->alg.type) {
> case CRYPTO_ALG_TYPE_AEAD:
> rc = crypto_register_aead(&alg->alg.u.aead);
> break;
>
> - case CRYPTO_ALG_TYPE_RNG:
> - rc = crypto_register_rng(&alg->alg.u.rng);
> - break;
> -
> default:
> rc = crypto_register_skcipher(&alg->alg.u.cipher);
> break;
> }
>
> @@ -1012,14 +1006,10 @@ static void crypto4xx_unregister_alg(struct crypto4xx_device *sec_dev)
> switch (alg->alg.type) {
> case CRYPTO_ALG_TYPE_AEAD:
> crypto_unregister_aead(&alg->alg.u.aead);
> break;
>
> - case CRYPTO_ALG_TYPE_RNG:
> - crypto_unregister_rng(&alg->alg.u.rng);
> - break;
> -
> default:
> crypto_unregister_skcipher(&alg->alg.u.cipher);
> }
> kfree(alg);
> }
> @@ -1074,73 +1064,10 @@ static irqreturn_t crypto4xx_ce_interrupt_handler_revb(int irq, void *data)
> {
> return crypto4xx_interrupt_handler(irq, data, PPC4XX_INTERRUPT_CLR |
> PPC4XX_TMO_ERR_INT);
> }
>
> -static int ppc4xx_prng_data_read(struct crypto4xx_device *dev,
> - u8 *data, unsigned int max)
> -{
> - unsigned int i, curr = 0;
> - u32 val[2];
> -
> - do {
> - /* trigger PRN generation */
> - writel(PPC4XX_PRNG_CTRL_AUTO_EN,
> - dev->ce_base + CRYPTO4XX_PRNG_CTRL);
> -
> - for (i = 0; i < 1024; i++) {
> - /* usually 19 iterations are enough */
> - if ((readl(dev->ce_base + CRYPTO4XX_PRNG_STAT) &
> - CRYPTO4XX_PRNG_STAT_BUSY))
> - continue;
> -
> - val[0] = readl_be(dev->ce_base + CRYPTO4XX_PRNG_RES_0);
> - val[1] = readl_be(dev->ce_base + CRYPTO4XX_PRNG_RES_1);
> - break;
> - }
> - if (i == 1024)
> - return -ETIMEDOUT;
> -
> - if ((max - curr) >= 8) {
> - memcpy(data, &val, 8);
> - data += 8;
> - curr += 8;
> - } else {
> - /* copy only remaining bytes */
> - memcpy(data, &val, max - curr);
> - break;
> - }
> - } while (curr < max);
> -
> - return curr;
> -}
> -
> -static int crypto4xx_prng_generate(struct crypto_rng *tfm,
> - const u8 *src, unsigned int slen,
> - u8 *dstn, unsigned int dlen)
> -{
> - struct rng_alg *alg = crypto_rng_alg(tfm);
> - struct crypto4xx_alg *amcc_alg;
> - struct crypto4xx_device *dev;
> - int ret;
> -
> - amcc_alg = container_of(alg, struct crypto4xx_alg, alg.u.rng);
> - dev = amcc_alg->dev;
> -
> - mutex_lock(&dev->core_dev->rng_lock);
> - ret = ppc4xx_prng_data_read(dev, dstn, dlen);
> - mutex_unlock(&dev->core_dev->rng_lock);
> - return ret;
> -}
> -
> -
> -static int crypto4xx_prng_seed(struct crypto_rng *tfm, const u8 *seed,
> - unsigned int slen)
> -{
> - return 0;
> -}
> -
> /*
> * Supported Crypto Algorithms
> */
> static struct crypto4xx_alg_common crypto4xx_alg[] = {
> /* Crypto AES modes */
> @@ -1266,22 +1193,10 @@ static struct crypto4xx_alg_common crypto4xx_alg[] = {
> .cra_blocksize = 1,
> .cra_ctxsize = sizeof(struct crypto4xx_ctx),
> .cra_module = THIS_MODULE,
> },
> } },
> - { .type = CRYPTO_ALG_TYPE_RNG, .u.rng = {
> - .base = {
> - .cra_name = "stdrng",
> - .cra_driver_name = "crypto4xx_rng",
> - .cra_priority = 300,
> - .cra_ctxsize = 0,
> - .cra_module = THIS_MODULE,
> - },
> - .generate = crypto4xx_prng_generate,
> - .seed = crypto4xx_prng_seed,
> - .seedsize = 0,
> - } },
> };
>
> /*
> * Module Initialization Routine
> */
> @@ -1351,13 +1266,10 @@ static int crypto4xx_probe(struct platform_device *ofdev)
> }
>
> core_dev->dev->core_dev = core_dev;
> core_dev->dev->is_revb = is_revb;
> core_dev->device = dev;
> - rc = devm_mutex_init(&ofdev->dev, &core_dev->rng_lock);
> - if (rc)
> - return rc;
> spin_lock_init(&core_dev->lock);
> INIT_LIST_HEAD(&core_dev->dev->alg_list);
> ratelimit_default_init(&core_dev->dev->aead_ratelimit);
> rc = crypto4xx_build_sdr(core_dev->dev);
> if (rc)
> diff --git a/drivers/crypto/amcc/crypto4xx_core.h b/drivers/crypto/amcc/crypto4xx_core.h
> index ee36630c670f..3a028aec3f0c 100644
> --- a/drivers/crypto/amcc/crypto4xx_core.h
> +++ b/drivers/crypto/amcc/crypto4xx_core.h
> @@ -12,14 +12,12 @@
>
> #ifndef __CRYPTO4XX_CORE_H__
> #define __CRYPTO4XX_CORE_H__
>
> #include <linux/ratelimit.h>
> -#include <linux/mutex.h>
> #include <linux/scatterlist.h>
> #include <crypto/internal/aead.h>
> -#include <crypto/internal/rng.h>
> #include <crypto/internal/skcipher.h>
> #include "crypto4xx_reg_def.h"
> #include "crypto4xx_sa.h"
>
> #define PPC460SX_SDR0_SRST 0x201
> @@ -109,11 +107,10 @@ struct crypto4xx_core_device {
> struct hwrng *trng;
> u32 int_status;
> u32 irq;
> struct tasklet_struct tasklet;
> spinlock_t lock;
> - struct mutex rng_lock;
> };
>
> struct crypto4xx_ctx {
> struct crypto4xx_device *dev;
> struct dynamic_sa_ctl *sa_in;
> @@ -133,11 +130,10 @@ struct crypto4xx_aead_reqctx {
> struct crypto4xx_alg_common {
> u32 type;
> union {
> struct skcipher_alg cipher;
> struct aead_alg aead;
> - struct rng_alg rng;
> } u;
> };
>
> struct crypto4xx_alg {
> struct list_head entry;
> diff --git a/drivers/crypto/amcc/crypto4xx_reg_def.h b/drivers/crypto/amcc/crypto4xx_reg_def.h
> index 1038061224da..73d626308a84 100644
> --- a/drivers/crypto/amcc/crypto4xx_reg_def.h
> +++ b/drivers/crypto/amcc/crypto4xx_reg_def.h
> @@ -88,24 +88,13 @@
>
> #define CRYPTO4XX_DMA_CFG 0x000600d4
> #define CRYPTO4XX_BYTE_ORDER_CFG 0x000600d8
> #define CRYPTO4XX_ENDIAN_CFG 0x000600d8
>
> -#define CRYPTO4XX_PRNG_STAT 0x00070000
> -#define CRYPTO4XX_PRNG_STAT_BUSY 0x1
> #define CRYPTO4XX_PRNG_CTRL 0x00070004
> #define CRYPTO4XX_PRNG_SEED_L 0x00070008
> #define CRYPTO4XX_PRNG_SEED_H 0x0007000c
> -
> -#define CRYPTO4XX_PRNG_RES_0 0x00070020
> -#define CRYPTO4XX_PRNG_RES_1 0x00070024
> -#define CRYPTO4XX_PRNG_RES_2 0x00070028
> -#define CRYPTO4XX_PRNG_RES_3 0x0007002C
> -
> -#define CRYPTO4XX_PRNG_LFSR_L 0x00070030
> -#define CRYPTO4XX_PRNG_LFSR_H 0x00070034
> -
Hmm, don't think these defines will hurt anyone? As these are part of the hardware spec.
Or do you forsee a future where AI-Agents will sent patches hallucinating that it "fixed"
the issue which readds it? I have no idea.
> /*
> * Initialize CRYPTO ENGINE registers, and memory bases.
> */
> #define PPC4XX_PDR_POLL 0x3ff
> #define PPC4XX_OUTPUT_THRESHOLD 2
Cheers,
Christian
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] crypto: crypto4xx - Remove insecure and unused rng_alg
2026-05-30 10:20 ` Christian Lamparter
@ 2026-05-30 19:12 ` Eric Biggers
0 siblings, 0 replies; 6+ messages in thread
From: Eric Biggers @ 2026-05-30 19:12 UTC (permalink / raw)
To: Christian Lamparter
Cc: linux-crypto, Herbert Xu, linuxppc-dev, linux-kernel, stable
On Sat, May 30, 2026 at 12:20:57PM +0200, Christian Lamparter wrote:
> > diff --git a/drivers/crypto/amcc/crypto4xx_reg_def.h b/drivers/crypto/amcc/crypto4xx_reg_def.h
> > index 1038061224da..73d626308a84 100644
> > --- a/drivers/crypto/amcc/crypto4xx_reg_def.h
> > +++ b/drivers/crypto/amcc/crypto4xx_reg_def.h
> > @@ -88,24 +88,13 @@
> > #define CRYPTO4XX_DMA_CFG 0x000600d4
> > #define CRYPTO4XX_BYTE_ORDER_CFG 0x000600d8
> > #define CRYPTO4XX_ENDIAN_CFG 0x000600d8
> > -#define CRYPTO4XX_PRNG_STAT 0x00070000
> > -#define CRYPTO4XX_PRNG_STAT_BUSY 0x1
> > #define CRYPTO4XX_PRNG_CTRL 0x00070004
> > #define CRYPTO4XX_PRNG_SEED_L 0x00070008
> > #define CRYPTO4XX_PRNG_SEED_H 0x0007000c
> > -
> > -#define CRYPTO4XX_PRNG_RES_0 0x00070020
> > -#define CRYPTO4XX_PRNG_RES_1 0x00070024
> > -#define CRYPTO4XX_PRNG_RES_2 0x00070028
> > -#define CRYPTO4XX_PRNG_RES_3 0x0007002C
> > -
> > -#define CRYPTO4XX_PRNG_LFSR_L 0x00070030
> > -#define CRYPTO4XX_PRNG_LFSR_H 0x00070034
> > -
>
> Hmm, don't think these defines will hurt anyone? As these are part of the hardware spec.
> Or do you forsee a future where AI-Agents will sent patches hallucinating that it "fixed"
> the issue which readds it? I have no idea.
Well, there's not really any point in keeping these when they aren't
used.
- Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: crypto4xx - Remove insecure and unused rng_alg
2026-05-29 22:04 [PATCH] crypto: crypto4xx - Remove insecure and unused rng_alg Eric Biggers
2026-05-30 10:20 ` Christian Lamparter
@ 2026-05-30 15:05 ` Aleksander Jan Bajkowski
2026-05-30 19:26 ` Eric Biggers
1 sibling, 1 reply; 6+ messages in thread
From: Aleksander Jan Bajkowski @ 2026-05-30 15:05 UTC (permalink / raw)
To: Eric Biggers, linux-crypto, Herbert Xu
Cc: Christian Lamparter, linuxppc-dev, linux-kernel, stable
Hi Eric,
On 30/05/2026 00:04, Eric Biggers wrote:
> Remove crypto4xx_rng, as it is insecure and unused:
>
> - It has only a 64-bit security strength, which is highly inadequate.
> This can be seen by the fact that crypto4xx_hw_init() seeds it with
> only 64 bits of entropy, and the fact that the original commit
> mentions that it implements ANSI X9.17 Annex C.
In addition to a seed, the PRNG also uses ring oscillators as sources of
entropy. The entropy should be higher than 64b. This is the Rambus EIP-73d
IP core. The same IP core is built into eip93 (EIP-73a), eip97 (EIP-73d),
and eip197 (EIP-73d). You can find the documentation online. The complete
"container" is actually Rambus EIP-94, and one of its parts is EIP-73d.
>
> Another issue was that this driver didn't implement the crypto_rng API
> correctly, as crypto4xx_prng_generate() didn't return 0 on success.
>
> - No user of this code is known. It's usable only theoretically via the
> "rng" algorithm type of AF_ALG. But userspace actually just uses the
> actual Linux RNG (/dev/random etc) instead. And rng_algs don't
> contribute entropy to the actual Linux RNG either. (This may have
> been confused with hwrng, which does contribute entropy.)
This PRNG is also used internally for Generation IV with IPSEC offload. The
IPSEC offload implementation for eip93 was recently submitted to upstream.
I am not sure whether eip94 shares some of the logic for IPSEC offload and
it will be possible to use some of the code.
>
> Fixes: d072bfa48853 ("crypto: crypto4xx - add prng crypto support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> Acked-by: Christian Lamparter <chunkeey@gmail.com>
> ---
> drivers/crypto/Kconfig | 1 -
> drivers/crypto/amcc/crypto4xx_core.c | 88 -------------------------
> drivers/crypto/amcc/crypto4xx_core.h | 4 --
> drivers/crypto/amcc/crypto4xx_reg_def.h | 11 ----
> 4 files changed, 104 deletions(-)
>
>
> base-commit: 49e05bb00f2e8168695f7af4d694c39e1423e8a2
>
> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index 3449b3c9c6ad..5dab813a9f74 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -299,11 +299,10 @@ config CRYPTO_DEV_PPC4XX
> select CRYPTO_AES
> select CRYPTO_LIB_AES
> select CRYPTO_CCM
> select CRYPTO_CTR
> select CRYPTO_GCM
> - select CRYPTO_RNG
> select CRYPTO_SKCIPHER
> help
> This option allows you to have support for AMCC crypto acceleration.
>
> config HW_RANDOM_PPC4XX
> diff --git a/drivers/crypto/amcc/crypto4xx_core.c b/drivers/crypto/amcc/crypto4xx_core.c
> index b7b6c97d2147..68c5ff7a85b4 100644
> --- a/drivers/crypto/amcc/crypto4xx_core.c
> +++ b/drivers/crypto/amcc/crypto4xx_core.c
> @@ -29,15 +29,13 @@
> #include <crypto/aead.h>
> #include <crypto/aes.h>
> #include <crypto/ctr.h>
> #include <crypto/gcm.h>
> #include <crypto/sha1.h>
> -#include <crypto/rng.h>
> #include <crypto/scatterwalk.h>
> #include <crypto/skcipher.h>
> #include <crypto/internal/aead.h>
> -#include <crypto/internal/rng.h>
> #include <crypto/internal/skcipher.h>
> #include "crypto4xx_reg_def.h"
> #include "crypto4xx_core.h"
> #include "crypto4xx_sa.h"
> #include "crypto4xx_trng.h"
> @@ -983,14 +981,10 @@ static int crypto4xx_register_alg(struct crypto4xx_device *sec_dev,
> switch (alg->alg.type) {
> case CRYPTO_ALG_TYPE_AEAD:
> rc = crypto_register_aead(&alg->alg.u.aead);
> break;
>
> - case CRYPTO_ALG_TYPE_RNG:
> - rc = crypto_register_rng(&alg->alg.u.rng);
> - break;
> -
> default:
> rc = crypto_register_skcipher(&alg->alg.u.cipher);
> break;
> }
>
> @@ -1012,14 +1006,10 @@ static void crypto4xx_unregister_alg(struct crypto4xx_device *sec_dev)
> switch (alg->alg.type) {
> case CRYPTO_ALG_TYPE_AEAD:
> crypto_unregister_aead(&alg->alg.u.aead);
> break;
>
> - case CRYPTO_ALG_TYPE_RNG:
> - crypto_unregister_rng(&alg->alg.u.rng);
> - break;
> -
> default:
> crypto_unregister_skcipher(&alg->alg.u.cipher);
> }
> kfree(alg);
> }
> @@ -1074,73 +1064,10 @@ static irqreturn_t crypto4xx_ce_interrupt_handler_revb(int irq, void *data)
> {
> return crypto4xx_interrupt_handler(irq, data, PPC4XX_INTERRUPT_CLR |
> PPC4XX_TMO_ERR_INT);
> }
>
> -static int ppc4xx_prng_data_read(struct crypto4xx_device *dev,
> - u8 *data, unsigned int max)
> -{
> - unsigned int i, curr = 0;
> - u32 val[2];
> -
> - do {
> - /* trigger PRN generation */
> - writel(PPC4XX_PRNG_CTRL_AUTO_EN,
> - dev->ce_base + CRYPTO4XX_PRNG_CTRL);
> -
> - for (i = 0; i < 1024; i++) {
> - /* usually 19 iterations are enough */
> - if ((readl(dev->ce_base + CRYPTO4XX_PRNG_STAT) &
> - CRYPTO4XX_PRNG_STAT_BUSY))
> - continue;
> -
> - val[0] = readl_be(dev->ce_base + CRYPTO4XX_PRNG_RES_0);
> - val[1] = readl_be(dev->ce_base + CRYPTO4XX_PRNG_RES_1);
> - break;
> - }
> - if (i == 1024)
> - return -ETIMEDOUT;
> -
> - if ((max - curr) >= 8) {
> - memcpy(data, &val, 8);
> - data += 8;
> - curr += 8;
> - } else {
> - /* copy only remaining bytes */
> - memcpy(data, &val, max - curr);
> - break;
> - }
> - } while (curr < max);
> -
> - return curr;
> -}
> -
> -static int crypto4xx_prng_generate(struct crypto_rng *tfm,
> - const u8 *src, unsigned int slen,
> - u8 *dstn, unsigned int dlen)
> -{
> - struct rng_alg *alg = crypto_rng_alg(tfm);
> - struct crypto4xx_alg *amcc_alg;
> - struct crypto4xx_device *dev;
> - int ret;
> -
> - amcc_alg = container_of(alg, struct crypto4xx_alg, alg.u.rng);
> - dev = amcc_alg->dev;
> -
> - mutex_lock(&dev->core_dev->rng_lock);
> - ret = ppc4xx_prng_data_read(dev, dstn, dlen);
> - mutex_unlock(&dev->core_dev->rng_lock);
> - return ret;
> -}
> -
> -
> -static int crypto4xx_prng_seed(struct crypto_rng *tfm, const u8 *seed,
> - unsigned int slen)
> -{
> - return 0;
> -}
> -
> /*
> * Supported Crypto Algorithms
> */
> static struct crypto4xx_alg_common crypto4xx_alg[] = {
> /* Crypto AES modes */
> @@ -1266,22 +1193,10 @@ static struct crypto4xx_alg_common crypto4xx_alg[] = {
> .cra_blocksize = 1,
> .cra_ctxsize = sizeof(struct crypto4xx_ctx),
> .cra_module = THIS_MODULE,
> },
> } },
> - { .type = CRYPTO_ALG_TYPE_RNG, .u.rng = {
> - .base = {
> - .cra_name = "stdrng",
> - .cra_driver_name = "crypto4xx_rng",
> - .cra_priority = 300,
> - .cra_ctxsize = 0,
> - .cra_module = THIS_MODULE,
> - },
> - .generate = crypto4xx_prng_generate,
> - .seed = crypto4xx_prng_seed,
> - .seedsize = 0,
> - } },
> };
>
> /*
> * Module Initialization Routine
> */
> @@ -1351,13 +1266,10 @@ static int crypto4xx_probe(struct platform_device *ofdev)
> }
>
> core_dev->dev->core_dev = core_dev;
> core_dev->dev->is_revb = is_revb;
> core_dev->device = dev;
> - rc = devm_mutex_init(&ofdev->dev, &core_dev->rng_lock);
> - if (rc)
> - return rc;
> spin_lock_init(&core_dev->lock);
> INIT_LIST_HEAD(&core_dev->dev->alg_list);
> ratelimit_default_init(&core_dev->dev->aead_ratelimit);
> rc = crypto4xx_build_sdr(core_dev->dev);
> if (rc)
> diff --git a/drivers/crypto/amcc/crypto4xx_core.h b/drivers/crypto/amcc/crypto4xx_core.h
> index ee36630c670f..3a028aec3f0c 100644
> --- a/drivers/crypto/amcc/crypto4xx_core.h
> +++ b/drivers/crypto/amcc/crypto4xx_core.h
> @@ -12,14 +12,12 @@
>
> #ifndef __CRYPTO4XX_CORE_H__
> #define __CRYPTO4XX_CORE_H__
>
> #include <linux/ratelimit.h>
> -#include <linux/mutex.h>
> #include <linux/scatterlist.h>
> #include <crypto/internal/aead.h>
> -#include <crypto/internal/rng.h>
> #include <crypto/internal/skcipher.h>
> #include "crypto4xx_reg_def.h"
> #include "crypto4xx_sa.h"
>
> #define PPC460SX_SDR0_SRST 0x201
> @@ -109,11 +107,10 @@ struct crypto4xx_core_device {
> struct hwrng *trng;
> u32 int_status;
> u32 irq;
> struct tasklet_struct tasklet;
> spinlock_t lock;
> - struct mutex rng_lock;
> };
>
> struct crypto4xx_ctx {
> struct crypto4xx_device *dev;
> struct dynamic_sa_ctl *sa_in;
> @@ -133,11 +130,10 @@ struct crypto4xx_aead_reqctx {
> struct crypto4xx_alg_common {
> u32 type;
> union {
> struct skcipher_alg cipher;
> struct aead_alg aead;
> - struct rng_alg rng;
> } u;
> };
>
> struct crypto4xx_alg {
> struct list_head entry;
> diff --git a/drivers/crypto/amcc/crypto4xx_reg_def.h b/drivers/crypto/amcc/crypto4xx_reg_def.h
> index 1038061224da..73d626308a84 100644
> --- a/drivers/crypto/amcc/crypto4xx_reg_def.h
> +++ b/drivers/crypto/amcc/crypto4xx_reg_def.h
> @@ -88,24 +88,13 @@
>
> #define CRYPTO4XX_DMA_CFG 0x000600d4
> #define CRYPTO4XX_BYTE_ORDER_CFG 0x000600d8
> #define CRYPTO4XX_ENDIAN_CFG 0x000600d8
>
> -#define CRYPTO4XX_PRNG_STAT 0x00070000
> -#define CRYPTO4XX_PRNG_STAT_BUSY 0x1
> #define CRYPTO4XX_PRNG_CTRL 0x00070004
> #define CRYPTO4XX_PRNG_SEED_L 0x00070008
> #define CRYPTO4XX_PRNG_SEED_H 0x0007000c
> -
> -#define CRYPTO4XX_PRNG_RES_0 0x00070020
> -#define CRYPTO4XX_PRNG_RES_1 0x00070024
> -#define CRYPTO4XX_PRNG_RES_2 0x00070028
> -#define CRYPTO4XX_PRNG_RES_3 0x0007002C
> -
> -#define CRYPTO4XX_PRNG_LFSR_L 0x00070030
> -#define CRYPTO4XX_PRNG_LFSR_H 0x00070034
> -
> /*
> * Initialize CRYPTO ENGINE registers, and memory bases.
> */
> #define PPC4XX_PDR_POLL 0x3ff
> #define PPC4XX_OUTPUT_THRESHOLD 2
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] crypto: crypto4xx - Remove insecure and unused rng_alg
2026-05-30 15:05 ` Aleksander Jan Bajkowski
@ 2026-05-30 19:26 ` Eric Biggers
2026-05-31 10:15 ` Aleksander Jan Bajkowski
0 siblings, 1 reply; 6+ messages in thread
From: Eric Biggers @ 2026-05-30 19:26 UTC (permalink / raw)
To: Aleksander Jan Bajkowski
Cc: linux-crypto, Herbert Xu, Christian Lamparter, linuxppc-dev,
linux-kernel, stable
On Sat, May 30, 2026 at 05:05:19PM +0200, Aleksander Jan Bajkowski wrote:
> Hi Eric,
>
> On 30/05/2026 00:04, Eric Biggers wrote:
> > Remove crypto4xx_rng, as it is insecure and unused:
> >
> > - It has only a 64-bit security strength, which is highly inadequate.
> > This can be seen by the fact that crypto4xx_hw_init() seeds it with
> > only 64 bits of entropy, and the fact that the original commit
> > mentions that it implements ANSI X9.17 Annex C.
>
> In addition to a seed, the PRNG also uses ring oscillators as sources of
> entropy. The entropy should be higher than 64b. This is the Rambus EIP-73d
> IP core. The same IP core is built into eip93 (EIP-73a), eip97 (EIP-73d),
> and eip197 (EIP-73d). You can find the documentation online. The complete
> "container" is actually Rambus EIP-94, and one of its parts is EIP-73d.
Just because it may have another source of entropy doesn't mean its
security strength is higher than 64 bits.
I cannot find any documentation other than
https://datasheet.octopart.com/PPC460EX-SUB800T-AMCC-datasheet-11553412.pdf
which says "ANSI X9.17 Annex C compliant using a DES algorithm".
DES actually has a 56-bit key, so maybe I was over-generous.
And according to https://cacr.uwaterloo.ca/hac/about/chap5.pdf ANSI
X9.17 has only a 64-bit state anyway. So even if we assume the
datasheet is incorrect and the algorithm is actually 3DES which has a
longer key, the state is likely still 64-bit.
So it isn't looking good. And since it's an undocumented proprietary
design it shouldn't be given the benefit of the doubt either.
> This PRNG is also used internally for Generation IV with IPSEC offload. The
> IPSEC offload implementation for eip93 was recently submitted to upstream.
> I am not sure whether eip94 shares some of the logic for IPSEC offload and
> it will be possible to use some of the code.
That's not related to this patch.
- Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: crypto4xx - Remove insecure and unused rng_alg
2026-05-30 19:26 ` Eric Biggers
@ 2026-05-31 10:15 ` Aleksander Jan Bajkowski
0 siblings, 0 replies; 6+ messages in thread
From: Aleksander Jan Bajkowski @ 2026-05-31 10:15 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-crypto, Herbert Xu, Christian Lamparter, linuxppc-dev,
linux-kernel, stable
Hi Eric,
On 30/05/2026 21:26, Eric Biggers wrote:
> On Sat, May 30, 2026 at 05:05:19PM +0200, Aleksander Jan Bajkowski wrote:
>> Hi Eric,
>>
>> On 30/05/2026 00:04, Eric Biggers wrote:
>>> Remove crypto4xx_rng, as it is insecure and unused:
>>>
>>> - It has only a 64-bit security strength, which is highly inadequate.
>>> This can be seen by the fact that crypto4xx_hw_init() seeds it with
>>> only 64 bits of entropy, and the fact that the original commit
>>> mentions that it implements ANSI X9.17 Annex C.
>> In addition to a seed, the PRNG also uses ring oscillators as sources of
>> entropy. The entropy should be higher than 64b. This is the Rambus EIP-73d
>> IP core. The same IP core is built into eip93 (EIP-73a), eip97 (EIP-73d),
>> and eip197 (EIP-73d). You can find the documentation online. The complete
>> "container" is actually Rambus EIP-94, and one of its parts is EIP-73d.
> Just because it may have another source of entropy doesn't mean its
> security strength is higher than 64 bits.
>
> I cannot find any documentation other than
> https://datasheet.octopart.com/PPC460EX-SUB800T-AMCC-datasheet-11553412.pdf
> which says "ANSI X9.17 Annex C compliant using a DES algorithm".
>
> DES actually has a 56-bit key, so maybe I was over-generous.
>
> And according to https://cacr.uwaterloo.ca/hac/about/chap5.pdf ANSI
> X9.17 has only a 64-bit state anyway. So even if we assume the
> datasheet is incorrect and the algorithm is actually 3DES which has a
> longer key, the state is likely still 64-bit.
According to the datasheet, there is no second source of entropy. The PRNG
has three built-in LFSRs. Each of them can be initialized independently. The
first LFSR is used to generate input data. The second and third are used to
generate keys for DES encryption. The output of the first LFSR is encrypted
using 3DES with two 64-bit keys. Between individual DES operations, data is
XORed with the seed. It sounds like a fairly secure design if properly
reseeded.
There is also a newer design (EIP73a) based on the same algorithm. The
only difference is the replacing of 3DES with AES using a 2TDEA scheme.
The DES-based variant is more widely used, even in new SoCs.
>
> So it isn't looking good. And since it's an undocumented proprietary
> design it shouldn't be given the benefit of the doubt either.
>
As I mentioned earlier, this IP core is quite well documented[1] (page 198).
Half of all SOHO routers have the EIP-73d built in. The algorithm is also
described in TRM for some of these SoCs :)
List od SoCs with EIP-73d:
AMCC PPC405EX/PPC460EX,
Intel/Maxlinear GRX350, URX850,
Marvell Armada 37x0, 7k, 8k,
Mediatek MT7623/MT7981/MT7986/MT7987/MT7988,
Qualcomm IPQ975x.
[1]
https://www.scribd.com/document/734250956/Safexcel-Ip-94-Plb-Sas-v1-5?_gl=1*dng4pf*_up*MQ..*_ga*OTQ4NjkzMTAxLjE3ODAyMjA4ODI.*_ga_Z4ZC50DED6*czE3ODAyMjA4ODEkbzEkZzEkdDE3ODAyMjA4ODEkajYwJGwwJGgw*_ga_8KZ8BV0P5W*czE3ODAyMjA4ODEkbzEkZzEkdDE3ODAyMjA4ODEkajYwJGwwJGgw
Best regards,
Aleksander
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-05-31 10:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-29 22:04 [PATCH] crypto: crypto4xx - Remove insecure and unused rng_alg Eric Biggers
2026-05-30 10:20 ` Christian Lamparter
2026-05-30 19:12 ` Eric Biggers
2026-05-30 15:05 ` Aleksander Jan Bajkowski
2026-05-30 19:26 ` Eric Biggers
2026-05-31 10:15 ` Aleksander Jan Bajkowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox