[PATCH] fix WARNING in pvr2_i2c_core_done

[B K Karthik]

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.

Reported-by: syzbot+e74a99...@syzkaller.appspotmail.com
Signed-off-by: B K Karthik <bkka...@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

[B K Karthik]

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
master

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
WARNING in pvr2_i2c_core_done

pvrusb2: You need to resolve the failing condition before this driver can function. There should be some earlier messages giving more information about the problem.
------------[ cut here ]------------
sysfs group 'power' not found for kobject '0-0011'
WARNING: CPU: 1 PID: 78 at fs/sysfs/group.c:279 sysfs_remove_group+0x126/0x170 fs/sysfs/group.c:279
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 78 Comm: pvrusb2-context Not tainted 5.8.0-rc6-next-20200720-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xf6/0x16e lib/dump_stack.c:118
panic+0x2aa/0x6e1 kernel/panic.c:231
__warn.cold+0x20/0x50 kernel/panic.c:600
report_bug+0x1bd/0x210 lib/bug.c:198
handle_bug+0x41/0x80 arch/x86/kernel/traps.c:234
exc_invalid_op+0x14/0x40 arch/x86/kernel/traps.c:254
asm_exc_invalid_op+0x12/0x20 arch/x86/include/asm/idtentry.h:545
RIP: 0010:sysfs_remove_group+0x126/0x170 fs/sysfs/group.c:279
Code: 48 89 d9 49 8b 14 24 48 b8 00 00 00 00 00 fc ff df 48 c1 e9 03 80 3c 01 00 75 37 48 8b 33 48 c7 c7 80 dd 11 86 e8 cc 0d 82 ff <0f> 0b eb 98 e8 d1 c7 d7 ff e9 01 ff ff ff 48 89 df e8 c4 c7 d7 ff
RSP: 0018:ffff8881d415fb88 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffffffff8637b680 RCX: 0000000000000000
RDX: ffff8881d4741900 RSI: ffffffff8129e643 RDI: ffffed103a82bf63
RBP: 0000000000000000 R08: 0000000000000001 R09: ffff8881db31fe8b
R10: 0000000000000000 R11: 0000000000000000 R12: ffff8881d3af1020
R13: ffffffff8637bc20 R14: 1ffff1103a82bf98 R15: dffffc0000000000
dpm_sysfs_remove+0x97/0xb0 drivers/base/power/sysfs.c:801
device_del+0x18b/0xd90 drivers/base/core.c:3075
device_unregister+0x22/0xc0 drivers/base/core.c:3130
i2c_unregister_device include/linux/err.h:41 [inline]
__unregister_client drivers/i2c/i2c-core-base.c:1476 [inline]
__unregister_client+0x95/0xa0 drivers/i2c/i2c-core-base.c:1472
device_for_each_child+0xf9/0x170 drivers/base/core.c:3230
i2c_del_adapter+0x37b/0x680 drivers/i2c/i2c-core-base.c:1539
pvr2_i2c_core_done+0x69/0xb6 drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c:652
pvr2_hdw_destroy+0x179/0x370 drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2680
pvr2_context_destroy+0x84/0x230 drivers/media/usb/pvrusb2/pvrusb2-context.c:70
pvr2_context_check drivers/media/usb/pvrusb2/pvrusb2-context.c:137 [inline]
pvr2_context_thread_func+0x64b/0x850 drivers/media/usb/pvrusb2/pvrusb2-context.c:158
kthread+0x392/0x470 kernel/kthread.c:292
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit: ab8be66e Add linux-next specific files for 20200720
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=1169f844900000
kernel config: https://syzkaller.appspot.com/x/.config?x=b952f3ba33bc1a00
dashboard link: https://syzkaller.appspot.com/bug?extid=e74a998ca8f1df9cc332
compiler: gcc (GCC) 10.1.0-syz 20200507

[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+e74a99...@syzkaller.appspotmail.com
> Signed-off-by: B K Karthik <bkka...@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-bug...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20200721075642.4cdlzbewml5jwbwm%40pesu.pes.edu.

[B K Karthik]

On Tue, Jul 21, 2020 at 2:05 PM Dan Carpenter <dan.ca...@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