[PATCH RESEND v2 1/1] crypto: atmel-sha204a - fix heap info leak on I2C transfer failure

[Lothar Rubusch]

On Thu, Jun 11, 2026 at 6:59 AM Herbert Xu <herbert at gondor.apana.org.au> wrote:
>
> On Tue, Jun 09, 2026 at 09:47:23AM +0000, Lothar Rubusch wrote:
> >
> > diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
> > index 4c9af737b33a..20cd915ea8a3 100644
> > --- a/drivers/crypto/atmel-sha204a.c
> > +++ b/drivers/crypto/atmel-sha204a.c
> > @@ -31,10 +31,15 @@ static void atmel_sha204a_rng_done(struct atmel_i2c_work_data *work_data,
> >       struct atmel_i2c_client_priv *i2c_priv = work_data->ctx;
> >       struct hwrng *rng = areq;
> >
> > -     if (status)
> > +     if (status) {
> >               dev_warn_ratelimited(&i2c_priv->client->dev,
> >                                    "i2c transaction failed (%d)\n",
> >                                    status);
> > +             kfree(work_data);
> > +             rng->priv = 0;
>
> Why is this necessary? It appears that rng_read_nonblocking already
> zeroes rng->priv.
>

IMHO this is not the same. The patch targets the error path. If the
`status` in `atmel_sha204a_rng_done()` is failed, then failed `work_data` is
still assigned and `rng->priv` is not zeroed at the moment. Only a
subsequent call to `rng_read_nonblocking()` will set `rng->priv = 0;`


The call order is something like this:
1. atmel_sha204a_init // module setup
2. atmel_sha204a_rng_read_nonblocking // call 1
3. atmel_sha204a_rng_done             // if fail, still copies
work_data <-- patch clears here
...
4. atmel_sha204a_rng_read_nonblocking // call 2, clears rng->priv = 0

Originally this was a sashiko finding, when I move the RNG part into the
common driver. Reason: Actually all Atmel ECC and Atmel SHA204a devices
support the same RNG mech. Thus part of my refactoring is moving it to the
common core driver atmel_i2c. I was advised by the maintainer to use also
sashiko's feedback. So, I went on identifying sashiko issues and have a
look into it, if I can provide a fix for it. This is one of them.

Sashiko asked:
"If the I2C transaction fails here, we still assign the work_data to
rng->priv. Since kmalloc_obj() uses GFP_ATOMIC and does not zero memory,
does this risk leaking uninitialized slab memory or stale data from
previous reads when the next non-blocking read copies from
work_data->cmd.data?"

ref: https://sashiko.dev/#/patchset/20260512224349.64621-1-l.rubusch%40gmail.com
[search for `atmel_i2c_rng_done` on that link]

I'm not sure about the risk or the (real) severity sashiko mentiones
here. But it seems to
be correct, when atmel_sha204a_rng_done() fails in the status, it
continues assigning the
failed result in the work_data:

    static void atmel_sha204a_rng_done(struct atmel_i2c_work_data *work_data,
                       void *areq, int status)
    {
        struct atmel_i2c_client_priv *i2c_priv = work_data->ctx;
        struct hwrng *rng = areq;

        if (status)
            dev_warn_ratelimited(&i2c_priv->client->dev,
                         "i2c transaction failed (%d)\n",
                         status);

        rng->priv = (unsigned long)work_data;
        atomic_dec(&i2c_priv->tfm_count);
    }

Hence, my proposed patch will stop it passing work_data, if status is
failed. It will not
assign rng->priv anymore then containing old data, but clear it. It
will free the `work_data`
to provoke a new allocation happening in `atmel_sha204a_rng_read_nonblocking()`.

The patch is sashiko and maintainer reviewed and solves sashikos complaints.
ref: https://sashiko.dev/#/patchset/20260609094723.47237-1-l.rubusch%40gmail.com
Setting `rng->priv = 0;` is rather safety here.

Thank you for asking. Accept, drop or modification needed - please,
leave me a note,
I'd highly appreciate.

Best,
L

> Thanks,
> --
> Email: Herbert Xu <herbert at gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt