| Message ID | 20200613112345.13515-5-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avcodec/iff: Fix off by x error | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Jun 13, 2020, 12:23 by michael@niedermayer.cc: > The code crashes and neither the authors nor anyone else did fix this > We cannot release code which crashes, so if noone fixes it, the only > option left is to disable or revert. Revert is difficult as there are > multiple commits afterwards > You're suggesting a revert when disabling it is so simple? As if the commits didn't fix an issue users have been begging for for years? Do you want a permanently dead project where no code changes because it may crash or break things? You should fork instead, we'd all be better off then. I even asked you on IRC whether your previous 2 commits fixed the crash, so I know whether to try fixing it, and you didn't even respond. You didn't even ping me anytime to check up. And yet you claim I've abandoned it? As if I don't have a million things to work on or worry about already.
On 6/13/20, Michael Niedermayer <michael@niedermayer.cc> wrote: > The code crashes and neither the authors nor anyone else did fix this > We cannot release code which crashes, so if noone fixes it, the only > option left is to disable or revert. Revert is difficult as there are > multiple commits afterwards > > Fixes: > 22082/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5688619118624768 > Fixes: crash from V-codecs/Theora/theora_testsuite_broken/multi2.ogg > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/oggdec.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c > index 9eb45499c6..1f3ed8024c 100644 > --- a/libavformat/oggdec.c > +++ b/libavformat/oggdec.c > @@ -229,6 +229,15 @@ static int ogg_replace_stream(AVFormatContext *s, > uint32_t serial, char *magic, > /* We only have a single stream anyway, so if there's a new stream with > * a different codec just replace it */ > os = &ogg->streams[0]; > + if (os->codec != codec) { > + /* > + * The codec change code from > 8296443a70f052a6f5c9a867d28b83a5eb7d304d and surounding commits > + * crashes with out of array accesses > + * testcase is > https://samples.ffmpeg.org/V-codecs/Theora/theora_testsuite_broken/multi2.ogg > + */ > + return AVERROR_PATCHWELCOME; > + } > + > os->serial = serial; > os->codec = codec; > os->serial = serial; > -- > 2.17.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". NAK It very aggressive and also pointless.
On 6/13/2020 9:12 AM, Lynne wrote: > Jun 13, 2020, 12:23 by michael@niedermayer.cc: > >> The code crashes and neither the authors nor anyone else did fix this >> We cannot release code which crashes, so if noone fixes it, the only >> option left is to disable or revert. Revert is difficult as there are >> multiple commits afterwards >> > > You're suggesting a revert when disabling it is so simple? > As if the commits didn't fix an issue users have been begging for for years? > Do you want a permanently dead project where no code changes because it may crash > or break things? You should fork instead, we'd all be better off then. It would be very helpful too if you could stop being so goddamn aggressive with every subject. > > I even asked you on IRC whether your previous 2 commits fixed the crash, so I know whether > to try fixing it, and you didn't even respond. You didn't even ping me anytime to check up. > And yet you claim I've abandoned it? As if I don't have a million things to work on or worry > about already. This patch was a good opportunity to ping him about your question that went unanswered on IRC, while also NAKing the patch itself, instead of the above.
On Sat, Jun 13, 2020 at 02:12:55PM +0200, Lynne wrote: > Jun 13, 2020, 12:23 by michael@niedermayer.cc: > > > The code crashes and neither the authors nor anyone else did fix this > > We cannot release code which crashes, so if noone fixes it, the only > > option left is to disable or revert. Revert is difficult as there are > > multiple commits afterwards > > > > You're suggesting a revert when disabling it is so simple? > As if the commits didn't fix an issue users have been begging for for years? > Do you want a permanently dead project where no code changes because it may crash > or break things? You should fork instead, we'd all be better off then. > > I even asked you on IRC whether your previous 2 commits fixed the crash, so I know whether > to try fixing it, and you didn't even respond. You didn't even ping me anytime to check up. > And yet you claim I've abandoned it? As if I don't have a million things to work on or worry > about already. I missed your questions on IRC, in fact ive been a little sick in the last few days, nothing serious, just some migraine and mild fever after a dentist. But that probably kept me from looking at IRC for too long at the wrong time sorry [...]
On Sat, Jun 13, 2020, at 16:18, James Almer wrote: > On 6/13/2020 9:12 AM, Lynne wrote: > > Jun 13, 2020, 12:23 by michael@niedermayer.cc: > > I even asked you on IRC whether your previous 2 commits fixed the crash, so I know whether > > to try fixing it, and you didn't even respond. You didn't even ping me anytime to check up. > > And yet you claim I've abandoned it? As if I don't have a million things to work on or worry > > about already. > > This patch was a good opportunity to ping him about your question that > went unanswered on IRC, while also NAKing the patch itself, instead of > the above. Can we have a fix without the drama, people? Come on, there is nothing unfixable here.
diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c index 9eb45499c6..1f3ed8024c 100644 --- a/libavformat/oggdec.c +++ b/libavformat/oggdec.c @@ -229,6 +229,15 @@ static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, /* We only have a single stream anyway, so if there's a new stream with * a different codec just replace it */ os = &ogg->streams[0]; + if (os->codec != codec) { + /* + * The codec change code from 8296443a70f052a6f5c9a867d28b83a5eb7d304d and surounding commits + * crashes with out of array accesses + * testcase is https://samples.ffmpeg.org/V-codecs/Theora/theora_testsuite_broken/multi2.ogg + */ + return AVERROR_PATCHWELCOME; + } + os->serial = serial; os->codec = codec; os->serial = serial;
The code crashes and neither the authors nor anyone else did fix this We cannot release code which crashes, so if noone fixes it, the only option left is to disable or revert. Revert is difficult as there are multiple commits afterwards Fixes: 22082/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5688619118624768 Fixes: crash from V-codecs/Theora/theora_testsuite_broken/multi2.ogg Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/oggdec.c | 9 +++++++++ 1 file changed, 9 insertions(+)