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

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

[Lothar Rubusch]

When a non-blocking read operation is requested, the driver dynamically
allocates memory to track asynchronous transfer status. If the underlying
I2C transmission fails, atmel_sha204a_rng_done() logs a rate-limited
warning but incorrectly proceeds to cache the pointer to this uninitialized
buffer inside the rng->priv data field anyway.

On subsequent execution passes, atmel_sha204a_rng_read_nonblocking()
detects the stale rng->priv value, skips executing a hardware data read,
and copies up to 32 bytes of uninitialized kernel heap data from this
garbage memory pool straight back into the system's hwrng data stream.

Fix this information disclosure vector by immediately releasing the
allocated asynchronous work data buffer and explicitly clearing the
tracking pointer context whenever an I2C transaction returns a non-zero
error status.

Additionally, duplicate the tfm counter decrement within the new error
path to ensure the reference counter is properly released before executing
the early return, maintaining the driver's availability for subsequent
requests.

Fixes: da001fb651b0 ("crypto: atmel-i2c - add support for SHA204A random number generator")
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/crypto/atmel-sha204a.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

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;
+		atomic_dec(&i2c_priv->tfm_count);
+		return;
+	}
 
 	rng->priv = (unsigned long)work_data;
 	atomic_dec(&i2c_priv->tfm_count);

base-commit: 5624ea54f3ba5c83d2e5503411a31a8be0278c1e
-- 
2.53.0

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

[Thorsten Blum]

On Fri, May 29, 2026 at 09:43:36AM +0000, Lothar Rubusch wrote:
> When a non-blocking read operation is requested, the driver dynamically
> allocates memory to track asynchronous transfer status. If the underlying
> I2C transmission fails, atmel_sha204a_rng_done() logs a rate-limited
> warning but incorrectly proceeds to cache the pointer to this uninitialized
> buffer inside the rng->priv data field anyway.

The buffer is not necessarily uninitialized. cmd.data could also contain
a device error/status response or stale data from a previous request. An
uninitialized buffer is only one possibility.

Also, "rng->priv data field" is a bit confusing; maybe say that the
callback caches the work_data pointer in rng->priv instead?

> On subsequent execution passes, atmel_sha204a_rng_read_nonblocking()
> detects the stale rng->priv value, skips executing a hardware data read,

The cache-hit path still queues a new async read request and copies
invalid/stale data, but "skips a hardware data read" is probably not
correct. Did you mean the returned bytes aren't from a successful read?

> and copies up to 32 bytes of uninitialized kernel heap data from this
> garbage memory pool straight back into the system's hwrng data stream.

Same as above: it's not necessarily garbage or uninitialized data.

> Fix this information disclosure vector by immediately releasing the
> allocated asynchronous work data buffer and explicitly clearing the
> tracking pointer context whenever an I2C transaction returns a non-zero
> error status.
> 
> Additionally, duplicate the tfm counter decrement within the new error
> path to ensure the reference counter is properly released before executing
> the early return, maintaining the driver's availability for subsequent
> requests.
> 
> Fixes: da001fb651b0 ("crypto: atmel-i2c - add support for SHA204A random number generator")
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---
>  drivers/crypto/atmel-sha204a.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> 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;

Setting rng->priv = 0 is redundant here. rng_read_nonblocking() already
clears it before enqueuing new work, and the ->tfm_count gate prevents
others from setting it.

> +		atomic_dec(&i2c_priv->tfm_count);
> +		return;
> +	}
>  
>  	rng->priv = (unsigned long)work_data;
>  	atomic_dec(&i2c_priv->tfm_count);
> 
> base-commit: 5624ea54f3ba5c83d2e5503411a31a8be0278c1e

The fix itself looks good to me.

For a v2, could you please reword the commit message to be more precise,
and ideally drop the redundant rng->priv assignment? And feel free to
add a stable tag, as this should probably be backported.

With the above addressed, feel free to add:

Reviewed-by: Thorsten Blum <thorsten.blum@linux.dev>

Thanks,
Thorsten