| Message ID | 20240623230137.1749178-3-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avcodec/aac/aacdec_usac: Test ac in usac | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Quoting Michael Niedermayer (2024-06-24 01:01:35) > Fixes: NULL pointer dereference > Fixes: 69623/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6549698459009024 seems wrong
On Tue, Jun 25, 2024 at 11:00:44AM +0200, Anton Khirnov wrote: > Quoting Michael Niedermayer (2024-06-24 01:01:35) > > Fixes: NULL pointer dereference > > Fixes: 69623/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6549698459009024 > > seems wrong Quite possible, but also your comment seems designed to be unhelpfull you leave the reader guessing what issue you saw exactly and why and what you think is better. Not saying that i couldnt guess but it simply would be nice if you wouldnt omit such details thx [...]
Quoting Michael Niedermayer (2024-06-26 01:52:30) > On Tue, Jun 25, 2024 at 11:00:44AM +0200, Anton Khirnov wrote: > > Quoting Michael Niedermayer (2024-06-24 01:01:35) > > > Fixes: NULL pointer dereference > > > Fixes: 69623/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6549698459009024 > > > > seems wrong > > Quite possible, but also your comment seems designed to be unhelpfull > you leave the reader guessing what issue you saw exactly and why and > what you think is better. My comment is designed to be as helpful as your commit message. "Fixes: NULL pointer dereference" says almost nothing about what actually goes wrong. It should be impossible to get to that point with the SPS being unset. Assuming it somehow does happen, the correct fix is to prevent it from happening, not add random checks to random places.
On Wed, Jun 26, 2024 at 08:38:43AM +0200, Anton Khirnov wrote: > Quoting Michael Niedermayer (2024-06-26 01:52:30) > > On Tue, Jun 25, 2024 at 11:00:44AM +0200, Anton Khirnov wrote: > > > Quoting Michael Niedermayer (2024-06-24 01:01:35) > > > > Fixes: NULL pointer dereference > > > > Fixes: 69623/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6549698459009024 > > > > > > seems wrong > > > > Quite possible, but also your comment seems designed to be unhelpfull > > you leave the reader guessing what issue you saw exactly and why and > > what you think is better. > > My comment is designed to be as helpful as your commit message. If you find my commit message inadequate, you should state that. Simply ommiting information you know in a reply is not efficient in moving a review forward > > "Fixes: NULL pointer dereference" says almost nothing about what > actually goes wrong. It should be impossible to get to that point with > the SPS being unset. Assuming it somehow does happen, the correct fix is > to prevent it from happening, not add random checks to random places. If it still reproduces when i look next time and noone else fixed it before then ill investigate what exactly is going on. thx [...]
diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c index d68d454537a..4dd84d4953c 100644 --- a/libavcodec/hevc/hevcdec.c +++ b/libavcodec/hevc/hevcdec.c @@ -3180,6 +3180,9 @@ static int decode_slice(HEVCContext *s, const H2645NAL *nal, GetBitContext *gb) } else if (!s->cur_frame) { av_log(s->avctx, AV_LOG_ERROR, "First slice in a frame missing.\n"); return AVERROR_INVALIDDATA; + } else if (!s->ps.sps) { + av_log(s->avctx, AV_LOG_ERROR, "sps not set in slice.\n"); + return AVERROR_INVALIDDATA; } if (s->nal_unit_type != s->first_nal_type) {
Fixes: NULL pointer dereference Fixes: 69623/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6549698459009024 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/hevc/hevcdec.c | 3 +++ 1 file changed, 3 insertions(+)