Bug in atmel-ecc driver

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

[-- Attachment #1.1: Type: text/plain, Size: 1416 bytes --]

Hello,

TL;DR: when a device bound to the drivers/crypto/atmel-ecc.c driver is
unbound while tfm_count isn't zero, this probably results in a
use-after-free.

The .remove function has:

	if (atomic_read(&i2c_priv->tfm_count)) {
                dev_err(&client->dev, "Device is busy\n");
                return -EBUSY;
        }

before actually calling the cleanup stuff. If this branch is hit the
result is likely:

 - "Device is busy" from drivers/crypto/atmel-ecc.c
 - "remove failed (EBUSY), will be ignored" from the i2c core
 - the devm cleanup callbacks are called, including the one kfreeing
   *i2c_priv
 - at a later time atmel_ecc_i2c_client_free() is called which does
   atomic_dec(&i2c_priv->tfm_count);
 - *boom*

I think to fix that you need to call get_device for the i2c device
before increasing tfm_count (and a matching put_device when decreasing
it). Having said that the architecture of this driver looks strange to
me, so there might be nicer fixes (probably with more effort).

I noticed this issue while working on my quest to make i2c-remove
callbacks return void. So if you address this, it would be great if you
did that in a way that makes atmel_ecc_remove always return 0. 

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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