[PATCH v3 0/3] crypto: atmel-sha204a - multiple RNG fixes

[Lothar Rubusch]

Hi Ard and ML!

On Thu, Apr 23, 2026 at 11:25 AM Ard Biesheuvel <ardb at kernel.org> wrote:
>
> Hi Lothar,
>
> On Wed, 22 Apr 2026, at 23:09, Lothar Rubusch wrote:
> > When testing the RNG functionality on the Atmel SHA204a hardware, I
> > found the following issues: rngtest reported failures and hexdump
> > reveiled only the first 8 bytes out of 32 provided actually entropy.
> >
> > Having a closer look into it, I found a (small) memory leak, missing
> > to free work_data, miss-reading of the count field into the entropy
> > fields and parts of the 32 random bytes staying 0 due to reading the
> > slow i2c device.
> >
> > The series proposes fixes and how fixed functionality can be/was
> > verified. Executing rngtest afterward showed a decent result, due
> > to the i2c bus a bit slow.
> >
> > All setups require selecting the Atmel-sha204a as active RNG.
> > $ cat /sys/class/misc/hw_random/rng_available
> >     3f104000.rng 1-0064 none
> >
> > $ echo 1-0064 > /sys/class/misc/hw_random/rng_current
> >
> > $ cat /sys/class/misc/hw_random/rng_current
> >     1-0064
> >
> > Testing RNG properties currently shows problematic results:
> > $ rngtest < /dev/hwrng
> >     rngtest 2.6
> >     Copyright (c) 2004 by Henrique de Moraes Holschuh
> >     This is free software; see the source for copying conditions.  There is NO
> >     warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> >
> >     rngtest: starting FIPS tests...
> >     rngtest: bits received from input: 1040032
> >     rngtest: FIPS 140-2 successes: 0
> >     rngtest: FIPS 140-2 failures: 52
> >     rngtest: FIPS 140-2(2001-10-10) Monobit: 52
> >     rngtest: FIPS 140-2(2001-10-10) Poker: 52
> >     rngtest: FIPS 140-2(2001-10-10) Runs: 52
> >     rngtest: FIPS 140-2(2001-10-10) Long run: 52
> >     rngtest: FIPS 140-2(2001-10-10) Continuous run: 52
> >     rngtest: input channel speed: (min=7.631; avg=7.804; max=7.827)Kibits/s
> >     rngtest: FIPS tests speed: (min=32.273; avg=32.701; max=33.056)Mibits/s
> >     rngtest: Program run time: 130177956 microseconds
> >
> > Signed-off-by: Lothar Rubusch <l.rubusch at gmail.com>
> > ---
> > v2 -> v3: Removal blank line, rebased
> > v1 -> v2: Removal of C++ style comment (I saw it too late, sry for that)
> > ---
> > Lothar Rubusch (3):
> >   crypto: atmel-sha204a - fix memory leak at non-blocking RNG work_data
> >   crypto: atmel-sha204a - fix truncated 32-byte blocking read
> >   crypto: atmel-sha204a - fix non-blocking read logic
> >
> >  drivers/crypto/atmel-sha204a.c | 60 ++++++++++++++++++++++------------
> >  1 file changed, 39 insertions(+), 21 deletions(-)
> >
>
> Thanks for the report and the fixes. However, I'm not sure you are entirely
> on the right track here. I managed to fix the rngtest issues that you report by
> making the changes below. As I already replied, I think it would be better to
> propose this as a standalone patch, and backport it to stable.
>

Thank you so much for taking the time and answering, I really appreciate!

Like two months ago I started playing a bit more with these Atmel i2c
devices. Actually I wanted to prepare something
different (and still pretend doing so). Figuring out things here, I
noticed this RNG feature did not work as I expected.
Around that time I prepared this patch set.
Probably, I was a bit too euphoric - I mean, it's a bit unlikely the
driver was upstreamed and never really worked.
I have to admit, I did not go thoroughly through the git history
first, since the driver does not do significantly more than
RNG, there must have happened a degradation. I'll keep this in mind:
first to build up better understanding of the driver
history. than to start digging into the code. Anyway, I wanted to get
some feedback.

> The remaining changes are somewhat debatable IMO: the leak is not really a leak,
> so I'd like to understand better what you are fixing here. The command field
> changes seems completely misguided (unless I am missing something)
>

So, firstly in the current RNG implementation I definitely see an
issue. If that's ok, I'll to prepare a patch to re-init probably
rather
to what you proposed, since it makes more sense to me now. Thank you
for pointing this out.

Secondly, I noticed the device is picky about its i2c communication
timings. Yes, definitely. Nowadays, after having worked with
it a bit more, I'd rather say generally reducing response time is
probably not the solution. I still think I see some timing issue,
which
I wanted to better address in an upcoming patch set (but probably not
related to this RNG thing). Currently the AtSHA204a and
the AtECC508a (and related families) are sharing the same Atmel i2c
driver. This core driver uses hardcoded max timings for
its i2c command operations. Going over the datasheets of both chips,
both have totally different max timings.
Currently just having RNG for AtSHA204a and ECDH and related for AtECC
- it probably doesn't matter too much. Anyway, also
read and write command timings differ and mix up then in the Atmel i2c
core driver. I guess, this was kind of a starting point for
me to dig deeper. So, I probably will drop that i2c timing change from
this RNG fix set.

Thirdly, the "leak" I think is probably rather a theoretic issue. I'll
sepearate it out, think over it, and in case prepare something,
let me then know what you think.

>
>
> --- a/drivers/crypto/atmel-sha204a.c
> +++ b/drivers/crypto/atmel-sha204a.c
> @@ -47,8 +47,8 @@
>
>         if (rng->priv) {
>                 work_data = (struct atmel_i2c_work_data *)rng->priv;
> -               max = min(sizeof(work_data->cmd.data), max);
> -               memcpy(data, &work_data->cmd.data, max);
> +               max = min(RANDOM_RSP_SIZE - CMD_OVERHEAD_SIZE, max);
> +               memcpy(data, &work_data->cmd.data[1], max);
>                 rng->priv = 0;
>         } else {
>                 work_data = kmalloc_obj(*work_data, GFP_ATOMIC);
> @@ -86,8 +86,8 @@
>         if (ret)
>                 return ret;
>
> -       max = min(sizeof(cmd.data), max);
> -       memcpy(data, cmd.data, max);
> +       max = min(RANDOM_RSP_SIZE - CMD_OVERHEAD_SIZE, max);
> +       memcpy(data, &cmd.data[1], max);
>
>         return max;
>  }