New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 698524 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug-Regression

Blocking:
issue 777555



Sign in to add a comment

Crash in av_parser_parse2

Project Member Reported by ClusterFuzz, Mar 4 2017

Issue description

Components: Blink>HTML>Parser
Labels: -Type-Bug M-59 Type-Bug-Regression
Owner: jzern@chromium.org
Status: Assigned (was: Untriaged)
jzern@: Based on changes made to the crashed file "utils.c" made by you assigning to you. Could you please take a look into this.
This is impacting to head.
Labels: Test-Predator-Wrong-CLs

Comment 3 by jzern@chromium.org, Mar 6 2017

Cc: jzern@chromium.org
Owner: wolenetz@chromium.org
I haven't made changes to this file in quite some time. It's possible a recent roll has caused the failure. Given the stacktrace it's not clear if the abort is occurring in the parse function pointer or the line afterward. Are we building with av_assert() enabled for these runs?
Reassigning to wolenetz@ who did the most recent roll.
Components: -Blink>HTML>Parser Internals>Media>FFmpeg
Cc: wolenetz@chromium.org
Owner: tguilbert@chromium.org
=>tguilbert as part of new ffmpeg roll.
Blocking: 698865
From the comment on av_assert0(), it seems like av_assert0() is always on (https://cs.chromium.org/chromium/src/third_party/ffmpeg/libavutil/avassert.h?l=37).

I think the stack trace is failing on the assert below.
Both ffplay and a local asan build of the media_pipeline_integration_fuzzer result in an "Aborted (core dumped)" termination.

Should we try to catch the error earlier and never end up in this situation, or change the fuzzer/test to accept that the abrupt termination is expected?
We don't want the renderer process to crash on such an assertion occurring while attempting media playback. Rather, we'd prefer a graceful error indication (like PIPELINE_ERROR_DECODE or somesuch) to result. IOW, the assert is protecting an assumption (most likely) which isn't being adhered to well enough. Can ffmpeg instead catch this error and return an error rather than die?
Ugh it looks like ffmpeg upstream knows this (https://cs.chromium.org/chromium/src/third_party/ffmpeg/libavcodec/parser.c?sq=package:chromium&type=cs&l=184) and uses the assert rather than fix their API. Perhaps a chat with upstream might help find a better solution?
Blocking: -698865 777555
Cc: tguilbert@chromium.org
Labels: -M-59 M-64
Owner: dalecur...@chromium.org
Cc: mummare...@chromium.org msrchandra@chromium.org rodger.c...@gmail.com
 Issue 705323  has been merged into this issue.
For more information, please see https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md.

The link referenced in the description is no longer valid.

(bulk edit)
Labels: -Pri-1 Pri-2
Status: ExternalDependency (was: Assigned)
Since this is effectively just a CHECK failure, it's not Pri-1, I've filed an upstream ticket https://trac.ffmpeg.org/ticket/6804 for them to solve this.
Project Member

Comment 14 by ClusterFuzz, Nov 14 2017

Status: WontFix (was: ExternalDependency)
ClusterFuzz testcase 4580570865860608 is flaky and no longer crashes, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Possibly fixed by the ffmpeg roll to some extent that it no longer repros.
Cc: kkaluri@chromium.org
 Issue 785798  has been merged into this issue.
Labels: ClusterFuzz-Wrong
Status: Assigned (was: WontFix)
Project Member

Comment 18 by bugdroid1@chromium.org, Nov 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aea3d2d4d8d304df1a029ef83d248508073bd066

commit aea3d2d4d8d304df1a029ef83d248508073bd066
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Sat Nov 18 06:56:26 2017

Roll ffmpeg DEPS and fix additional ubsan issues.

This change enables AV_EF_EXPLODE such that all serious errors
encountered during demuxing are fatal. Previously ffmpeg would
try to ignore these in some cases; leading to ubsan or other
issues. Specifically  crbug.com/698524  and  crbug.com/710791 .

Due to the removal of the speex parser from ogg, there is one
test that needs updating with the roll too.

https://chromium.googlesource.com/chromium/third_party/ffmpeg.git/+log/1e816bccb5ff..252244150ad7

$ git log 1e816bccb..252244150 --date=short --no-merges --format='%ad %ae %s'
2017-11-17 dalecurtis [mpeg4video] Fix undefined shift on assumed 8-bit input.
2017-11-17 dalecurtis Disable unused ogg codec parsers; they have bugs we don't care about.
2017-11-17 dalecurtis Use ff_thread_once for fixed, float table init.
2017-11-17 dalecurtis Fixup some patches messages.
2017-11-17 dalecurtis [mov] Fix leak of frame_duration_buffer in mov_fix_index().
2017-11-17 dalecurtis Prevent undefined shift with wrap_bits >= 63.
2017-11-15 hubbe avformat/mov: Check size of STSC allocation
2017-11-17 jstebbins [PATCH] lavf/mov: don't read outside frag_index bounds

Created with:
  roll-dep src/third_party/ffmpeg

BUG= 786269 , 782074 , 783459 , 784159 , 654612 , 779924 , 710791 , 698524 
TEST=security test cases no longer fail.

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ibbf3c32080705d6484682351a351663c51a7f752
Reviewed-on: https://chromium-review.googlesource.com/777408
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517709}
[modify] https://crrev.com/aea3d2d4d8d304df1a029ef83d248508073bd066/DEPS
[modify] https://crrev.com/aea3d2d4d8d304df1a029ef83d248508073bd066/media/filters/ffmpeg_demuxer_unittest.cc
[modify] https://crrev.com/aea3d2d4d8d304df1a029ef83d248508073bd066/media/filters/ffmpeg_glue.cc

Status: Fixed (was: Assigned)
Should be fixed, CF shows this as non-reproducible, but duped bugs are marked as fixed.

Sign in to add a comment