Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2471973003:
Allow H264 bitstream parser to fail gracefully (Closed)
|
Created:
4 years, 1 month ago by kthelgason Modified:
4 years, 1 month ago CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAllow H264 bitstream parser to fail gracefully
This CL allows the H264 bitstream parser to abort and
report an error on invalid input rather than crashing, and it fixes
several crashes found when fuzzing.
BUG=webrtc:6454
R=magjed@webrtc.org,pbos@webrtc.org
Committed: https://crrev.com/f752bca4a5a9dcf6218e820f50623201a71cb0a7
Cr-Commit-Position: refs/heads/master@{#14929}
Patch Set 1 #Patch Set 2 : rebase #Patch Set 3 : land fuzzer CL separately #
Total comments: 2
Patch Set 4 #
Messages
Total messages: 34 (26 generated)
PTAL. pbos: This CL contains your fuzzer implementation as well.
The CQ bit was checked by kthelgason@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: presubmit on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/9799)
The CQ bit was checked by kthelgason@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_clang_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_clang_dbg/build...)
The CQ bit was checked by kthelgason@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Allow bitstream parser to fail gracefully This CL adds a fuzzer for H264 bitstream parser, allows it to abort and report an error on invalid input rather than crashing, and it fixes several crashes found when fuzzing. BUG=webrtc:6454 R=magjed@webrtc.org,pbos@webrtc.org patch from issue 2430703002 at patchset 20001 (http://crrev.com/2430703002#ps20001) ========== to ========== Allow bitstream parser to fail gracefully This CL allows the H264 bitstream parser to abort and report an error on invalid input rather than crashing, and it fixes several crashes found when fuzzing. BUG=webrtc:6454 R=magjed@webrtc.org,pbos@webrtc.org patch from issue 2430703002 at patchset 20001 (http://crrev.com/2430703002#ps20001) ==========
Description was changed from ========== Allow bitstream parser to fail gracefully This CL allows the H264 bitstream parser to abort and report an error on invalid input rather than crashing, and it fixes several crashes found when fuzzing. BUG=webrtc:6454 R=magjed@webrtc.org,pbos@webrtc.org patch from issue 2430703002 at patchset 20001 (http://crrev.com/2430703002#ps20001) ========== to ========== Allow bitstream parser to fail gracefully This CL allows the H264 bitstream parser to abort and report an error on invalid input rather than crashing, and it fixes several crashes found when fuzzing. BUG=webrtc:6454 R=magjed@webrtc.org,pbos@webrtc.org ==========
lgtm, I removed the reference to the fuzzer in the CL desc https://codereview.webrtc.org/2471973003/diff/40001/webrtc/common_video/h264/... File webrtc/common_video/h264/h264_bitstream_parser.cc (right): https://codereview.webrtc.org/2471973003/diff/40001/webrtc/common_video/h264/... webrtc/common_video/h264/h264_bitstream_parser.cc:30: #define RETURN_INV_ON_FAIL(x) RETURN_ON_FAIL(x, kInvalidStream) I think you can s/INV/INVALID_STREAM/g
The CQ bit was checked by kthelgason@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: CLs for remote refs other than refs/pending/heads/master must contain NOTRY=true and NOPRESUBMIT=true in order for the CQ to process them
Description was changed from ========== Allow bitstream parser to fail gracefully This CL allows the H264 bitstream parser to abort and report an error on invalid input rather than crashing, and it fixes several crashes found when fuzzing. BUG=webrtc:6454 R=magjed@webrtc.org,pbos@webrtc.org ========== to ========== Allow H264 bitstream parser to fail gracefully This CL allows the H264 bitstream parser to abort and report an error on invalid input rather than crashing, and it fixes several crashes found when fuzzing. BUG=webrtc:6454 R=magjed@webrtc.org,pbos@webrtc.org ==========
lgtm https://codereview.webrtc.org/2471973003/diff/40001/webrtc/common_video/h264/... File webrtc/common_video/h264/h264_bitstream_parser.cc (right): https://codereview.webrtc.org/2471973003/diff/40001/webrtc/common_video/h264/... webrtc/common_video/h264/h264_bitstream_parser.cc:275: LOG(LS_INFO) << "Failed to parse bitstream. Error: " << res; nit: I think we should do at least LS_WARNING here.
The CQ bit was checked by kthelgason@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
CLs for remote refs other than refs/pending/heads/master must contain NOTRY=true and NOPRESUBMIT=true in order for the CQ to process them
The CQ bit was unchecked by kthelgason@webrtc.org
The CQ bit was checked by kthelgason@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org, pbos@webrtc.org Link to the patchset: https://codereview.webrtc.org/2471973003/#ps60001 (title: "formatting")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Message was sent while issue was closed.
Description was changed from ========== Allow H264 bitstream parser to fail gracefully This CL allows the H264 bitstream parser to abort and report an error on invalid input rather than crashing, and it fixes several crashes found when fuzzing. BUG=webrtc:6454 R=magjed@webrtc.org,pbos@webrtc.org ========== to ========== Allow H264 bitstream parser to fail gracefully This CL allows the H264 bitstream parser to abort and report an error on invalid input rather than crashing, and it fixes several crashes found when fuzzing. BUG=webrtc:6454 R=magjed@webrtc.org,pbos@webrtc.org ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Allow H264 bitstream parser to fail gracefully This CL allows the H264 bitstream parser to abort and report an error on invalid input rather than crashing, and it fixes several crashes found when fuzzing. BUG=webrtc:6454 R=magjed@webrtc.org,pbos@webrtc.org ========== to ========== Allow H264 bitstream parser to fail gracefully This CL allows the H264 bitstream parser to abort and report an error on invalid input rather than crashing, and it fixes several crashes found when fuzzing. BUG=webrtc:6454 R=magjed@webrtc.org,pbos@webrtc.org Committed: https://crrev.com/f752bca4a5a9dcf6218e820f50623201a71cb0a7 Cr-Commit-Position: refs/heads/master@{#14929} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/f752bca4a5a9dcf6218e820f50623201a71cb0a7 Cr-Commit-Position: refs/heads/master@{#14929} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
