| Message ID | 1577439272-10362-1-git-send-email-vulab@iscas.ac.cn |
|---|---|
| State | Accepted |
| Headers | show |
| Series | None | expand |
> 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 <vulab@iscas.ac.cn> 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 <vulab@iscas.ac.cn> 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 <vulab@iscas.ac.cn> Applied to for-next, thanks!
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);
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 <vulab@iscas.ac.cn> --- drivers/i2c/i2c-core-base.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)