diff mbox

[FFmpeg-devel,09/12] avformat/mxfdec: add support for clip wrapped essences

Message ID alpine.LSU.2.20.1806122353560.19807@iq
State Superseded
Headers show

Commit Message

Marton Balint June 12, 2018, 9:59 p.m. UTC
On Tue, 12 Jun 2018, Michael Niedermayer wrote:

> On Tue, Jun 12, 2018 at 10:47:24AM +0200, Marton Balint wrote:
>>
>>
>> On Tue, 12 Jun 2018, Michael Niedermayer wrote:
>>
>>> On Sun, Jun 10, 2018 at 12:36:47PM +0200, Marton Balint wrote:
>>>> Also use common code with opAtom.
>>>>
>>>> Fixes ticket #2776.
>>>> Partially fixes ticket #5671.
>>>> Fixes ticket #5866.
>>>>
>>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>>> ---
>>>> libavformat/mxfdec.c | 281 ++++++++++++++++++++++++---------------------------
>>>> 1 file changed, 130 insertions(+), 151 deletions(-)
>>>
>>> causes a segfault:
>>>
>>> ==23735== Invalid read of size 8
>>> ==23735==    at 0x75A627: mxf_set_pts (mxfdec.c:3277)
>>> ==23735==    by 0x75ACAD: mxf_read_packet_old (mxfdec.c:3396)
>>> ==23735==    by 0x7E099D: ff_read_packet (utils.c:856)
>>> ==23735==    by 0x7E39FF: read_frame_internal (utils.c:1581)
>>> ==23735==    by 0x7EB82B: avformat_find_stream_info (utils.c:3773)
>>> ==23735==    by 0x415534: open_input_file (ffmpeg_opt.c:1091)
>>> ==23735==    by 0x41EB11: open_files (ffmpeg_opt.c:3206)
>>> ==23735==    by 0x41ECA3: ffmpeg_parse_options (ffmpeg_opt.c:3246)
>>> ==23735==    by 0x43D1A3: main (ffmpeg.c:4832)
>>> ==23735==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
>>> ==23735==
>>> ==23735==
>>> ==23735== Process terminating with default action of signal 11 (SIGSEGV)
>>> ==23735==  Access not within mapped region at address 0x0
>>> ==23735==    at 0x75A627: mxf_set_pts (mxfdec.c:3277)
>>> ==23735==    by 0x75ACAD: mxf_read_packet_old (mxfdec.c:3396)
>>> ==23735==    by 0x7E099D: ff_read_packet (utils.c:856)
>>> ==23735==    by 0x7E39FF: read_frame_internal (utils.c:1581)
>>> ==23735==    by 0x7EB82B: avformat_find_stream_info (utils.c:3773)
>>> ==23735==    by 0x415534: open_input_file (ffmpeg_opt.c:1091)
>>> ==23735==    by 0x41EB11: open_files (ffmpeg_opt.c:3206)
>>> ==23735==    by 0x41ECA3: ffmpeg_parse_options (ffmpeg_opt.c:3246)
>>> ==23735==    by 0x43D1A3: main (ffmpeg.c:4832)
>>> ==23735==  If you believe this happened as a result of a stack
>>> ==23735==  overflow in your program's main thread (unlikely but
>>> ==23735==  possible), you can try to increase the size of the
>>> ==23735==  main thread stack using the --main-stacksize= flag.
>>> ==23735==  The main thread stack size used in this run was 8388608.
>>
>> I don't see this. What is your command line?
>
> testcase sent privatly

index_table->nb_ptses was negative, but that did not cause problems before 
the patch, because the comparison to nb_ptses was signed, after the patch 
it became unsigned.

It is better to explicitly disallow a negative nb_ptses, please apply the 
attached patch before this one, you should be good.

Thanks,
Marton

Comments

