Free the adap structure only after we are done using it.
This patch just moves the put_device() down a bit to avoid the
use after free.
Signed-off-by: Xu Wang <[email protected]>
---
drivers/i2c/i2c-core-base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 9f8dcd3..160d43e 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -2301,8 +2301,8 @@ void i2c_put_adapter(struct i2c_adapter *adap)
if (!adap)
return;
- put_device(&adap->dev);
module_put(adap->owner);
+ put_device(&adap->dev);
}
EXPORT_SYMBOL(i2c_put_adapter);
--
2.7.4
> Free the adap structure only after we are done using it.
This information can be reasonable.
> This patch just moves the put_device() down a bit to avoid the
> use after free.
I suggest to reconsider such a change because a device reference count
should eventually be decremented before decrementing the reference count
for the module which is managed by this programming interface.
Would you like to clarify the dependencies for such an use case any more?
Regards,
Markus
On Fri, Dec 27, 2019 at 09:34:32AM +0000, Xu Wang wrote:
> Free the adap structure only after we are done using it.
> This patch just moves the put_device() down a bit to avoid the
> use after free.
>
> Signed-off-by: Xu Wang <[email protected]>
Do you have a testcase to reproduce it?
I wonder because we are freeing the device structure which is embedded
inside the adap structure, not the adap structure itself. Or?
> ---
> drivers/i2c/i2c-core-base.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 9f8dcd3..160d43e 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -2301,8 +2301,8 @@ void i2c_put_adapter(struct i2c_adapter *adap)
> if (!adap)
> return;
>
> - put_device(&adap->dev);
> module_put(adap->owner);
> + put_device(&adap->dev);
> }
> EXPORT_SYMBOL(i2c_put_adapter);
>
> --
> 2.7.4
>
On Fri, Dec 27, 2019 at 09:34:32AM +0000, Xu Wang wrote:
> Free the adap structure only after we are done using it.
> This patch just moves the put_device() down a bit to avoid the
> use after free.
>
> Signed-off-by: Xu Wang <[email protected]>
Added a comment why we reverse the order for putting our stuff and
applied to for-next, thanks! This way, we get more testing until it hits
upstream. Still, stable tag added because we want it to be backported if
all is well.
On Fri, Dec 27, 2019 at 09:34:32AM +0000, Xu Wang wrote:
> Free the adap structure only after we are done using it.
> This patch just moves the put_device() down a bit to avoid the
> use after free.
>
> Signed-off-by: Xu Wang <[email protected]>
Applied to for-next, thanks!