[PATCH] media: pvrusb2: fix warning in pvr2_i2c_core_done

[PATCH] media: pvrusb2: fix warning in pvr2_i2c_core_done

[Anirudh Rayabharam]

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(-)

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);
-- 
2.26.2

Re: [PATCH] media: pvrusb2: fix warning in pvr2_i2c_core_done

[Greg KH]

On Thu, Apr 01, 2021 at 06:03:38PM +0530, 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
> 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(-)
> 
> 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);
> -- 
> 2.26.2

Looks sane to me, nice work tracking this down.

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH] media: pvrusb2: fix warning in pvr2_i2c_core_done

[Hans Verkuil]

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);
> 

Re: [PATCH] media: pvrusb2: fix warning in pvr2_i2c_core_done

[Anirudh Rayabharam]

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);
> > 
> 

Re: [PATCH] media: pvrusb2: fix warning in pvr2_i2c_core_done

[Anirudh Rayabharam]

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.