Michael Niedermayer June 13, 2018, 2:30 p.m. UTC | #1
On Tue, Jun 12, 2018 at 11:59:26PM +0200, Marton Balint wrote:
> 
> 
> On Tue, 12 Jun 2018, Michael Niedermayer wrote:
> 
> >On Tue, Jun 12, 2018 at 10:47:24AM +0200, Marton Balint wrote:
> >>
> >>
> >>On Tue, 12 Jun 2018, Michael Niedermayer wrote:
> >>
> >>>On Sun, Jun 10, 2018 at 12:36:47PM +0200, Marton Balint wrote:
> >>>>Also use common code with opAtom.
> >>>>
> >>>>Fixes ticket #2776.
> >>>>Partially fixes ticket #5671.
> >>>>Fixes ticket #5866.
> >>>>
> >>>>Signed-off-by: Marton Balint <cus@passwd.hu>
> >>>>---
> >>>>libavformat/mxfdec.c | 281 ++++++++++++++++++++++++---------------------------
> >>>>1 file changed, 130 insertions(+), 151 deletions(-)
> >>>
> >>>causes a segfault:
> >>>
> >>>==23735== Invalid read of size 8
> >>>==23735==    at 0x75A627: mxf_set_pts (mxfdec.c:3277)
> >>>==23735==    by 0x75ACAD: mxf_read_packet_old (mxfdec.c:3396)
> >>>==23735==    by 0x7E099D: ff_read_packet (utils.c:856)
> >>>==23735==    by 0x7E39FF: read_frame_internal (utils.c:1581)
> >>>==23735==    by 0x7EB82B: avformat_find_stream_info (utils.c:3773)
> >>>==23735==    by 0x415534: open_input_file (ffmpeg_opt.c:1091)
> >>>==23735==    by 0x41EB11: open_files (ffmpeg_opt.c:3206)
> >>>==23735==    by 0x41ECA3: ffmpeg_parse_options (ffmpeg_opt.c:3246)
> >>>==23735==    by 0x43D1A3: main (ffmpeg.c:4832)
> >>>==23735==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
> >>>==23735==
> >>>==23735==
> >>>==23735== Process terminating with default action of signal 11 (SIGSEGV)
> >>>==23735==  Access not within mapped region at address 0x0
> >>>==23735==    at 0x75A627: mxf_set_pts (mxfdec.c:3277)
> >>>==23735==    by 0x75ACAD: mxf_read_packet_old (mxfdec.c:3396)
> >>>==23735==    by 0x7E099D: ff_read_packet (utils.c:856)
> >>>==23735==    by 0x7E39FF: read_frame_internal (utils.c:1581)
> >>>==23735==    by 0x7EB82B: avformat_find_stream_info (utils.c:3773)
> >>>==23735==    by 0x415534: open_input_file (ffmpeg_opt.c:1091)
> >>>==23735==    by 0x41EB11: open_files (ffmpeg_opt.c:3206)
> >>>==23735==    by 0x41ECA3: ffmpeg_parse_options (ffmpeg_opt.c:3246)
> >>>==23735==    by 0x43D1A3: main (ffmpeg.c:4832)
> >>>==23735==  If you believe this happened as a result of a stack
> >>>==23735==  overflow in your program's main thread (unlikely but
> >>>==23735==  possible), you can try to increase the size of the
> >>>==23735==  main thread stack using the --main-stacksize= flag.
> >>>==23735==  The main thread stack size used in this run was 8388608.
> >>
> >>I don't see this. What is your command line?
> >
> >testcase sent privatly
> 
> index_table->nb_ptses was negative, but that did not cause problems before
> the patch, because the comparison to nb_ptses was signed, after the patch it
> became unsigned.
> 
> It is better to explicitly disallow a negative nb_ptses, please apply the
> attached patch before this one, you should be good.

confirmed, seems to fix it

thanks

[...]
diff mbox

Patch

From c3f9e8442fe66f474466d769f44f782532eacb82 Mon Sep 17 00:00:00 2001
From: Marton Balint <cus@passwd.hu>
Date: Tue, 12 Jun 2018 23:42:16 +0200
Subject: [PATCH] avformat/mxfdec: avoid index_table->nb_ptses overflow in
 mxf_compute_ptses_fake_index

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mxfdec.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 32ca9e0f99..b2930087ab 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -1528,6 +1528,12 @@  static int mxf_compute_ptses_fake_index(MXFContext *mxf, MXFIndexTable *index_ta
             return 0;                               /* no TemporalOffsets */
         }
 
+        if (s->index_duration > INT_MAX - index_table->nb_ptses) {
+            index_table->nb_ptses = 0;
+            av_log(mxf->fc, AV_LOG_ERROR, "ignoring IndexSID %d, duration is too large\n", s->index_sid);
+            return 0;
+        }
+
         index_table->nb_ptses += s->index_duration;
     }
 
-- 
2.16.4