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

Issue 868695 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jul 31
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug

Blocking:
issue 868463



Sign in to add a comment

Abrt in media::VideoFrame::CreateFrameWithLayout

Project Member Reported by ClusterFuzz, Jul 29

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=6378302265360384

Fuzzer: libFuzzer_mediasource_MP4_AV1_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: Abrt
Crash Address: 0x053900003b92
Crash State:
  media::VideoFrame::CreateFrameWithLayout
  media::VideoFrame::CreateFrameInternal
  media::VideoFrame::CreateZeroInitializedFrame
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=578802:578803

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6378302265360384

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.
 
Project Member

Comment 1 by ClusterFuzz, Jul 29

Components: Internals>Media
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 2 by ClusterFuzz, Jul 29

Labels: Test-Predator-Auto-Owner
Owner: johannko...@google.com
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/f2813ce352ac65a2d4259ae7aba9fa739d3903aa (enable av1 playback by default).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
Blocking: 868463
Cc: urvang@chromium.org huisu@google.com dalecur...@chromium.org wtc@google.com
Cc: johannko...@google.com
Owner: dalecur...@chromium.org
Dale, do you know if this is in the lib or the wrapper?
Owner: johannkoenig@chromium.org
This is the lib giving us bad values:

[0730/001026.872527:FATAL:video_frame.cc(1164)] CreateFrameWithLayout Invalid config.format:PIXEL_FORMAT_I420 storage_type:OWNED_MEMORY coded_size:320x240 visible_rect:0,0 320x240 natural_size:320x76800

Visible Rect can't be (0,0) and we get those from (aom_image->d_w, aom_image->d_h)
Cc: -urvang@chromium.org
Owner: urvang@chromium.org
Thanks!
Oh and natural_size exceeds 32767 in one dimension; that's on the media/ side.

Keep in mind this is a DCHECK and is properly handled in release builds. So if you believe libaom is doing the right thing here, we can consider converting this from LOG(DFATAL) to just (D)LOG(ERROR)
libaom's cmake options specify "-DCONFIG_SIZE_LIMIT=1 -DDECODE_HEIGHT_LIMIT=16384 -DDECODE_WIDTH_LIMIT=16384", so this is an invalid file for sure (given one dim is 76800 in "natural_size").

Can you explain what exactly is "natural_size", btw? (And where do you get that from in libaom?)

My guess is that some path is detecting that some path is detecting invalid dimension, and is therefore not setting any values inside d_w & d_h. But, I'll try to repro this locally to confirm.
Cc: urvang@chromium.org
Owner: dalecur...@chromium.org
Reproed this locally.

@Dale: The visible rect is actually x = 0, y = 0, width = 320 and height = 240. So, that seems valid.

Here's the flow if I step through with gdb:

(1) AomVideoDecoder::Initialize() is called with a config that has natural_size = [1,240], while coded size is [320,240].

Is this even a valid initialization?


Full config_.AsHumanReadableString() is:
codec: av1 format: 1 profile: av1 profile main coded size: [320,240] visible rect: [0,0,320,240] natural
size: [1,240] has extra data? false encryption scheme: Unencrypted rotation: 0°

(2) Now, Inside AomVideoDecoder::CopyImageToVideoFrame(), I get:
img->d_w = 320
img->d_h = 240
(Which are as expected I believe -- same as coded size).

(3) Then, CreateFrame() is called with natural_size = [320x76800], because the config_ had width = 1.

(4) IsValidConfig() returns false because 'natural_size.height()' is over the max limit.

So, not really sure if this is libaom's fault.
What do you think?

P.S. Here's the full 'img' struct and 'visible_rect' in CopyImageToVideoFrame() function at the exact momemnt of assertion failure.

(gdb) p *img
$1 = {fmt = AOM_IMG_FMT_I42016, cp = AOM_CICP_CP_UNSPECIFIED, tc = AOM_CICP_TC_UNSPECIFIED,
  mc = AOM_CICP_MC_UNSPECIFIED, monochrome = 0, csp = AOM_CSP_UNKNOWN, range = AOM_CR_STUDIO_RANGE,
  w = 320, h = 240, bit_depth = 8, d_w = 320, d_h = 240, r_w = 320, r_h = 240, x_chroma_shift = 1,
  y_chroma_shift = 1, planes = {0x7fffd07afa60 "q", 0x7fffd08b6140 "\177", 0x7fffd090f540 "\177", 0x0},
  stride = {1792, 896, 896, 1792}, sz = 0, bps = 12, temporal_id = 0, spatial_id = 0, user_priv = 0x0,
  img_data = 0x7fffd0731820 "", img_data_owner = 0, self_allocd = 0, fb_priv = 0x614000000c50}


(gdb) p visible_rect
$2 = {origin_ = {x_ = 0, y_ = 0}, size_ = {width_ = 320, height_ = 240}}


Ah, I see I misread the visible rect; yeah it sounds like this just shouldn't be a DFATAL, but instead a handled failure case. I'll change these to DLOG(ERROR)
Makes sense. Thanks!
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 31

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

commit 21a783cc46ee68a3ee61cf1147ce24974e306556
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Tue Jul 31 00:42:50 2018

Don't use LOG(DFATAL) in VideoFrame, these paths must be handled.

Per chromium style guide we should not be DCHECK'ing handled cases;
these cases must be handled in release builds and thus should not
have special handling in debug mode.

BUG= 868695 
TEST=none

Change-Id: I1c17d5c2aadeda43fcc4193746623edaadae45e6
Reviewed-on: https://chromium-review.googlesource.com/1155825
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579260}
[modify] https://crrev.com/21a783cc46ee68a3ee61cf1147ce24974e306556/media/base/video_frame.cc

Note: This needs to be merged to M69, after baked in canary for 24 hours.
Shouldn't need to be. It's just the removal of a dcheck that was already backed by a conditional in release builds. We can if tpms want it though.
Project Member

Comment 17 by ClusterFuzz, Jul 31

ClusterFuzz has detected this issue as fixed in range 579258:579264.

Detailed report: https://clusterfuzz.com/testcase?key=6378302265360384

Fuzzer: libFuzzer_mediasource_MP4_AV1_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: Abrt
Crash Address: 0x053900003b92
Crash State:
  media::VideoFrame::CreateFrameWithLayout
  media::VideoFrame::CreateFrameInternal
  media::VideoFrame::CreateZeroInitializedFrame
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=578802:578803
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=579258:579264

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6378302265360384

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 18 by ClusterFuzz, Jul 31

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 6378302265360384 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Sign in to add a comment