[PATCH v2] crypto: atmel-sha204a - drop hwrng quality reduction for ATSHA204A

[PATCH v2] crypto: atmel-sha204a - drop hwrng quality reduction for ATSHA204A

[Thorsten Blum]

Commit 8006aff15516 ("crypto: atmel-sha204a - Set hwrng quality to
lowest possible") reduced the hwrng quality to 1 based on a review by
Bill Cox [1]. However, despite its title, the review only tested the
ATSHA204, not the ATSHA204A.

In the same thread, Atmel engineer Landon Cox wrote "this behavior has
been eliminated entirely"[2] in the ATSHA204A and "this problem does not
affect the ATECC108 or the ATECC108A (or the ATSHA204A)"[3].

According to the official ATSHA204A datasheet [4], the device contains a
high-quality hardware RNG that combines its output with an internal seed
value stored in EEPROM or SRAM to generate random numbers. The device
also implements all security functions using SHA-256, and the driver
uses the chip's Random command in seed-update mode.

Keep 'quality = 1' for ATSHA204, but drop the explicit hwrng quality
reduction for ATSHA204A and fall back to the hwrng core default.

[1] https://www.metzdowd.com/pipermail/cryptography/2014-December/023858.html
[2] https://www.metzdowd.com/pipermail/cryptography/2014-December/023852.html
[3] https://www.metzdowd.com/pipermail/cryptography/2014-December/023886.html
[4] https://ww1.microchip.com/downloads/en/DeviceDoc/ATSHA204A-Data-Sheet-40002025A.pdf

Fixes: 8006aff15516 ("crypto: atmel-sha204a - Set hwrng quality to lowest possible")
Cc: stable@vger.kernel.org
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
Changes in v2:
- Drop the enum and dereference match data if needed as suggested by Ard
- Keep the review comment
- v1: https://lore.kernel.org/lkml/20260427124030.315590-3-thorsten.blum@linux.dev/
---
 drivers/crypto/atmel-sha204a.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index dbb39ed0cea1..a8c1b00b12f5 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -19,6 +19,12 @@
 #include <linux/workqueue.h>
 #include "atmel-i2c.h"
 
+/*
+ * According to review by Bill Cox [1], the ATSHA204 has very low entropy.
+ * [1] https://www.metzdowd.com/pipermail/cryptography/2014-December/023858.html
+ */
+static const unsigned short atsha204_quality = 1;
+
 static void atmel_sha204a_rng_done(struct atmel_i2c_work_data *work_data,
 				   void *areq, int status)
 {
@@ -158,6 +164,7 @@ static const struct attribute_group atmel_sha204a_groups = {
 static int atmel_sha204a_probe(struct i2c_client *client)
 {
 	struct atmel_i2c_client_priv *i2c_priv;
+	const unsigned short *quality;
 	int ret;
 
 	ret = atmel_i2c_probe(client);
@@ -171,11 +178,9 @@ static int atmel_sha204a_probe(struct i2c_client *client)
 	i2c_priv->hwrng.name = dev_name(&client->dev);
 	i2c_priv->hwrng.read = atmel_sha204a_rng_read;
 
-	/*
-	 * According to review by Bill Cox [1], this HWRNG has very low entropy.
-	 * [1] https://www.metzdowd.com/pipermail/cryptography/2014-December/023858.html
-	 */
-	i2c_priv->hwrng.quality = 1;
+	quality = i2c_get_match_data(client);
+	if (quality)
+		i2c_priv->hwrng.quality = *quality;
 
 	ret = devm_hwrng_register(&client->dev, &i2c_priv->hwrng);
 	if (ret)
@@ -203,14 +208,14 @@ static void atmel_sha204a_remove(struct i2c_client *client)
 }
 
 static const struct of_device_id atmel_sha204a_dt_ids[] __maybe_unused = {
-	{ .compatible = "atmel,atsha204", },
+	{ .compatible = "atmel,atsha204", .data = &atsha204_quality },
 	{ .compatible = "atmel,atsha204a", },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, atmel_sha204a_dt_ids);
 
 static const struct i2c_device_id atmel_sha204a_id[] = {
-	{ "atsha204" },
+	{ "atsha204", (kernel_ulong_t)&atsha204_quality },
 	{ "atsha204a" },
 	{ /* sentinel */ }
 };

Re: [PATCH v2] crypto: atmel-sha204a - drop hwrng quality reduction for ATSHA204A

[Ard Biesheuvel]

On Tue, 28 Apr 2026, at 12:14, Thorsten Blum wrote:
> Commit 8006aff15516 ("crypto: atmel-sha204a - Set hwrng quality to
> lowest possible") reduced the hwrng quality to 1 based on a review by
> Bill Cox [1]. However, despite its title, the review only tested the
> ATSHA204, not the ATSHA204A.
>
> In the same thread, Atmel engineer Landon Cox wrote "this behavior has
> been eliminated entirely"[2] in the ATSHA204A and "this problem does not
> affect the ATECC108 or the ATECC108A (or the ATSHA204A)"[3].
>
> According to the official ATSHA204A datasheet [4], the device contains a
> high-quality hardware RNG that combines its output with an internal seed
> value stored in EEPROM or SRAM to generate random numbers. The device
> also implements all security functions using SHA-256, and the driver
> uses the chip's Random command in seed-update mode.
>
> Keep 'quality = 1' for ATSHA204, but drop the explicit hwrng quality
> reduction for ATSHA204A and fall back to the hwrng core default.
>
> [1] 
> https://www.metzdowd.com/pipermail/cryptography/2014-December/023858.html
> [2] 
> https://www.metzdowd.com/pipermail/cryptography/2014-December/023852.html
> [3] 
> https://www.metzdowd.com/pipermail/cryptography/2014-December/023886.html
> [4] 
> https://ww1.microchip.com/downloads/en/DeviceDoc/ATSHA204A-Data-Sheet-40002025A.pdf
>
> Fixes: 8006aff15516 ("crypto: atmel-sha204a - Set hwrng quality to 
> lowest possible")
> Cc: stable@vger.kernel.org
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

Re: [PATCH v2] crypto: atmel-sha204a - drop hwrng quality reduction for ATSHA204A

[Marek Behún]

Adding Bill Cox (waywardgeek) to the conversation.

In the meantime Nack from me on this patch.

From the original messages by Bill, it seems to me the part he was reviewing
was the ATSHA204A.

In subsequent reply [1] Bill states

  While there is some evidence, there is still no convincing proof that there
  is an entropy source in this device at all.  There is some evidence that
  Atmel has inserted a back-door.  My advice is to avoid this line of parts
  from Atmel for cryptographic use.

In another message Peter Gutmann asks about ATECC108 [2] and Bill replies [3]

  This part uses the same language to describe the random number generator.
  It is "high quality".  I think that's pretty funny.
  I would be interested in seeing if the new part can generate random numbers
  continuously, or if it fails after it's EEPROM wears out like their other
  parts.  The use of an EEPROM seed is for PWN-ing your RNG, not making it
  more secure.

IMO the comments from the actual reviewer are more relevant than those of the
engineer working for the company which was accused of creating low quality
/ backdoored TRNG, at least until the Atmel engineer provides some evaluation
code for the device (which they suggested they might do [4], but never did as
far as I can find).

Maybe we can instead change the ATECC quality to something like 32? Does that
even make sense?

Marek

[1] https://www.metzdowd.com/pipermail/cryptography/2014-December/023857.html
[2] https://www.metzdowd.com/pipermail/cryptography/2014-December/023870.html
[3] https://www.metzdowd.com/pipermail/cryptography/2014-December/023879.html
[4] https://www.metzdowd.com/pipermail/cryptography/2014-December/023886.html

On Tue, Apr 28, 2026 at 12:14:32PM +0200, Thorsten Blum wrote:
> Commit 8006aff15516 ("crypto: atmel-sha204a - Set hwrng quality to
> lowest possible") reduced the hwrng quality to 1 based on a review by
> Bill Cox [1]. However, despite its title, the review only tested the
> ATSHA204, not the ATSHA204A.
> 
> In the same thread, Atmel engineer Landon Cox wrote "this behavior has
> been eliminated entirely"[2] in the ATSHA204A and "this problem does not
> affect the ATECC108 or the ATECC108A (or the ATSHA204A)"[3].
> 
> According to the official ATSHA204A datasheet [4], the device contains a
> high-quality hardware RNG that combines its output with an internal seed
> value stored in EEPROM or SRAM to generate random numbers. The device
> also implements all security functions using SHA-256, and the driver
> uses the chip's Random command in seed-update mode.
> 
> Keep 'quality = 1' for ATSHA204, but drop the explicit hwrng quality
> reduction for ATSHA204A and fall back to the hwrng core default.
> 
> [1] https://www.metzdowd.com/pipermail/cryptography/2014-December/023858.html
> [2] https://www.metzdowd.com/pipermail/cryptography/2014-December/023852.html
> [3] https://www.metzdowd.com/pipermail/cryptography/2014-December/023886.html
> [4] https://ww1.microchip.com/downloads/en/DeviceDoc/ATSHA204A-Data-Sheet-40002025A.pdf
> 
> Fixes: 8006aff15516 ("crypto: atmel-sha204a - Set hwrng quality to lowest possible")
> Cc: stable@vger.kernel.org
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> Changes in v2:
> - Drop the enum and dereference match data if needed as suggested by Ard
> - Keep the review comment
> - v1: https://lore.kernel.org/lkml/20260427124030.315590-3-thorsten.blum@linux.dev/
> ---
>  drivers/crypto/atmel-sha204a.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
> index dbb39ed0cea1..a8c1b00b12f5 100644
> --- a/drivers/crypto/atmel-sha204a.c
> +++ b/drivers/crypto/atmel-sha204a.c
> @@ -19,6 +19,12 @@
>  #include <linux/workqueue.h>
>  #include "atmel-i2c.h"
>  
> +/*
> + * According to review by Bill Cox [1], the ATSHA204 has very low entropy.
> + * [1] https://www.metzdowd.com/pipermail/cryptography/2014-December/023858.html
> + */
> +static const unsigned short atsha204_quality = 1;
> +
>  static void atmel_sha204a_rng_done(struct atmel_i2c_work_data *work_data,
>  				   void *areq, int status)
>  {
> @@ -158,6 +164,7 @@ static const struct attribute_group atmel_sha204a_groups = {
>  static int atmel_sha204a_probe(struct i2c_client *client)
>  {
>  	struct atmel_i2c_client_priv *i2c_priv;
> +	const unsigned short *quality;
>  	int ret;
>  
>  	ret = atmel_i2c_probe(client);
> @@ -171,11 +178,9 @@ static int atmel_sha204a_probe(struct i2c_client *client)
>  	i2c_priv->hwrng.name = dev_name(&client->dev);
>  	i2c_priv->hwrng.read = atmel_sha204a_rng_read;
>  
> -	/*
> -	 * According to review by Bill Cox [1], this HWRNG has very low entropy.
> -	 * [1] https://www.metzdowd.com/pipermail/cryptography/2014-December/023858.html
> -	 */
> -	i2c_priv->hwrng.quality = 1;
> +	quality = i2c_get_match_data(client);
> +	if (quality)
> +		i2c_priv->hwrng.quality = *quality;
>  
>  	ret = devm_hwrng_register(&client->dev, &i2c_priv->hwrng);
>  	if (ret)
> @@ -203,14 +208,14 @@ static void atmel_sha204a_remove(struct i2c_client *client)
>  }
>  
>  static const struct of_device_id atmel_sha204a_dt_ids[] __maybe_unused = {
> -	{ .compatible = "atmel,atsha204", },
> +	{ .compatible = "atmel,atsha204", .data = &atsha204_quality },
>  	{ .compatible = "atmel,atsha204a", },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, atmel_sha204a_dt_ids);
>  
>  static const struct i2c_device_id atmel_sha204a_id[] = {
> -	{ "atsha204" },
> +	{ "atsha204", (kernel_ulong_t)&atsha204_quality },
>  	{ "atsha204a" },
>  	{ /* sentinel */ }
>  };
>