diff mbox series

[FFmpeg-devel,v3,33/34] avdevice/dshow: prevent NULL access

Message ID 20210706092020.1057-34-dcnieho@gmail.com
State New
Headers show
Series avdevice (mostly dshow) enhancements
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Diederick C. Niehorster July 6, 2021, 9:20 a.m. UTC
list_options true would crash when both a video and an audio device were
specified as input. Crash would occur on line 1588 (in this new rev)
because ctx->device_unique_name[otherDevType] would be NULL

Signed-off-by: Diederick Niehorster <dcnieho@gmail.com>
---
 libavdevice/dshow.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Andreas Rheinhardt Aug. 3, 2021, 2:22 p.m. UTC | #1
Diederick Niehorster:
> list_options true would crash when both a video and an audio device were
> specified as input. Crash would occur on line 1588 (in this new rev)
> because ctx->device_unique_name[otherDevType] would be NULL
> 
> Signed-off-by: Diederick Niehorster <dcnieho@gmail.com>
> ---
>  libavdevice/dshow.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index f900e89988..a66f7b81fd 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -1519,9 +1519,9 @@ dshow_list_device_options(AVFormatContext *avctx, ICreateDevEnum *devenum,
>      if ((r = dshow_cycle_devices(avctx, devenum, devtype, sourcetype, &device_filter, &device_unique_name, NULL)) < 0)
>          return r;
>      ctx->device_filter[devtype] = device_filter;
> +    ctx->device_unique_name[devtype] = device_unique_name;
>      if ((r = dshow_cycle_pins(avctx, devtype, sourcetype, device_filter, ranges ? &device_pin : NULL, ranges, query_type)) < 0)
>          return r;
> -    av_freep(&device_unique_name);
>      return 0;
>  }
>  
> @@ -2043,6 +2043,7 @@ static int dshow_read_header(AVFormatContext *avctx)
>                  }
>              }
>          }
> +        // don't exit yet, allow it to list crossbar options in dshow_open_device
>      }
>      ctx->is_running = 0;
>      if (ctx->device_name[VideoDevice]) {
> 
Is this an issue that can also happen on current git master? If so, then
it should be the first of this whole series, so that it can be
backported to the still supported release branches. If not, then you
should incorporate this into the patch that introduces the crash in
order to make sure that it never exists (we do not knowingly commit bugs).

- Andreas
Diederick C. Niehorster Aug. 5, 2021, 10:17 a.m. UTC | #2
On Tue, Aug 3, 2021 at 4:22 PM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Diederick Niehorster:
> > list_options true would crash when both a video and an audio device were
> > specified as input. Crash would occur on line 1588 (in this new rev)
> > because ctx->device_unique_name[otherDevType] would be NULL
> >
> > Signed-off-by: Diederick Niehorster <dcnieho@gmail.com>
> > ---
> >  libavdevice/dshow.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> > index f900e89988..a66f7b81fd 100644
> > --- a/libavdevice/dshow.c
> > +++ b/libavdevice/dshow.c
> > @@ -1519,9 +1519,9 @@ dshow_list_device_options(AVFormatContext *avctx, ICreateDevEnum *devenum,
> >      if ((r = dshow_cycle_devices(avctx, devenum, devtype, sourcetype, &device_filter, &device_unique_name, NULL)) < 0)
> >          return r;
> >      ctx->device_filter[devtype] = device_filter;
> > +    ctx->device_unique_name[devtype] = device_unique_name;
> >      if ((r = dshow_cycle_pins(avctx, devtype, sourcetype, device_filter, ranges ? &device_pin : NULL, ranges, query_type)) < 0)
> >          return r;
> > -    av_freep(&device_unique_name);
> >      return 0;
> >  }
> >
> > @@ -2043,6 +2043,7 @@ static int dshow_read_header(AVFormatContext *avctx)
> >                  }
> >              }
> >          }
> > +        // don't exit yet, allow it to list crossbar options in dshow_open_device
> >      }
> >      ctx->is_running = 0;
> >      if (ctx->device_name[VideoDevice]) {
> >
> Is this an issue that can also happen on current git master? If so, then
> it should be the first of this whole series, so that it can be
> backported to the still supported release branches. If not, then you
> should incorporate this into the patch that introduces the crash in
> order to make sure that it never exists (we do not knowingly commit bugs).

Thanks for the comment! Yes, the crash it fixes also occurs with
current master, I'll make it the first patch in the series.

/Dee
diff mbox series

Patch

diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
index f900e89988..a66f7b81fd 100644
--- a/libavdevice/dshow.c
+++ b/libavdevice/dshow.c
@@ -1519,9 +1519,9 @@  dshow_list_device_options(AVFormatContext *avctx, ICreateDevEnum *devenum,
     if ((r = dshow_cycle_devices(avctx, devenum, devtype, sourcetype, &device_filter, &device_unique_name, NULL)) < 0)
         return r;
     ctx->device_filter[devtype] = device_filter;
+    ctx->device_unique_name[devtype] = device_unique_name;
     if ((r = dshow_cycle_pins(avctx, devtype, sourcetype, device_filter, ranges ? &device_pin : NULL, ranges, query_type)) < 0)
         return r;
-    av_freep(&device_unique_name);
     return 0;
 }
 
@@ -2043,6 +2043,7 @@  static int dshow_read_header(AVFormatContext *avctx)
                 }
             }
         }
+        // don't exit yet, allow it to list crossbar options in dshow_open_device
     }
     ctx->is_running = 0;
     if (ctx->device_name[VideoDevice]) {