| Message ID | CAF1j9YNW370Fi6VkhTVNBcR+Lst-B62W6eRpuCa9-U28fNMMzQ@mail.gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Feb 15, 2018 at 12:10:33PM -0800, Xiaohan Wang (王消寒) wrote: > > mov.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > 5597d0b095f8b15eb11503010a51c2bc2c022413 0001-ffmpeg-Fix-memset-size-on-ctts_data-in-mov_read_trun.patch > From 7c1e6b50ebe35b2a38c4f1d0a988e31eccbd0ead Mon Sep 17 00:00:00 2001 > From: Xiaohan Wang <xhwang@chromium.org> > Date: Thu, 15 Feb 2018 12:05:53 -0800 > Subject: [PATCH] ffmpeg: Fix memset size on ctts_data in mov_read_trun() > > The allocated size of sc->ctts_data is > (st->nb_index_entries + entries) * sizeof(*sc->ctts_data). > > The size to memset at offset sc->ctts_data + sc->ctts_count should be > (st->nb_index_entries + entries - sc->ctts_count) * sizeof(*sc->ctts_data)) > > The current code missed |entries| I believe. shouldnt "entries" be read by this function later and so shouldnt need a memset? I didnt write this, but it looks a bit to me as if it was intended to only clear the area that would not be read later [...]
+jstebbins@ who wrote that code. On Fri, Feb 16, 2018 at 12:30 PM, Michael Niedermayer < michael@niedermayer.cc> wrote: > On Thu, Feb 15, 2018 at 12:10:33PM -0800, Xiaohan Wang (王消寒) wrote: > > > > > mov.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > 5597d0b095f8b15eb11503010a51c2bc2c022413 0001-ffmpeg-Fix-memset-size- > on-ctts_data-in-mov_read_trun.patch > > From 7c1e6b50ebe35b2a38c4f1d0a988e31eccbd0ead Mon Sep 17 00:00:00 2001 > > From: Xiaohan Wang <xhwang@chromium.org> > > Date: Thu, 15 Feb 2018 12:05:53 -0800 > > Subject: [PATCH] ffmpeg: Fix memset size on ctts_data in mov_read_trun() > > > > The allocated size of sc->ctts_data is > > (st->nb_index_entries + entries) * sizeof(*sc->ctts_data). > > > > The size to memset at offset sc->ctts_data + sc->ctts_count should be > > (st->nb_index_entries + entries - sc->ctts_count) * > sizeof(*sc->ctts_data)) > > > > The current code missed |entries| I believe. > > shouldnt "entries" be read by this function later and so shouldnt need a > memset? > I didnt write this, but it looks a bit to me as if it was intended to only > clear the area that would not be read later > I thought we only had sc->ctts_count entries before av_fast_realloc, so memset everything starting from sc->ctts_data + sc->ctts_count couldn't go wrong. But I am not familiar with this code and that could totally be wrong. I added jstebbins@ who wrote the code and hopefully we can get expert opinion there. > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > No great genius has ever existed without some touch of madness. -- > Aristotle > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >
jstebbins: kindly ping! On Fri, Feb 16, 2018 at 2:42 PM, Xiaohan Wang (王消寒) <xhwang@chromium.org> wrote: > +jstebbins@ who wrote that code. > > On Fri, Feb 16, 2018 at 12:30 PM, Michael Niedermayer < > michael@niedermayer.cc> wrote: > >> On Thu, Feb 15, 2018 at 12:10:33PM -0800, Xiaohan Wang (王消寒) wrote: >> > >> >> > mov.c | 3 ++- >> > 1 file changed, 2 insertions(+), 1 deletion(-) >> > 5597d0b095f8b15eb11503010a51c2bc2c022413 >> 0001-ffmpeg-Fix-memset-size-on-ctts_data-in-mov_read_trun.patch >> > From 7c1e6b50ebe35b2a38c4f1d0a988e31eccbd0ead Mon Sep 17 00:00:00 2001 >> > From: Xiaohan Wang <xhwang@chromium.org> >> > Date: Thu, 15 Feb 2018 12:05:53 -0800 >> > Subject: [PATCH] ffmpeg: Fix memset size on ctts_data in mov_read_trun() >> > >> > The allocated size of sc->ctts_data is >> > (st->nb_index_entries + entries) * sizeof(*sc->ctts_data). >> > >> > The size to memset at offset sc->ctts_data + sc->ctts_count should be >> > (st->nb_index_entries + entries - sc->ctts_count) * >> sizeof(*sc->ctts_data)) >> > >> > The current code missed |entries| I believe. >> >> shouldnt "entries" be read by this function later and so shouldnt need a >> memset? >> I didnt write this, but it looks a bit to me as if it was intended to only >> clear the area that would not be read later >> > > I thought we only had sc->ctts_count entries before av_fast_realloc, so > memset everything starting from sc->ctts_data + sc->ctts_count couldn't > go wrong. But I am not familiar with this code and that could totally be > wrong. I added jstebbins@ who wrote the code and hopefully we can get > expert opinion there. > > >> [...] >> -- >> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB >> >> No great genius has ever existed without some touch of madness. -- >> Aristotle >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> >
From 7c1e6b50ebe35b2a38c4f1d0a988e31eccbd0ead Mon Sep 17 00:00:00 2001 From: Xiaohan Wang <xhwang@chromium.org> Date: Thu, 15 Feb 2018 12:05:53 -0800 Subject: [PATCH] ffmpeg: Fix memset size on ctts_data in mov_read_trun() The allocated size of sc->ctts_data is (st->nb_index_entries + entries) * sizeof(*sc->ctts_data). The size to memset at offset sc->ctts_data + sc->ctts_count should be (st->nb_index_entries + entries - sc->ctts_count) * sizeof(*sc->ctts_data)) The current code missed |entries| I believe. BUG=812567 --- libavformat/mov.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index a3725692a7..6407d60050 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -4723,7 +4723,8 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom) // zero valued entries. This ensures clips which mix boxes with and // without ctts entries don't pickup uninitialized data. memset(sc->ctts_data + sc->ctts_count, 0, - (st->nb_index_entries - sc->ctts_count) * sizeof(*sc->ctts_data)); + (st->nb_index_entries + entries - sc->ctts_count) * + sizeof(*sc->ctts_data)); if (index_entry_pos < st->nb_index_entries) { // Make hole in index_entries and ctts_data for new samples -- 2.16.1.291.g4437f3f132-goog