* [PATCH 1/1] media: video/cx18, fix potential null dereference
@ 2010-01-10 8:56 Jiri Slaby
2010-01-11 23:48 ` Andy Walls
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2010-01-10 8:56 UTC (permalink / raw)
To: mchehab; +Cc: hverkuil, awalls, ivtv-devel, linux-media, linux-kernel,
jirislaby
Stanse found a potential null dereference in cx18_dvb_start_feed
and cx18_dvb_stop_feed. There is a check for stream being NULL,
but it is dereferenced earlier. Move the dereference after the
check.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/media/video/cx18/cx18-dvb.c | 18 ++++++++++--------
1 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/media/video/cx18/cx18-dvb.c b/drivers/media/video/cx18/cx18-dvb.c
index 71ad2d1..0ad5b63 100644
--- a/drivers/media/video/cx18/cx18-dvb.c
+++ b/drivers/media/video/cx18/cx18-dvb.c
@@ -213,10 +213,14 @@ static int cx18_dvb_start_feed(struct dvb_demux_feed *feed)
{
struct dvb_demux *demux = feed->demux;
struct cx18_stream *stream = (struct cx18_stream *) demux->priv;
- struct cx18 *cx = stream->cx;
+ struct cx18 *cx;
int ret;
u32 v;
+ if (!stream)
+ return -EINVAL;
+
+ cx = stream->cx;
CX18_DEBUG_INFO("Start feed: pid = 0x%x index = %d\n",
feed->pid, feed->index);
@@ -253,9 +257,6 @@ static int cx18_dvb_start_feed(struct dvb_demux_feed *feed)
if (!demux->dmx.frontend)
return -EINVAL;
- if (!stream)
- return -EINVAL;
-
mutex_lock(&stream->dvb.feedlock);
if (stream->dvb.feeding++ == 0) {
CX18_DEBUG_INFO("Starting Transport DMA\n");
@@ -279,13 +280,14 @@ static int cx18_dvb_stop_feed(struct dvb_demux_feed *feed)
{
struct dvb_demux *demux = feed->demux;
struct cx18_stream *stream = (struct cx18_stream *)demux->priv;
- struct cx18 *cx = stream->cx;
+ struct cx18 *cx;
int ret = -EINVAL;
- CX18_DEBUG_INFO("Stop feed: pid = 0x%x index = %d\n",
- feed->pid, feed->index);
-
if (stream) {
+ cx = stream->cx;
+ CX18_DEBUG_INFO("Stop feed: pid = 0x%x index = %d\n",
+ feed->pid, feed->index);
+
mutex_lock(&stream->dvb.feedlock);
if (--stream->dvb.feeding == 0) {
CX18_DEBUG_INFO("Stopping Transport DMA\n");
--
1.6.5.7
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] media: video/cx18, fix potential null dereference
2010-01-10 8:56 [PATCH 1/1] media: video/cx18, fix potential null dereference Jiri Slaby
@ 2010-01-11 23:48 ` Andy Walls
2010-01-12 11:28 ` Jiri Slaby
0 siblings, 1 reply; 6+ messages in thread
From: Andy Walls @ 2010-01-11 23:48 UTC (permalink / raw)
To: Jiri Slaby
Cc: mchehab, hverkuil, ivtv-devel, linux-media, linux-kernel,
jirislaby
On Sun, 2010-01-10 at 09:56 +0100, Jiri Slaby wrote:
> Stanse found a potential null dereference in cx18_dvb_start_feed
> and cx18_dvb_stop_feed. There is a check for stream being NULL,
> but it is dereferenced earlier. Move the dereference after the
> check.
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Reviewed-by: Andy Walls <awalls@radix.net>
Acked-by: Andy Walls <awalls@radix.net>
Regards,
Andy
> ---
> drivers/media/video/cx18/cx18-dvb.c | 18 ++++++++++--------
> 1 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/video/cx18/cx18-dvb.c b/drivers/media/video/cx18/cx18-dvb.c
> index 71ad2d1..0ad5b63 100644
> --- a/drivers/media/video/cx18/cx18-dvb.c
> +++ b/drivers/media/video/cx18/cx18-dvb.c
> @@ -213,10 +213,14 @@ static int cx18_dvb_start_feed(struct dvb_demux_feed *feed)
> {
> struct dvb_demux *demux = feed->demux;
> struct cx18_stream *stream = (struct cx18_stream *) demux->priv;
> - struct cx18 *cx = stream->cx;
> + struct cx18 *cx;
> int ret;
> u32 v;
>
> + if (!stream)
> + return -EINVAL;
> +
> + cx = stream->cx;
> CX18_DEBUG_INFO("Start feed: pid = 0x%x index = %d\n",
> feed->pid, feed->index);
>
> @@ -253,9 +257,6 @@ static int cx18_dvb_start_feed(struct dvb_demux_feed *feed)
> if (!demux->dmx.frontend)
> return -EINVAL;
>
> - if (!stream)
> - return -EINVAL;
> -
> mutex_lock(&stream->dvb.feedlock);
> if (stream->dvb.feeding++ == 0) {
> CX18_DEBUG_INFO("Starting Transport DMA\n");
> @@ -279,13 +280,14 @@ static int cx18_dvb_stop_feed(struct dvb_demux_feed *feed)
> {
> struct dvb_demux *demux = feed->demux;
> struct cx18_stream *stream = (struct cx18_stream *)demux->priv;
> - struct cx18 *cx = stream->cx;
> + struct cx18 *cx;
> int ret = -EINVAL;
>
> - CX18_DEBUG_INFO("Stop feed: pid = 0x%x index = %d\n",
> - feed->pid, feed->index);
> -
> if (stream) {
> + cx = stream->cx;
> + CX18_DEBUG_INFO("Stop feed: pid = 0x%x index = %d\n",
> + feed->pid, feed->index);
> +
> mutex_lock(&stream->dvb.feedlock);
> if (--stream->dvb.feeding == 0) {
> CX18_DEBUG_INFO("Stopping Transport DMA\n");
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] media: video/cx18, fix potential null dereference
2010-01-11 23:48 ` Andy Walls
@ 2010-01-12 11:28 ` Jiri Slaby
2010-01-13 11:32 ` Andy Walls
2010-01-13 11:43 ` Manu Abraham
0 siblings, 2 replies; 6+ messages in thread
From: Jiri Slaby @ 2010-01-12 11:28 UTC (permalink / raw)
To: Andy Walls; +Cc: mchehab, hverkuil, ivtv-devel, linux-media, linux-kernel
On 01/12/2010 12:48 AM, Andy Walls wrote:
> On Sun, 2010-01-10 at 09:56 +0100, Jiri Slaby wrote:
>> Stanse found a potential null dereference in cx18_dvb_start_feed
>> and cx18_dvb_stop_feed. There is a check for stream being NULL,
>> but it is dereferenced earlier. Move the dereference after the
>> check.
>>
>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>
> Reviewed-by: Andy Walls <awalls@radix.net>
> Acked-by: Andy Walls <awalls@radix.net>
You definitely know the code better, have you checked that it can happen
at all? I mean may demux->priv be NULL?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] media: video/cx18, fix potential null dereference
2010-01-12 11:28 ` Jiri Slaby
@ 2010-01-13 11:32 ` Andy Walls
2010-01-13 11:35 ` Jiri Slaby
2010-01-13 11:43 ` Manu Abraham
1 sibling, 1 reply; 6+ messages in thread
From: Andy Walls @ 2010-01-13 11:32 UTC (permalink / raw)
To: Jiri Slaby; +Cc: mchehab, hverkuil, ivtv-devel, linux-media, linux-kernel
On Tue, 2010-01-12 at 12:28 +0100, Jiri Slaby wrote:
> On 01/12/2010 12:48 AM, Andy Walls wrote:
> > On Sun, 2010-01-10 at 09:56 +0100, Jiri Slaby wrote:
> >> Stanse found a potential null dereference in cx18_dvb_start_feed
> >> and cx18_dvb_stop_feed. There is a check for stream being NULL,
> >> but it is dereferenced earlier. Move the dereference after the
> >> check.
> >>
> >> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> >
> > Reviewed-by: Andy Walls <awalls@radix.net>
> > Acked-by: Andy Walls <awalls@radix.net>
>
> You definitely know the code better, have you checked that it can happen
> at all? I mean may demux->priv be NULL?
I'm wasn't sure, and that's the one reason I didn't NAK the patch.
I can tell you no one has ever reported an Ooops or Bug due to that
condition.
I know the cx18 code very well. However, I am less familiar with the
dvb core code and any bad behavior that may exist there. When relying
on data structures the dvb core accesses I would have to research what
could happen in the dvb core to possibly generate that condition.
Since I'm busy this week with work related to my day job (nothing to do
with Linux), it was easiest to let the NULL check stay in for now.
If you don't mind a delay of until Sunday or so to get the patch applied
to the V4L-DVB tree, I can take the patch and work it in my normal path
through Mauro. Let me know.
Regards,
Andy
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] media: video/cx18, fix potential null dereference
2010-01-12 11:28 ` Jiri Slaby
2010-01-13 11:32 ` Andy Walls
@ 2010-01-13 11:43 ` Manu Abraham
1 sibling, 0 replies; 6+ messages in thread
From: Manu Abraham @ 2010-01-13 11:43 UTC (permalink / raw)
To: Jiri Slaby
Cc: Andy Walls, mchehab, hverkuil, ivtv-devel, linux-media,
linux-kernel
On Tue, Jan 12, 2010 at 3:28 PM, Jiri Slaby <jirislaby@gmail.com> wrote:
> On 01/12/2010 12:48 AM, Andy Walls wrote:
>> On Sun, 2010-01-10 at 09:56 +0100, Jiri Slaby wrote:
>>> Stanse found a potential null dereference in cx18_dvb_start_feed
>>> and cx18_dvb_stop_feed. There is a check for stream being NULL,
>>> but it is dereferenced earlier. Move the dereference after the
>>> check.
>>>
>>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>>
>> Reviewed-by: Andy Walls <awalls@radix.net>
>> Acked-by: Andy Walls <awalls@radix.net>
>
> You definitely know the code better, have you checked that it can happen
> at all? I mean may demux->priv be NULL?
It is highly unlikely that demux->priv becoming NULL. The only time
that could happen would be when there is a dvb register failed. But in
which case, it wouldn't reach the stage where you want to start/stop a
stream.
Regards,
Manu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-01-13 11:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-10 8:56 [PATCH 1/1] media: video/cx18, fix potential null dereference Jiri Slaby
2010-01-11 23:48 ` Andy Walls
2010-01-12 11:28 ` Jiri Slaby
2010-01-13 11:32 ` Andy Walls
2010-01-13 11:35 ` Jiri Slaby
2010-01-13 11:43 ` Manu Abraham
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox