Re: [PATCH] fix WARNING in pvr2_i2c_core_done

Re: [PATCH] fix WARNING in pvr2_i2c_core_done

[Dan Carpenter]

The subject isn't right.  Cc the correct people from get_maintainer.pl

On Tue, Jul 21, 2020 at 01:26:42PM +0530, B K Karthik wrote:
> i2c_acpi_remove_space_handler makes a call to
> kmem_cache_free() through acpi_ut_delete_generic_state
> in drivers/acpi/osl.c. since this removes the kobject,
> there is a warning thrown in i2c_del_adapter. The group
> can not be found because it has already been removed.

The commit message needs to have a cut and paste of the warning.
I don't think you can't ask syzbot to test linux-next when the patch is
not in linux-next.

https://lkml.org/lkml/2019/9/25/302

There was some discussion about this bug in Sept and it looked like the
correct fix was to unregister in the release handler instead of the
disconnect handler.  I'm not sure if the pvr2 maintainers were ever
CC'd about this or if anyone wrote a patch.

regards,
dan carpenter

> 
> Reported-by: syzbot+e74a998ca8f1df9cc332@syzkaller.appspotmail.com
> Signed-off-by: B K Karthik <bkkarthik@pesu.pes.edu>
> ---
>  drivers/i2c/i2c-core-base.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 26f03a14a478..cecf27fcc4f9 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -1527,7 +1527,8 @@ void i2c_del_adapter(struct i2c_adapter *adap)
>  		dev_dbg(&adap->dev, "Removing %s at 0x%x\n", client->name,
>  			client->addr);
>  		list_del(&client->detected);
> -		i2c_unregister_device(client);
> +		if (client->dev->kobj)
> +			i2c_unregister_device(client);
>  	}
>  	mutex_unlock(&adap->userspace_clients_lock);
>  
> -- 
> 2.20.1
> 
> -- 
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20200721075642.4cdlzbewml5jwbwm%40pesu.pes.edu.

Re: [PATCH] fix WARNING in pvr2_i2c_core_done

[B K Karthik]

On Tue, Jul 21, 2020 at 2:05 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> The subject isn't right.  Cc the correct people from get_maintainer.pl

I'm sorry, but I just wanted syzbot to test this because I was unable
to test it myself for some reason
>
> On Tue, Jul 21, 2020 at 01:26:42PM +0530, B K Karthik wrote:
> > i2c_acpi_remove_space_handler makes a call to
> > kmem_cache_free() through acpi_ut_delete_generic_state
> > in drivers/acpi/osl.c. since this removes the kobject,
> > there is a warning thrown in i2c_del_adapter. The group
> > can not be found because it has already been removed.
>
> The commit message needs to have a cut and paste of the warning.
> I don't think you can't ask syzbot to test linux-next when the patch is
> not in linux-next.
>
> https://lkml.org/lkml/2019/9/25/302
>
> There was some discussion about this bug in Sept and it looked like the
> correct fix was to unregister in the release handler instead of the
> disconnect handler.  I'm not sure if the pvr2 maintainers were ever
> CC'd about this or if anyone wrote a patch.
>
Yes, I'm aware of that discussion. I am not sure what exactly you mean
by release and disconnect handlers though.
And yes, I did not find a patch that makes the change you said.

Forgive me, but i noticed this:

in i2c_del_adapter+0x373/0x660 drivers/i2c/i2c-core-base.c:1516
(quoted from crash log)
i2c_acpi_remove_space_handler(adap); that takes us to
drivers/i2c/i2c-core-acpi.c:753
acpi_remove_address_space_handler() that takes us to
drivers/acpi/acpica/evxfregn.c:205
acpi_ut_remove_reference() that takes us to drivers/acpi/acpica/utdelete.c:736
acpi_ut_update_object_reference() that takes us to
drivers/acpi/acpica/utdelete.c:654
acpi_ut_delete_generic_state() that takes us to drivers/acpi/osl.c:1708

where a call to kmem_cache_free() is made.
Hence, the object does not exist anymore. (am i right?)

now that i know the object does exist, was it meant to be '0-0011' ?

later in the same function
in i2c_del_adapter+0x373/0x660 drivers/i2c/i2c-core-base.c:1532
(function name quoted from crash log)
a call to i2c_unregister_device() is made, where it tries to
unregister the device and it is a 2 pass process.

it tries to unregister the client device first, and then the dummy
device since we can not remove
the dummy devices during the first pass. (comment in
drivers/i2c/i2c-core-base.c:1540)

hence, the problem is not with the sysfs group, but with the invalid kobject.
that is why in the bug reported, it says kobject '0-0011' (am i right?)

if this is what you meant, sorry for wasting your time.

I thought since a call to kmem_cache_free() is being made, the kobject
was removed.
thus, i sent a patch with the if() statement. If i am thinking the
wrong way, please let me know.

thanks,

karthik