| Message ID | alpine.DEB.2.21.1911052034300.31133@sheridan.isely.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pvrusb2: Fix oops on tear-down when radio support is not present | expand |
Hi Mike, For some reason your mailer didn't include a "From:" line, only a "Reply-To:" line. This means that the patch authorship is not detected by patchwork and git. Can you repost with a valid From: line in the email header? Thanks! Hans On 11/6/19 3:36 AM, wrote: > In some device configurations there's no radio or radio support in the > driver. That's OK, as the driver sets itself up accordingly. However > on tear-down in these caes it's still trying to tear down radio > related context when there isn't anything there, leading to > dereferences 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@pobox.com> > --- > drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c > index aa4fbc3e88cc..339119f6cc23 100644 > --- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c > +++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c > @@ -909,8 +909,12 @@ 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 +950,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_empty(&vp->dev_radio->devbase.fh_list))) { > pvr2_v4l2_destroy_no_lock(vp); > } > return 0; >
Thanks for spotting that. I think I had eliminated the from header because I thought I was seeing a duplicate from header and didn't want to cause confusion. That'll teach me. It's been reposted. -Mike On Wed, 6 Nov 2019, Hans Verkuil wrote: > Hi Mike, > > For some reason your mailer didn't include a "From:" line, only a "Reply-To:" > line. This means that the patch authorship is not detected by patchwork and > git. Can you repost with a valid From: line in the email header? > > Thanks! > > Hans > [...]
diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c index aa4fbc3e88cc..339119f6cc23 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c @@ -909,8 +909,12 @@ 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 +950,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_empty(&vp->dev_radio->devbase.fh_list))) { pvr2_v4l2_destroy_no_lock(vp); } return 0;
In some device configurations there's no radio or radio support in the driver. That's OK, as the driver sets itself up accordingly. However on tear-down in these caes it's still trying to tear down radio related context when there isn't anything there, leading to dereferences 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@pobox.com> --- drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)