[PATCH 1/1] crypto: atmel-ecc - fix use after free situation

[PATCH 1/1] crypto: atmel-ecc - fix use after free situation

[Lothar Rubusch]

Fixes a possible race condition, when having multiple of such devices
attached (identified by sashiko feedback).

The Scenario:
    Thread A (Device 1 Probe): Successfully adds i2c_priv to the global
             list (Line 324). The lock is released.
    Thread B (An active crypto request): Concurrently calls
              atmel_ecc_i2c_client_alloc(). It scans the global list, sees
              Device 1, and assigns a crypto job to it.
    Thread A: Moves to line 332. crypto_register_kpp() fails (e.g., out of
              memory or name clash).
    Thread A: Enters the error path. It removes Device 1 from the list and
              frees the i2c_priv memory.
    Thread B: Is still actively trying to talk to the I2C hardware using
              the i2c_priv pointer it grabbed in Step 2. The memory is now
              gone. Result: Kernel crash (Use-After-Free).

Fixes: 11105693fa05 ("crypto: atmel-ecc - introduce Microchip / Atmel ECC driver")
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/crypto/atmel-ecc.c | 10 ++++++++++
 drivers/crypto/atmel-i2c.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 0ca02995a1de..d391fe1462f6 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -218,6 +218,8 @@ static struct i2c_client *atmel_ecc_i2c_client_alloc(void)
 
 	list_for_each_entry(i2c_priv, &driver_data.i2c_client_list,
 			    i2c_client_list_node) {
+		if (!i2c_priv->ready)
+			continue;
 		tfm_cnt = atomic_read(&i2c_priv->tfm_count);
 		if (tfm_cnt < min_tfm_cnt) {
 			min_tfm_cnt = tfm_cnt;
@@ -322,20 +324,24 @@ static int atmel_ecc_probe(struct i2c_client *client)
 		return ret;
 
 	i2c_priv = i2c_get_clientdata(client);
+	i2c_priv->ready = false;
 
 	spin_lock(&driver_data.i2c_list_lock);
 	list_add_tail(&i2c_priv->i2c_client_list_node,
 		      &driver_data.i2c_client_list);
+	i2c_priv->ready = true;
 	spin_unlock(&driver_data.i2c_list_lock);
 
 	ret = crypto_register_kpp(&atmel_ecdh_nist_p256);
 	if (ret) {
 		spin_lock(&driver_data.i2c_list_lock);
+		i2c_priv->ready = false;
 		list_del(&i2c_priv->i2c_client_list_node);
 		spin_unlock(&driver_data.i2c_list_lock);
 
 		dev_err(&client->dev, "%s alg registration failed\n",
 			atmel_ecdh_nist_p256.base.cra_driver_name);
+		return ret;
 	} else {
 		dev_info(&client->dev, "atmel ecc algorithms registered in /proc/crypto\n");
 	}
@@ -347,6 +353,10 @@ static void atmel_ecc_remove(struct i2c_client *client)
 {
 	struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
 
+	spin_lock(&driver_data.i2c_list_lock);
+	i2c_priv->ready = false;
+	spin_unlock(&driver_data.i2c_list_lock);
+
 	/* Return EBUSY if i2c client already allocated. */
 	if (atomic_read(&i2c_priv->tfm_count)) {
 		/*
diff --git a/drivers/crypto/atmel-i2c.h b/drivers/crypto/atmel-i2c.h
index 72f04c15682f..e3b12030f9c4 100644
--- a/drivers/crypto/atmel-i2c.h
+++ b/drivers/crypto/atmel-i2c.h
@@ -129,6 +129,7 @@ struct atmel_ecc_driver_data {
  * @wake_token_sz       : size in bytes of the wake_token
  * @tfm_count           : number of active crypto transformations on i2c client
  * @hwrng               : hold the hardware generated rng
+ * @ready               : hw client is ready to use
  *
  * Reads and writes from/to the i2c client are sequential. The first byte
  * transmitted to the device is treated as the byte size. Any attempt to send
@@ -145,6 +146,7 @@ struct atmel_i2c_client_priv {
 	size_t wake_token_sz;
 	atomic_t tfm_count ____cacheline_aligned;
 	struct hwrng hwrng;
+	bool ready;
 };
 
 /**

base-commit: 5624ea54f3ba5c83d2e5503411a31a8be0278c1e
-- 
2.53.0

Re: [PATCH 1/1] crypto: atmel-ecc - fix use after free situation

[Thorsten Blum]

On Fri, May 29, 2026 at 09:27:03AM +0000, Lothar Rubusch wrote:
> Fixes a possible race condition, when having multiple of such devices
> attached (identified by sashiko feedback).
> 
> The Scenario:
>     Thread A (Device 1 Probe): Successfully adds i2c_priv to the global
>              list (Line 324). The lock is released.
>     Thread B (An active crypto request): Concurrently calls
>               atmel_ecc_i2c_client_alloc(). It scans the global list, sees
>               Device 1, and assigns a crypto job to it.
>     Thread A: Moves to line 332. crypto_register_kpp() fails (e.g., out of
>               memory or name clash).
>     Thread A: Enters the error path. It removes Device 1 from the list and
>               frees the i2c_priv memory.
>     Thread B: Is still actively trying to talk to the I2C hardware using
>               the i2c_priv pointer it grabbed in Step 2. The memory is now
>               gone. Result: Kernel crash (Use-After-Free).
> 
> Fixes: 11105693fa05 ("crypto: atmel-ecc - introduce Microchip / Atmel ECC driver")
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---
>  drivers/crypto/atmel-ecc.c | 10 ++++++++++
>  drivers/crypto/atmel-i2c.h |  2 ++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> index 0ca02995a1de..d391fe1462f6 100644
> --- a/drivers/crypto/atmel-ecc.c
> +++ b/drivers/crypto/atmel-ecc.c
> @@ -218,6 +218,8 @@ static struct i2c_client *atmel_ecc_i2c_client_alloc(void)
>  
>  	list_for_each_entry(i2c_priv, &driver_data.i2c_client_list,
>  			    i2c_client_list_node) {
> +		if (!i2c_priv->ready)
> +			continue;
>  		tfm_cnt = atomic_read(&i2c_priv->tfm_count);
>  		if (tfm_cnt < min_tfm_cnt) {
>  			min_tfm_cnt = tfm_cnt;
> @@ -322,20 +324,24 @@ static int atmel_ecc_probe(struct i2c_client *client)
>  		return ret;
>  
>  	i2c_priv = i2c_get_clientdata(client);
> +	i2c_priv->ready = false;
>  
>  	spin_lock(&driver_data.i2c_list_lock);
>  	list_add_tail(&i2c_priv->i2c_client_list_node,
>  		      &driver_data.i2c_client_list);
> +	i2c_priv->ready = true;
>  	spin_unlock(&driver_data.i2c_list_lock);
>  
>  	ret = crypto_register_kpp(&atmel_ecdh_nist_p256);
>  	if (ret) {
>  		spin_lock(&driver_data.i2c_list_lock);
> +		i2c_priv->ready = false;
>  		list_del(&i2c_priv->i2c_client_list_node);
>  		spin_unlock(&driver_data.i2c_list_lock);
>  
>  		dev_err(&client->dev, "%s alg registration failed\n",
>  			atmel_ecdh_nist_p256.base.cra_driver_name);
> +		return ret;
>  	} else {
>  		dev_info(&client->dev, "atmel ecc algorithms registered in /proc/crypto\n");
>  	}
> @@ -347,6 +353,10 @@ static void atmel_ecc_remove(struct i2c_client *client)
>  {
>  	struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
>  
> +	spin_lock(&driver_data.i2c_list_lock);
> +	i2c_priv->ready = false;
> +	spin_unlock(&driver_data.i2c_list_lock);
> +
>  	/* Return EBUSY if i2c client already allocated. */
>  	if (atomic_read(&i2c_priv->tfm_count)) {
>  		/*
> diff --git a/drivers/crypto/atmel-i2c.h b/drivers/crypto/atmel-i2c.h
> index 72f04c15682f..e3b12030f9c4 100644
> --- a/drivers/crypto/atmel-i2c.h
> +++ b/drivers/crypto/atmel-i2c.h
> @@ -129,6 +129,7 @@ struct atmel_ecc_driver_data {
>   * @wake_token_sz       : size in bytes of the wake_token
>   * @tfm_count           : number of active crypto transformations on i2c client
>   * @hwrng               : hold the hardware generated rng
> + * @ready               : hw client is ready to use
>   *
>   * Reads and writes from/to the i2c client are sequential. The first byte
>   * transmitted to the device is treated as the byte size. Any attempt to send
> @@ -145,6 +146,7 @@ struct atmel_i2c_client_priv {
>  	size_t wake_token_sz;
>  	atomic_t tfm_count ____cacheline_aligned;
>  	struct hwrng hwrng;
> +	bool ready;
>  };

I don't think the ready flag fixes the race. A concurrent tfm can still
bind to the shared I2C client after atmel_ecc_probe() adds it to the
global list and marks it as ready, but before crypto_register_kpp()
fails.

Thanks,
Thorsten