[stable] [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open

[stable] [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open

[Jarod Wilson]

From: Mark Jenks
https://www.redhat.com/mailman/private/video4linux-list/2009-January/msg00041.html

The Hauppauge WinTV HVR-1800 tv tuner card has both digital and analog
abilities, both of which are supported by v4l/dvb under 2.6.27.y. The analog
side also features a hardware mpeg2 encoder. The HVR-1250 tv tuner card
has both digital and analog abilities, but analog isn't currently supported
under any kernel. These cards both utilize the cx23885 driver, but with
slightly different usage. When the code paths for each card is executed,
they wind up poking a cx23885_devlist, which contains devices from both
of the cards, and access attempts are made to portions of 'struct
cx23885_dev' that aren't valid for that device. Simply add some extra
checks before trying to access these structs.

More gory details:
  http://article.gmane.org/gmane.linux.drivers.dvb/46630

This was triggering on my own system at home w/both cards in it, and
no longer happens with this patch included.

Signed-off-by: Jarod Wilson <jarod@redhat.com>
Reviewed-by: Michael Krufky <mkrufky@linuxtv.org>

---

 drivers/media/video/cx23885/cx23885-417.c   |    2 +-
 drivers/media/video/cx23885/cx23885-video.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/cx23885/cx23885-417.c b/drivers/media/video/cx23885/cx23885-417.c
index 7b0e8c0..19154b6 100644
--- a/drivers/media/video/cx23885/cx23885-417.c
+++ b/drivers/media/video/cx23885/cx23885-417.c
@@ -1585,7 +1585,7 @@ static int mpeg_open(struct inode *inode, struct file *file)
 
 	list_for_each(list, &cx23885_devlist) {
 		h = list_entry(list, struct cx23885_dev, devlist);
-		if (h->v4l_device->minor == minor) {
+		if (h->v4l_device && h->v4l_device->minor == minor) {
 			dev = h;
 			break;
 		}
diff --git a/drivers/media/video/cx23885/cx23885-video.c b/drivers/media/video/cx23885/cx23885-video.c
index 6047c78..a2b5a0c 100644
--- a/drivers/media/video/cx23885/cx23885-video.c
+++ b/drivers/media/video/cx23885/cx23885-video.c
@@ -733,7 +733,7 @@ static int video_open(struct inode *inode, struct file *file)
 
 	list_for_each(list, &cx23885_devlist) {
 		h = list_entry(list, struct cx23885_dev, devlist);
-		if (h->video_dev->minor == minor) {
+		if (h->video_dev && h->video_dev->minor == minor) {
 			dev  = h;
 			type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 		}


-- 
Jarod Wilson
jarod@redhat.com

Fwd: [stable] [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open

[Michael Krufky]

Can we have this merged into -stable?  Jarod Wilson sent this last
month, but he left off the cc to stable@kernel.org

Signed-off-by: Michael Krufky <mkrufky@linuxtv.org>


---------- Forwarded message ----------
From: Jarod Wilson <jarod@redhat.com>
Date: Tue, Feb 24, 2009 at 6:00 PM
Subject: [stable] [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open
To: linux-kernel@vger.kernel.org
Cc: Mike Krufky <mkrufky@linuxtv.org>


From: Mark Jenks
https://www.redhat.com/mailman/private/video4linux-list/2009-January/msg00041.html

The Hauppauge WinTV HVR-1800 tv tuner card has both digital and analog
abilities, both of which are supported by v4l/dvb under 2.6.27.y. The analog
side also features a hardware mpeg2 encoder. The HVR-1250 tv tuner card
has both digital and analog abilities, but analog isn't currently supported
under any kernel. These cards both utilize the cx23885 driver, but with
slightly different usage. When the code paths for each card is executed,
they wind up poking a cx23885_devlist, which contains devices from both
of the cards, and access attempts are made to portions of 'struct
cx23885_dev' that aren't valid for that device. Simply add some extra
checks before trying to access these structs.

More gory details:
 http://article.gmane.org/gmane.linux.drivers.dvb/46630

This was triggering on my own system at home w/both cards in it, and
no longer happens with this patch included.

Signed-off-by: Jarod Wilson <jarod@redhat.com>
Reviewed-by: Michael Krufky <mkrufky@linuxtv.org>

---

 drivers/media/video/cx23885/cx23885-417.c   |    2 +-
 drivers/media/video/cx23885/cx23885-video.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/cx23885/cx23885-417.c
b/drivers/media/video/cx23885/cx23885-417.c
index 7b0e8c0..19154b6 100644
--- a/drivers/media/video/cx23885/cx23885-417.c
+++ b/drivers/media/video/cx23885/cx23885-417.c
@@ -1585,7 +1585,7 @@ static int mpeg_open(struct inode *inode, struct
file *file)

       list_for_each(list, &cx23885_devlist) {
               h = list_entry(list, struct cx23885_dev, devlist);
-               if (h->v4l_device->minor == minor) {
+               if (h->v4l_device && h->v4l_device->minor == minor) {
                       dev = h;
                       break;
               }
diff --git a/drivers/media/video/cx23885/cx23885-video.c
b/drivers/media/video/cx23885/cx23885-video.c
index 6047c78..a2b5a0c 100644
--- a/drivers/media/video/cx23885/cx23885-video.c
+++ b/drivers/media/video/cx23885/cx23885-video.c
@@ -733,7 +733,7 @@ static int video_open(struct inode *inode, struct
file *file)

       list_for_each(list, &cx23885_devlist) {
               h = list_entry(list, struct cx23885_dev, devlist);
-               if (h->video_dev->minor == minor) {
+               if (h->video_dev && h->video_dev->minor == minor) {
                       dev  = h;
                       type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
               }


--
Jarod Wilson
jarod@redhat.com

Fwd: [stable] [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open

[Michael Krufky]

Can we have this merged into -stable?  Jarod Wilson sent this last
month, but he left off the cc to stable@kernel.org

Signed-off-by: Michael Krufky <mkrufky@linuxtv.org>


---------- Forwarded message ----------
From: Jarod Wilson <jarod@redhat.com>
Date: Tue, Feb 24, 2009 at 6:00 PM
Subject: [stable] [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open
To: linux-kernel@vger.kernel.org
Cc: Mike Krufky <mkrufky@linuxtv.org>


From: Mark Jenks
https://www.redhat.com/mailman/private/video4linux-list/2009-January/msg00041.html

The Hauppauge WinTV HVR-1800 tv tuner card has both digital and analog
abilities, both of which are supported by v4l/dvb under 2.6.27.y. The analog
side also features a hardware mpeg2 encoder. The HVR-1250 tv tuner card
has both digital and analog abilities, but analog isn't currently supported
under any kernel. These cards both utilize the cx23885 driver, but with
slightly different usage. When the code paths for each card is executed,
they wind up poking a cx23885_devlist, which contains devices from both
of the cards, and access attempts are made to portions of 'struct
cx23885_dev' that aren't valid for that device. Simply add some extra
checks before trying to access these structs.

More gory details:
 http://article.gmane.org/gmane.linux.drivers.dvb/46630

This was triggering on my own system at home w/both cards in it, and
no longer happens with this patch included.

Signed-off-by: Jarod Wilson <jarod@redhat.com>
Reviewed-by: Michael Krufky <mkrufky@linuxtv.org>

---

 drivers/media/video/cx23885/cx23885-417.c   |    2 +-
 drivers/media/video/cx23885/cx23885-video.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/cx23885/cx23885-417.c
b/drivers/media/video/cx23885/cx23885-417.c
index 7b0e8c0..19154b6 100644
--- a/drivers/media/video/cx23885/cx23885-417.c
+++ b/drivers/media/video/cx23885/cx23885-417.c
@@ -1585,7 +1585,7 @@ static int mpeg_open(struct inode *inode, struct
file *file)

       list_for_each(list, &cx23885_devlist) {
               h = list_entry(list, struct cx23885_dev, devlist);
-               if (h->v4l_device->minor == minor) {
+               if (h->v4l_device && h->v4l_device->minor == minor) {
                       dev = h;
                       break;
               }
diff --git a/drivers/media/video/cx23885/cx23885-video.c
b/drivers/media/video/cx23885/cx23885-video.c
index 6047c78..a2b5a0c 100644
--- a/drivers/media/video/cx23885/cx23885-video.c
+++ b/drivers/media/video/cx23885/cx23885-video.c
@@ -733,7 +733,7 @@ static int video_open(struct inode *inode, struct
file *file)

       list_for_each(list, &cx23885_devlist) {
               h = list_entry(list, struct cx23885_dev, devlist);
-               if (h->video_dev->minor == minor) {
+               if (h->video_dev && h->video_dev->minor == minor) {
                       dev  = h;
                       type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
               }


--
Jarod Wilson
jarod@redhat.com

Re: Fwd: [stable] [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open

[Andy Walls]

On Thu, 2009-03-12 at 16:24 -0400, Michael Krufky wrote:
> Can we have this merged into -stable?  Jarod Wilson sent this last
> month, but he left off the cc to stable@kernel.org
> 
> Signed-off-by: Michael Krufky <mkrufky@linuxtv.org>

Mike,

A version of this is already in the v4l-dvb hg development repository:

hg log -vp --limit 1 linux/drivers/media/video/cx23885/cx23885-417.c
hg log -vp --limit 2 linux/drivers/media/video/cx23885/cx23885-video.c 

I helped Mark work through the solution: I coded some of it, he coded
some of it and he also tested it.

Regards,
Andy


> 
> ---------- Forwarded message ----------
> From: Jarod Wilson <jarod@redhat.com>
> Date: Tue, Feb 24, 2009 at 6:00 PM
> Subject: [stable] [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open
> To: linux-kernel@vger.kernel.org
> Cc: Mike Krufky <mkrufky@linuxtv.org>
> 
> 
> From: Mark Jenks
> https://www.redhat.com/mailman/private/video4linux-list/2009-January/msg00041.html
> 
> The Hauppauge WinTV HVR-1800 tv tuner card has both digital and analog
> abilities, both of which are supported by v4l/dvb under 2.6.27.y. The analog
> side also features a hardware mpeg2 encoder. The HVR-1250 tv tuner card
> has both digital and analog abilities, but analog isn't currently supported
> under any kernel. These cards both utilize the cx23885 driver, but with
> slightly different usage. When the code paths for each card is executed,
> they wind up poking a cx23885_devlist, which contains devices from both
> of the cards, and access attempts are made to portions of 'struct
> cx23885_dev' that aren't valid for that device. Simply add some extra
> checks before trying to access these structs.
> 
> More gory details:
>  http://article.gmane.org/gmane.linux.drivers.dvb/46630
> 
> This was triggering on my own system at home w/both cards in it, and
> no longer happens with this patch included.
> 
> Signed-off-by: Jarod Wilson <jarod@redhat.com>
> Reviewed-by: Michael Krufky <mkrufky@linuxtv.org>
> 
> ---
> 
>  drivers/media/video/cx23885/cx23885-417.c   |    2 +-
>  drivers/media/video/cx23885/cx23885-video.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/video/cx23885/cx23885-417.c
> b/drivers/media/video/cx23885/cx23885-417.c
> index 7b0e8c0..19154b6 100644
> --- a/drivers/media/video/cx23885/cx23885-417.c
> +++ b/drivers/media/video/cx23885/cx23885-417.c
> @@ -1585,7 +1585,7 @@ static int mpeg_open(struct inode *inode, struct
> file *file)
> 
>        list_for_each(list, &cx23885_devlist) {
>                h = list_entry(list, struct cx23885_dev, devlist);
> -               if (h->v4l_device->minor == minor) {
> +               if (h->v4l_device && h->v4l_device->minor == minor) {
>                        dev = h;
>                        break;
>                }
> diff --git a/drivers/media/video/cx23885/cx23885-video.c
> b/drivers/media/video/cx23885/cx23885-video.c
> index 6047c78..a2b5a0c 100644
> --- a/drivers/media/video/cx23885/cx23885-video.c
> +++ b/drivers/media/video/cx23885/cx23885-video.c
> @@ -733,7 +733,7 @@ static int video_open(struct inode *inode, struct
> file *file)
> 
>        list_for_each(list, &cx23885_devlist) {
>                h = list_entry(list, struct cx23885_dev, devlist);
> -               if (h->video_dev->minor == minor) {
> +               if (h->video_dev && h->video_dev->minor == minor) {
>                        dev  = h;
>                        type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>                }
> 
> 
> --
> Jarod Wilson
> jarod@redhat.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: Fwd: [stable] [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open

[Andy Walls]

On Thu, 2009-03-12 at 16:24 -0400, Michael Krufky wrote:
> Can we have this merged into -stable?  Jarod Wilson sent this last
> month, but he left off the cc to stable@kernel.org
> 
> Signed-off-by: Michael Krufky <mkrufky@linuxtv.org>

Mike,

A version of this is already in the v4l-dvb hg development repository:

hg log -vp --limit 1 linux/drivers/media/video/cx23885/cx23885-417.c
hg log -vp --limit 2 linux/drivers/media/video/cx23885/cx23885-video.c 

I helped Mark work through the solution: I coded some of it, he coded
some of it and he also tested it.

Regards,
Andy


> 
> ---------- Forwarded message ----------
> From: Jarod Wilson <jarod@redhat.com>
> Date: Tue, Feb 24, 2009 at 6:00 PM
> Subject: [stable] [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open
> To: linux-kernel@vger.kernel.org
> Cc: Mike Krufky <mkrufky@linuxtv.org>
> 
> 
> From: Mark Jenks
> https://www.redhat.com/mailman/private/video4linux-list/2009-January/msg00041.html
> 
> The Hauppauge WinTV HVR-1800 tv tuner card has both digital and analog
> abilities, both of which are supported by v4l/dvb under 2.6.27.y. The analog
> side also features a hardware mpeg2 encoder. The HVR-1250 tv tuner card
> has both digital and analog abilities, but analog isn't currently supported
> under any kernel. These cards both utilize the cx23885 driver, but with
> slightly different usage. When the code paths for each card is executed,
> they wind up poking a cx23885_devlist, which contains devices from both
> of the cards, and access attempts are made to portions of 'struct
> cx23885_dev' that aren't valid for that device. Simply add some extra
> checks before trying to access these structs.
> 
> More gory details:
>  http://article.gmane.org/gmane.linux.drivers.dvb/46630
> 
> This was triggering on my own system at home w/both cards in it, and
> no longer happens with this patch included.
> 
> Signed-off-by: Jarod Wilson <jarod@redhat.com>
> Reviewed-by: Michael Krufky <mkrufky@linuxtv.org>
> 
> ---
> 
>  drivers/media/video/cx23885/cx23885-417.c   |    2 +-
>  drivers/media/video/cx23885/cx23885-video.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/video/cx23885/cx23885-417.c
> b/drivers/media/video/cx23885/cx23885-417.c
> index 7b0e8c0..19154b6 100644
> --- a/drivers/media/video/cx23885/cx23885-417.c
> +++ b/drivers/media/video/cx23885/cx23885-417.c
> @@ -1585,7 +1585,7 @@ static int mpeg_open(struct inode *inode, struct
> file *file)
> 
>        list_for_each(list, &cx23885_devlist) {
>                h = list_entry(list, struct cx23885_dev, devlist);
> -               if (h->v4l_device->minor == minor) {
> +               if (h->v4l_device && h->v4l_device->minor == minor) {
>                        dev = h;
>                        break;
>                }
> diff --git a/drivers/media/video/cx23885/cx23885-video.c
> b/drivers/media/video/cx23885/cx23885-video.c
> index 6047c78..a2b5a0c 100644
> --- a/drivers/media/video/cx23885/cx23885-video.c
> +++ b/drivers/media/video/cx23885/cx23885-video.c
> @@ -733,7 +733,7 @@ static int video_open(struct inode *inode, struct
> file *file)
> 
>        list_for_each(list, &cx23885_devlist) {
>                h = list_entry(list, struct cx23885_dev, devlist);
> -               if (h->video_dev->minor == minor) {
> +               if (h->video_dev && h->video_dev->minor == minor) {
>                        dev  = h;
>                        type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>                }
> 
> 
> --
> Jarod Wilson
> jarod@redhat.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: Fwd: [stable] [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open

[Michael Krufky]

Andy Walls wrote:
> On Thu, 2009-03-12 at 16:24 -0400, Michael Krufky wrote:
>   
>> Can we have this merged into -stable?  Jarod Wilson sent this last
>> month, but he left off the cc to stable@kernel.org
>>
>> Signed-off-by: Michael Krufky <mkrufky@linuxtv.org>
>>     
>
> Mike,
>
> A version of this is already in the v4l-dvb hg development repository:
>
> hg log -vp --limit 1 linux/drivers/media/video/cx23885/cx23885-417.c
> hg log -vp --limit 2 linux/drivers/media/video/cx23885/cx23885-video.c 
>
> I helped Mark work through the solution: I coded some of it, he coded
> some of it and he also tested it.
>
> Regards,
> Andy

I'm aware of that, Andy -- That's why I am sending this off to the 
-stable team for 2.6.27.y

Thanks & regards,

Mike

Re: Fwd: [stable] [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open

[Andy Walls]

On Thu, 2009-03-12 at 19:18 -0400, Michael Krufky wrote:
> Andy Walls wrote:
> > On Thu, 2009-03-12 at 16:24 -0400, Michael Krufky wrote:
> >   
> >> Can we have this merged into -stable?  Jarod Wilson sent this last
> >> month, but he left off the cc to stable@kernel.org
> >>
> >> Signed-off-by: Michael Krufky <mkrufky@linuxtv.org>
> >>     
> >
> > Mike,
> >
> > A version of this is already in the v4l-dvb hg development repository:
> >
> > hg log -vp --limit 1 linux/drivers/media/video/cx23885/cx23885-417.c
> > hg log -vp --limit 2 linux/drivers/media/video/cx23885/cx23885-video.c 
> >
> > I helped Mark work through the solution: I coded some of it, he coded
> > some of it and he also tested it.
> >
> > Regards,
> > Andy
> 
> I'm aware of that, Andy -- That's why I am sending this off to the 
> -stable team for 2.6.27.y

Ooops. Sorry for my cluelessness. :)

Regards,
Andy


> Thanks & regards,
> 
> Mike
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [stable] Fwd: [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open

[Greg KH]

On Thu, Mar 12, 2009 at 04:24:38PM -0400, Michael Krufky wrote:
> Can we have this merged into -stable?  Jarod Wilson sent this last
> month, but he left off the cc to stable@kernel.org

What is the git commit id of the patch in Linus's tree that matches up
with this?

thanks,

greg k-h

Re: [stable] Fwd: [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open

[Jarod Wilson]

Greg KH wrote:
> On Thu, Mar 12, 2009 at 04:24:38PM -0400, Michael Krufky wrote:
>> Can we have this merged into -stable?  Jarod Wilson sent this last
>> month, but he left off the cc to stable@kernel.org
> 
> What is the git commit id of the patch in Linus's tree that matches up
> with this?
> 
> thanks,
> 
> greg k-h


Now that I look closer, seems there's an even more complete patch 
already in linus' tree, commit id:

cd8f894eacf13996d920fdd2aef1afc55156b191

Shoulda just forwarded that along, but I was under the impression that 
area of code had changed significantly in 2.6.28 and the patch was no 
longer applicable. Maybe that was the v4l/dvb hg tree though... So I'd 
just grab that, and add:

Signed-off-by: Jarod Wilson <jarod@redhat.com>


--jarod

Re: [stable] Fwd: [PATCH] 2.6.27.y: fix NULL ptr deref in cx23885 video_open

[Greg KH]

On Thu, Mar 12, 2009 at 11:00:11PM -0400, Jarod Wilson wrote:
> Greg KH wrote:
>> On Thu, Mar 12, 2009 at 04:24:38PM -0400, Michael Krufky wrote:
>>> Can we have this merged into -stable?  Jarod Wilson sent this last
>>> month, but he left off the cc to stable@kernel.org
>> What is the git commit id of the patch in Linus's tree that matches up
>> with this?
>> thanks,
>> greg k-h
>
>
> Now that I look closer, seems there's an even more complete patch already 
> in linus' tree, commit id:
>
> cd8f894eacf13996d920fdd2aef1afc55156b191
>
> Shoulda just forwarded that along, but I was under the impression that area 
> of code had changed significantly in 2.6.28 and the patch was no longer 
> applicable. Maybe that was the v4l/dvb hg tree though... So I'd just grab 
> that, and add:
>
> Signed-off-by: Jarod Wilson <jarod@redhat.com>

Ok, will do that.

thanks,

greg k-h