[pvrusb2] [PATCH] pvrusb2: Fix oops on tear-down when radio support is not present

[Mike Isely]

The sysfs teardown issue right now is largely cosmetic - you just get 
log noise but the end result appears to still be correct.  Obviously 
this still needs to be fixed, because getting stack traces in the kernel 
message log generally sucks.

There actually is a pvrusb2 kernel config parameter you can set at 
compile time which will disable the sysfs piece of this.  (Not a 
run-time switch though.)

  -Mike

On Sun, 27 Oct 2019, Diego Rivera wrote:

> I had a thought about the sysfs teardown race you mentioned. Would it cause
> too many problems if instead you added a module parameter to selectively
> disable that bit and let the rest of the kernel do the teardown instead?
> 
> That might be enough of an optional workaround for now, since that does
> indeed seem like a bigger challenge...unless, of course, that approach
> brings more problems into focus...
> 
> Just a thought...
> 
> Cheers!
> 
> --
> 
> Diego Rivera
> 
> On Sun, Oct 27, 2019, 16:42 Mike Isely <isely at isely.net> wrote:
> 
> >
> > The tabs are probably my fault.  I've been too used to just letting
> > emacs deal automagically with indentation, and for projects I've done
> > over the past ten years I typically configure emacs to just never use
> > tabs at all.  However that driver source uses tabs - and I probably
> > forgot to adjust emacs to deal with it.
> >
> > Fortunately this isn't python, so tab vs spaces screwups won't break the
> > correctness of the change.  But yeah, it may cause readability problems.
> > I'll get that cleaned up.
> >
> >   -Mike
> >
> >
> > On Sun, 27 Oct 2019, Diego Rivera wrote:
> >
> > > The build is off to the races (24 thread concurrency), with the patch
> > incorporated. It was offset by
> > > 10 lines, and had to ignore whitespace (for some reason the Ubuntu folks
> > seem to have changed some
> > > indents to tabs...but only *some*....oh well).
> > > I'll let you know when it's ready and running.
> > > Cheers!
> > > On Sun, 2019-10-27 at 16:00 -0600, Diego Rivera wrote:
> > > > I'll do that today. I'm setting up a kernel build system now, should
> > be able to fire off a build
> > > > soon.
> > > > Cheers!
> > > > --
> > > >
> > > >
> > > >
> > > > Diego Rivera
> > > >
> > > > On Sun, 2019-10-27 at 16:47 -0500, Mike Isely wrote:
> > > > > If I can get independent confirmation that this definitely helps
> > matters, I will post the patch
> > > > > upstream.  Just being absolutely paranoid...
> > > > >   -Mike
> > > > > On Sun, 27 Oct 2019, Mike Isely wrote:
> > > > > > In some device configurations there's no radio or radio support in
> > thedriver.  That's OK, as
> > > > > > the driver sets itself up accordingly.  Howeveron tear-down in
> > these caes it's still trying to
> > > > > > tear down radiorelated context when there isn't anything there,
> > leading todereferences through
> > > > > > a null pointer and chaos follows.
> > > > > > How this bug survived unfixed for 11 years in the pvrusb2 driver
> > is a mystery to me.
> > > > > > Signed-off-by: Mike Isely <isely at pobox.com>---
> > drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 8
> > > > > > ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
> > > > > > diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
> > b/drivers/media/usb/pvrusb2/pvrusb2-
> > > > > > v4l2.cindex aa4fbc3e88cc..0a831849a2b0 100644---
> > a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c+++
> > > > > > b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c@@ -909,8 +909,11 @@
> > static void
> > > > > > pvr2_v4l2_internal_check(struct pvr2_channel *chp)
> > pvr2_v4l2_dev_disassociate_parent(vp
> > > > > > ->dev_video);   pvr2_v4l2_dev_disassociate_parent(vp->dev_radio);
> >      if (!list_empty(&vp-
> > > > > > >dev_video->devbase.fh_list) ||-
> > !list_empty(&vp->dev_radio->devbase.fh_list))+          ((
> > > > > > vp->dev_radio != NULL) &&+
> >  !list_empty(&vp->dev_radio->devbase.fh_list))) {+
> > > > > >         pvr2_trace(PVR2_TRACE_STRUCT,"pvr2_v4l2 internal_check
> > exit-empty id=%p",vp);
> > > > > > return;+        }       pvr2_v4l2_destroy_no_lock(vp); } @@ -946,7
> > +949,8 @@ static int
> > > > > > pvr2_v4l2_release(struct file *file)    kfree(fhp);     if
> > (vp->channel.mc_head-
> > > > > > >disconnect_flag &&
> >  list_empty(&vp->dev_video->devbase.fh_list) &&-         list_empty
> > > > > > (&vp->dev_radio->devbase.fh_list)) {+       ((vp->dev_radio ==
> > NULL) ||+             list_empt
> > > > > > y(&vp->dev_radio->devbase.fh_list))) {
> > pvr2_v4l2_destroy_no_lock(vp);  }
> > > > > > return 0;--
> > 2.20.1_______________________________________________pvrusb2 mailing
> > > > > > listpvrusb2 at isely.net
> > > > > > http://www.isely.net/cgi-bin/mailman/listinfo/pvrusb2
> > > > > >
> > >
> >
> > --
> >
> > Mike Isely
> > isely @ isely (dot) net
> > PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
> > _______________________________________________
> > pvrusb2 mailing list
> > pvrusb2 at isely.net
> > http://www.isely.net/cgi-bin/mailman/listinfo/pvrusb2
> >
> _______________________________________________
> pvrusb2 mailing list
> pvrusb2 at isely.net
> http://www.isely.net/cgi-bin/mailman/listinfo/pvrusb2
> 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8