Re: [PATCH] crypto: atmel-ecc - Remove duplicated error reporting in .remove()

[<Tudor.Ambarus@microchip.com>]

On 5/20/22 20:21, Uwe Kleine-König wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Returning an error value in an i2c remove callback results in an error
> message being emitted by the i2c core, but otherwise it doesn't make a
> difference. The device goes away anyhow and the devm cleanups are
> called.
> 
> As atmel_ecc_remove() already emits an error message on failure and the
> additional error message by the i2c core doesn't add any useful
> information, change the return value to zero to suppress this message.
> 
> Also make the error message a bit more drastical because when the device
> is still busy on remove, it's likely that it will access freed memory
> soon.
> 
> This patch is a preparation for making i2c remove callbacks return void.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

> ---
>  drivers/crypto/atmel-ecc.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> index 333fbefbbccb..6ba38275de8c 100644
> --- a/drivers/crypto/atmel-ecc.c
> +++ b/drivers/crypto/atmel-ecc.c
> @@ -349,8 +349,16 @@ static int atmel_ecc_remove(struct i2c_client *client)
> 
>         /* Return EBUSY if i2c client already allocated. */
>         if (atomic_read(&i2c_priv->tfm_count)) {
> -               dev_err(&client->dev, "Device is busy\n");
> -               return -EBUSY;
> +               /*
> +                * After we return here, the memory backing the device is freed.
> +                * That happens no matter what the return value of this function
> +                * is because in the Linux device model there is no error
> +                * handling for unbinding a driver.
> +                * If there is still some action pending, it probably involves
> +                * accessing the freed memory.
> +                */
> +               dev_emerg(&client->dev, "Device is busy, expect memory corruption.\n");
> +               return 0;
>         }
> 
>         crypto_unregister_kpp(&atmel_ecdh_nist_p256);
> 
> base-commit: 3123109284176b1532874591f7c81f3837bbdc17
> --
> 2.35.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel