chcunningham: Can you please take a look? Thanks.

This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Matt, I have a fix ready but git cl upload is giving me trouble. Did you have to workaround this issue?

---

$ git cl upload
Using 50% similarity for rename/copy detection. Override with --similarity.
Running presubmit upload checks ...

Presubmit checks passed.
 chromium/patches/README | 7 +++++--
 libavformat/mp3dec.c    | 3 ++-
 2 files changed, 7 insertions(+), 3 deletions(-)
remote: Processing changes: refs: 1, done            
To https://chromium.googlesource.com/chromium/third_party/ffmpeg.git
 ! [remote rejected] c0994b039a9c0c481a4ecaf54041be6e2a4b0b2e -> refs/for/refs/heads/master%notify=NONE (change https://chromium-review.googlesource.com/371419 closed)
error: failed to push some refs to 'https://chromium.googlesource.com/chromium/third_party/ffmpeg.git'
Traceback (most recent call last):
  File "/ssd/depot_tools/git_cl.py", line 5403, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/ssd/depot_tools/git_cl.py", line 5385, in main
    return dispatcher.execute(OptionParser(), argv)
  File "/ssd/depot_tools/subcommand.py", line 252, in execute
    return command(parser, args[1:])
  File "/ssd/depot_tools/git_cl.py", line 4210, in CMDupload
    return cl.CMDUpload(options, args, orig_args)
  File "/ssd/depot_tools/git_cl.py", line 1614, in CMDUpload
    ret = self.CMDUploadChange(options, git_diff_args, change)
  File "/ssd/depot_tools/git_cl.py", line 2828, in CMDUploadChange
    filter_fn=lambda _: sys.stdout.flush())
  File "/ssd/depot_tools/gclient_utils.py", line 538, in CheckCallAndFilter
    rv, args, kwargs.get('cwd', None), None, None)
subprocess2.CalledProcessError: Command 'git push origin c0994b039a9c0c481a4ecaf54041be6e2a4b0b2e:refs/for/refs/heads/master%notify=NONE' returned non-zero exit status 1

Chris, was the due to the FFmpeg roll? If so, please mark this issue as blocking  issue 591845  so that it's on my radar.

@ #8: I didn't have such an issue. What I had problems with was creating new branches in ffmpeg repo (issue 659238). (Currently, our config is patched so that at least Dale and I can do that). Is your change rebased to current downstream master, and just one commit ahead of it on some bug-specific-branch local to your checkout (this is my normal workflow)?

If still having issues, try filing a bug at go/fix-chrome-git. I did that after it being recommended to do that in response to an email I had sent to chrome-infra@ when I was struggling with issue 659238.

Alternatively, just send me a patch out-of-band and I'll 'git cl upload' it with you as author (if I can figure out how to do that :)).

Sorry - dumb mistake on my part. Branch was not entirely fresh. Fix is uploaded here
https://chromium-review.googlesource.com/#/c/413605/

This regression did come with the most recent roll.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/5ed6e20c09840320784c43b86b75b3ede69742f6

commit 5ed6e20c09840320784c43b86b75b3ede69742f6
Author: Chris Cunningham <chcunningham@chromium.org>
Date: Tue Nov 22 21:54:50 2016

mp3dec: fix msan warning when verifying mpa header

MPEG Audio frame header must be 4 bytes. If we fail to read
4 bytes bail early to avoid Use-of-uninitialized-value msan error.

BUG= 666874 
TEST=libfuzzer_media_pipeline_integration_fuzzer

Change-Id: I3a3fdeb1dbd8c8b2f1f81d621bbbafab9b77bb34
Reviewed-on: https://chromium-review.googlesource.com/413605
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>

[modify] https://crrev.com/5ed6e20c09840320784c43b86b75b3ede69742f6/libavformat/mp3dec.c
[modify] https://crrev.com/5ed6e20c09840320784c43b86b75b3ede69742f6/chromium/patches/README

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/5ed6e20c09840320784c43b86b75b3ede69742f6

commit 5ed6e20c09840320784c43b86b75b3ede69742f6
Author: Chris Cunningham <chcunningham@chromium.org>
Date: Tue Nov 22 21:54:50 2016

mp3dec: fix msan warning when verifying mpa header

MPEG Audio frame header must be 4 bytes. If we fail to read
4 bytes bail early to avoid Use-of-uninitialized-value msan error.

BUG= 666874 
TEST=libfuzzer_media_pipeline_integration_fuzzer

Change-Id: I3a3fdeb1dbd8c8b2f1f81d621bbbafab9b77bb34
Reviewed-on: https://chromium-review.googlesource.com/413605
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>

[modify] https://crrev.com/5ed6e20c09840320784c43b86b75b3ede69742f6/libavformat/mp3dec.c
[modify] https://crrev.com/5ed6e20c09840320784c43b86b75b3ede69742f6/chromium/patches/README

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/5ed6e20c09840320784c43b86b75b3ede69742f6

commit 5ed6e20c09840320784c43b86b75b3ede69742f6
Author: Chris Cunningham <chcunningham@chromium.org>
Date: Tue Nov 22 21:54:50 2016

mp3dec: fix msan warning when verifying mpa header

MPEG Audio frame header must be 4 bytes. If we fail to read
4 bytes bail early to avoid Use-of-uninitialized-value msan error.

BUG= 666874 
TEST=libfuzzer_media_pipeline_integration_fuzzer

Change-Id: I3a3fdeb1dbd8c8b2f1f81d621bbbafab9b77bb34
Reviewed-on: https://chromium-review.googlesource.com/413605
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>

[modify] https://crrev.com/5ed6e20c09840320784c43b86b75b3ede69742f6/libavformat/mp3dec.c
[modify] https://crrev.com/5ed6e20c09840320784c43b86b75b3ede69742f6/chromium/patches/README

CC-=smut@.

Sana requested to be re-cc'ed, with @google.

Deps roll for tip of tree is in CQ here.
https://codereview.chromium.org/2527563002/

Assigning to wolenetz@ for merge to 56.

CC+=chcunningham@, so he can follow along the status of this issue that he fixed :)

I'll request merge to M56 once the deps roll (https://codereview.chromium.org/2527563002/) lands and CF verifies this is fixed.

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

commit 0c71998a74cf73fc7b0bb7d089d286449318b691
Author: chcunningham <chcunningham@chromium.org>
Date: Wed Nov 23 01:05:04 2016

Roll src/third_party/ffmpeg 141e56c:5ed6e20 (mp3 msan fix)

Summary of changes available at:
https://chromium.googlesource.com/chromium/third_party/ffmpeg/+log/141e56c..5ed6e20

Brings in single commit:
5ed6e20 mp3dec: fix msan warning when verifying mpa header

BUG= 666874 , 591845 
TBR=wolenetz@chromium.org

Review-Url: https://codereview.chromium.org/2527563002
Cr-Commit-Position: refs/heads/master@{#434055}

[modify] https://crrev.com/0c71998a74cf73fc7b0bb7d089d286449318b691/DEPS

ClusterFuzz has detected this issue as fixed in range 433980:434071.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4504136617033728

Fuzzer: libfuzzer_media_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  check
  mp3_read_header
  avformat_open_input
  
Recommended Security Severity: Medium

Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_msan&range=433017:433116
Fixed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_msan&range=433980:434071

Minimized Testcase (2.73 Kb): https://cluster-fuzz.appspot.com/download/AMIfv94jyXuqQ05kzlUqHRRb_7EvsrWWQ1725UwC6qkJSGmuHn4YhyrxcudkRP_d0YX7IUmQZeN9mihcz99q7IlZGVwPzfzp1WYQmXgtp25YLYfdqK8jI5NHaW95FxMtiP2Jl2Zt0A9PT3MqL9W-x1lZwiy7NQtZ0A?testcase_id=4504136617033728

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

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

ClusterFuzz testcase is verified as fixed, closing issue.

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

Requesting merge to M56 (along with  issue 591845 , which already has merge request pending). #20 fixes a regression caused by  issue 591845 .

[Automated comment] DEPS changes referenced in bugdroid comments, needs manual review.

This change meets the bar and is approved for merge into M56

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

commit 54e6804de2a9c3d3784afaa7775ac97bbf433176
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Wed Nov 30 00:08:47 2016

To M56: Roll src/third_party/ffmpeg 141e56c:5ed6e20 (mp3 msan fix)

Summary of changes available at:
https://chromium.googlesource.com/chromium/third_party/ffmpeg/+log/141e56c..5ed6e20

Brings in single commit:
5ed6e20 mp3dec: fix msan warning when verifying mpa header

BUG= 666874 , 591845 
TBR=chcunningham@chromium.org

Review-Url: https://codereview.chromium.org/2527563002
Cr-Commit-Position: refs/heads/master@{#434055}
(cherry picked from commit 0c71998a74cf73fc7b0bb7d089d286449318b691)

Review URL: https://codereview.chromium.org/2543473002 .

Cr-Commit-Position: refs/branch-heads/2924@{#176}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/54e6804de2a9c3d3784afaa7775ac97bbf433176/DEPS

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/f4e4b1ff5dbdf3701bed5a709344372780af24d6

commit f4e4b1ff5dbdf3701bed5a709344372780af24d6
Author: Alex Mineer <amineer@google.com>
Date: Wed Nov 30 00:20:17 2016

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

commit 0bf26e16060899a224a208cfbc40549fc924f1c0
Author: wolenetz <wolenetz@chromium.org>
Date: Thu Dec 08 20:45:39 2016

Add ffmpeg regression tests for multiple issues from M56 roll

Note: Neither I nor chcunningham@ were able to reproduce 666874 with
current msan tooling, though both CF and chcunningham@ confirmed the
fix previously. Perhaps toolchain or sanitizer changes in the interim
have impacted ability to repro this case.

Excepting above, all new tests repro prior to their fix, and no longer
repro on trunk.

For  issue 666770 , a seek to GetStartTime() was insufficient for repro,
so a _SEEKING version of the test macro was added to obtain
repro.

Added 8b80a219364dd4c4baaa9297005218f43dc5c49f to internal repo.

BUG= 666794 , 666874 ,667063, 666770 
R=dalecurtis@chromium.org,chcunningham@chromium.org

Review-Url: https://codereview.chromium.org/2556343002
Cr-Commit-Position: refs/heads/master@{#437331}

[modify] https://crrev.com/0bf26e16060899a224a208cfbc40549fc924f1c0/media/ffmpeg/ffmpeg_regression_tests.cc

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/2db39ae520c90db0db865a6b5d15017f1f9f4d09

commit 2db39ae520c90db0db865a6b5d15017f1f9f4d09
Author: Alex Mineer <amineer@google.com>
Date: Thu Dec 15 22:13:56 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/f2b039af93bf22b422b46af2fb6f7aed10ff4f3e

commit f2b039af93bf22b422b46af2fb6f7aed10ff4f3e
Author: Alex Mineer <amineer@google.com>
Date: Thu Dec 15 23:22:05 2016

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot