| Message ID | 20210401123338.3937-1-mail@anirudhrb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: pvrusb2: fix warning in pvr2_i2c_core_done | expand |
On 01/04/2021 14:33, Anirudh Rayabharam wrote: > syzbot has reported the following warning in pvr2_i2c_done: > > sysfs group 'power' not found for kobject '1-0043' > > When the device is disconnected (pvr_hdw_disconnect), the i2c adapter is > not unregistered along with the USB and vl42 teardown. As part of the vl42 -> v4l2 > USB device disconnect, the sysfs files of the subdevices are also > deleted. So, by the time pvr_i2c_core_done is called by > pvr_context_destroy, the sysfs files have been deleted. > > To fix this, unregister the i2c adapter too in pvr_hdw_disconnect. Make > the device deregistration code shared by calling pvr_hdw_disconnect from > pvr2_hdw_destory. destory -> destroy > > Reported-and-tested-by: syzbot+e74a998ca8f1df9cc332@syzkaller.appspotmail.com > Signed-off-by: Anirudh Rayabharam <mail@anirudhrb.com> > --- > drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > index f4a727918e35..791227787ff5 100644 > --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > @@ -2676,9 +2676,7 @@ void pvr2_hdw_destroy(struct pvr2_hdw *hdw) > pvr2_stream_destroy(hdw->vid_stream); > hdw->vid_stream = NULL; > } > - pvr2_i2c_core_done(hdw); > - v4l2_device_unregister(&hdw->v4l2_dev); I think this should still remain since pvr2_hdw_disconnect() doesn't call v4l2_device_unregister(). Can you test that with syzbot? Regards, Hans > - pvr2_hdw_remove_usb_stuff(hdw); > + pvr2_hdw_disconnect(hdw); > mutex_lock(&pvr2_unit_mtx); > do { > if ((hdw->unit_number >= 0) && > @@ -2705,6 +2703,7 @@ void pvr2_hdw_disconnect(struct pvr2_hdw *hdw) > { > pvr2_trace(PVR2_TRACE_INIT,"pvr2_hdw_disconnect(hdw=%p)",hdw); > LOCK_TAKE(hdw->big_lock); > + pvr2_i2c_core_done(hdw); > LOCK_TAKE(hdw->ctl_lock); > pvr2_hdw_remove_usb_stuff(hdw); > LOCK_GIVE(hdw->ctl_lock); >
On Tue, Apr 06, 2021 at 11:38:25AM +0200, Hans Verkuil wrote: > On 01/04/2021 14:33, Anirudh Rayabharam wrote: > > syzbot has reported the following warning in pvr2_i2c_done: > > > > sysfs group 'power' not found for kobject '1-0043' > > > > When the device is disconnected (pvr_hdw_disconnect), the i2c adapter is > > not unregistered along with the USB and vl42 teardown. As part of the > > vl42 -> v4l2 > > > USB device disconnect, the sysfs files of the subdevices are also > > deleted. So, by the time pvr_i2c_core_done is called by > > pvr_context_destroy, the sysfs files have been deleted. > > > > To fix this, unregister the i2c adapter too in pvr_hdw_disconnect. Make > > the device deregistration code shared by calling pvr_hdw_disconnect from > > pvr2_hdw_destory. > > destory -> destroy > Ack, will fix these typos in v2. > > > > Reported-and-tested-by: syzbot+e74a998ca8f1df9cc332@syzkaller.appspotmail.com > > Signed-off-by: Anirudh Rayabharam <mail@anirudhrb.com> > > --- > > drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > > index f4a727918e35..791227787ff5 100644 > > --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > > +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > > @@ -2676,9 +2676,7 @@ void pvr2_hdw_destroy(struct pvr2_hdw *hdw) > > pvr2_stream_destroy(hdw->vid_stream); > > hdw->vid_stream = NULL; > > } > > - pvr2_i2c_core_done(hdw); > > - v4l2_device_unregister(&hdw->v4l2_dev); > > I think this should still remain since pvr2_hdw_disconnect() doesn't call > v4l2_device_unregister(). Then we might run into the same warning again. pvr2_hdw_disconnect() calls pvr2_hdw_remove_usb_stuff() which calls v4l2_device_disconnect(). Perhaps there we should call v4l2_device_unregister() instead? > > Can you test that with syzbot? Will do. Thanks! - Anirudh. > > Regards, > > Hans > > > - pvr2_hdw_remove_usb_stuff(hdw); > > + pvr2_hdw_disconnect(hdw); > > mutex_lock(&pvr2_unit_mtx); > > do { > > if ((hdw->unit_number >= 0) && > > @@ -2705,6 +2703,7 @@ void pvr2_hdw_disconnect(struct pvr2_hdw *hdw) > > { > > pvr2_trace(PVR2_TRACE_INIT,"pvr2_hdw_disconnect(hdw=%p)",hdw); > > LOCK_TAKE(hdw->big_lock); > > + pvr2_i2c_core_done(hdw); > > LOCK_TAKE(hdw->ctl_lock); > > pvr2_hdw_remove_usb_stuff(hdw); > > LOCK_GIVE(hdw->ctl_lock); > > >
On Tue, Apr 06, 2021 at 11:38:25AM +0200, Hans Verkuil wrote: > On 01/04/2021 14:33, Anirudh Rayabharam wrote: > > syzbot has reported the following warning in pvr2_i2c_done: > > > > sysfs group 'power' not found for kobject '1-0043' > > > > When the device is disconnected (pvr_hdw_disconnect), the i2c adapter is > > not unregistered along with the USB and vl42 teardown. As part of the > > vl42 -> v4l2 > > > USB device disconnect, the sysfs files of the subdevices are also > > deleted. So, by the time pvr_i2c_core_done is called by > > pvr_context_destroy, the sysfs files have been deleted. > > > > To fix this, unregister the i2c adapter too in pvr_hdw_disconnect. Make > > the device deregistration code shared by calling pvr_hdw_disconnect from > > pvr2_hdw_destory. > > destory -> destroy > > > > > Reported-and-tested-by: syzbot+e74a998ca8f1df9cc332@syzkaller.appspotmail.com > > Signed-off-by: Anirudh Rayabharam <mail@anirudhrb.com> > > --- > > drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > > index f4a727918e35..791227787ff5 100644 > > --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > > +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > > @@ -2676,9 +2676,7 @@ void pvr2_hdw_destroy(struct pvr2_hdw *hdw) > > pvr2_stream_destroy(hdw->vid_stream); > > hdw->vid_stream = NULL; > > } > > - pvr2_i2c_core_done(hdw); > > - v4l2_device_unregister(&hdw->v4l2_dev); > > I think this should still remain since pvr2_hdw_disconnect() doesn't call > v4l2_device_unregister(). > > Can you test that with syzbot? Sent v2 with this change. Tested it with syzbot and didn't find any problems. Thanks. - Anirudh.
diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c index f4a727918e35..791227787ff5 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c @@ -2676,9 +2676,7 @@ void pvr2_hdw_destroy(struct pvr2_hdw *hdw) pvr2_stream_destroy(hdw->vid_stream); hdw->vid_stream = NULL; } - pvr2_i2c_core_done(hdw); - v4l2_device_unregister(&hdw->v4l2_dev); - pvr2_hdw_remove_usb_stuff(hdw); + pvr2_hdw_disconnect(hdw); mutex_lock(&pvr2_unit_mtx); do { if ((hdw->unit_number >= 0) && @@ -2705,6 +2703,7 @@ void pvr2_hdw_disconnect(struct pvr2_hdw *hdw) { pvr2_trace(PVR2_TRACE_INIT,"pvr2_hdw_disconnect(hdw=%p)",hdw); LOCK_TAKE(hdw->big_lock); + pvr2_i2c_core_done(hdw); LOCK_TAKE(hdw->ctl_lock); pvr2_hdw_remove_usb_stuff(hdw); LOCK_GIVE(hdw->ctl_lock);
syzbot has reported the following warning in pvr2_i2c_done: sysfs group 'power' not found for kobject '1-0043' When the device is disconnected (pvr_hdw_disconnect), the i2c adapter is not unregistered along with the USB and vl42 teardown. As part of the USB device disconnect, the sysfs files of the subdevices are also deleted. So, by the time pvr_i2c_core_done is called by pvr_context_destroy, the sysfs files have been deleted. To fix this, unregister the i2c adapter too in pvr_hdw_disconnect. Make the device deregistration code shared by calling pvr_hdw_disconnect from pvr2_hdw_destory. Reported-and-tested-by: syzbot+e74a998ca8f1df9cc332@syzkaller.appspotmail.com Signed-off-by: Anirudh Rayabharam <mail@anirudhrb.com> --- drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)