diff mbox series

[FFmpeg-devel,1/6] avcodec/utils: allocate a line more for VC1 and WMV3

Message ID 20230111204221.22550-1-michael@niedermayer.cc
State Accepted
Commit 01636a63d452c592ece35af6f72bb7affcad58f2
Headers show
Series [FFmpeg-devel,1/6] avcodec/utils: allocate a line more for VC1 and WMV3 | expand

Checks

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

Commit Message

Michael Niedermayer Jan. 11, 2023, 8:42 p.m. UTC
Fixes: out of array read on 32bit
Fixes: 54857/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VC1_fuzzer-5840588224462848

The chroma MC code reads over the currently allocated frame.
Alternative fixes would be allocating a few bytes more at the end instead of a whole
line extra or to adjust the threshold where the edge emu code is activated

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/utils.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Andreas Rheinhardt Jan. 12, 2023, 7:38 a.m. UTC | #1
Michael Niedermayer:
> Fixes: out of array read on 32bit
> Fixes: 54857/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VC1_fuzzer-5840588224462848
> 
> The chroma MC code reads over the currently allocated frame.
> Alternative fixes would be allocating a few bytes more at the end instead of a whole
> line extra or to adjust the threshold where the edge emu code is activated
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/utils.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 2b63a498b9..1aa0a05a31 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -321,6 +321,7 @@ void avcodec_align_dimensions2(AVCodecContext *s, int *width, int *height,
>      *width  = FFALIGN(*width, w_align);
>      *height = FFALIGN(*height, h_align);
>      if (s->codec_id == AV_CODEC_ID_H264 || s->lowres ||
> +        s->codec_id == AV_CODEC_ID_VC1  || s->codec_id == AV_CODEC_ID_WMV3 ||
>          s->codec_id == AV_CODEC_ID_VP5  || s->codec_id == AV_CODEC_ID_VP6 ||
>          s->codec_id == AV_CODEC_ID_VP6F || s->codec_id == AV_CODEC_ID_VP6A
>      ) {

Does this only happen on 32bit systems? If so, why?

- Andreas
Michael Niedermayer Jan. 12, 2023, 2:43 p.m. UTC | #2
On Thu, Jan 12, 2023 at 08:38:08AM +0100, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: out of array read on 32bit
> > Fixes: 54857/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VC1_fuzzer-5840588224462848
> > 
> > The chroma MC code reads over the currently allocated frame.
> > Alternative fixes would be allocating a few bytes more at the end instead of a whole
> > line extra or to adjust the threshold where the edge emu code is activated
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/utils.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > index 2b63a498b9..1aa0a05a31 100644
> > --- a/libavcodec/utils.c
> > +++ b/libavcodec/utils.c
> > @@ -321,6 +321,7 @@ void avcodec_align_dimensions2(AVCodecContext *s, int *width, int *height,
> >      *width  = FFALIGN(*width, w_align);
> >      *height = FFALIGN(*height, h_align);
> >      if (s->codec_id == AV_CODEC_ID_H264 || s->lowres ||
> > +        s->codec_id == AV_CODEC_ID_VC1  || s->codec_id == AV_CODEC_ID_WMV3 ||
> >          s->codec_id == AV_CODEC_ID_VP5  || s->codec_id == AV_CODEC_ID_VP6 ||
> >          s->codec_id == AV_CODEC_ID_VP6F || s->codec_id == AV_CODEC_ID_VP6A
> >      ) {
> 
> Does this only happen on 32bit systems? If so, why?

Id have to double check, some of the issues in this set where 32bit specific some not
But the x86 codepath has special cases i think for non interpolated cases while
the C path which was used in the issue does not.
A check could also be added to the C path to not read the next line whan not needed
the patch is just folllowing what h264 does 

thx

[...]
Michael Niedermayer Feb. 23, 2023, 10:26 p.m. UTC | #3
On Wed, Jan 11, 2023 at 09:42:16PM +0100, Michael Niedermayer wrote:
> Fixes: out of array read on 32bit
> Fixes: 54857/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VC1_fuzzer-5840588224462848
> 
> The chroma MC code reads over the currently allocated frame.
> Alternative fixes would be allocating a few bytes more at the end instead of a whole
> line extra or to adjust the threshold where the edge emu code is activated
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/utils.c | 1 +
>  1 file changed, 1 insertion(+)

will apply remaining bits of this patchset

[...]
diff mbox series

Patch

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 2b63a498b9..1aa0a05a31 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -321,6 +321,7 @@  void avcodec_align_dimensions2(AVCodecContext *s, int *width, int *height,
     *width  = FFALIGN(*width, w_align);
     *height = FFALIGN(*height, h_align);
     if (s->codec_id == AV_CODEC_ID_H264 || s->lowres ||
+        s->codec_id == AV_CODEC_ID_VC1  || s->codec_id == AV_CODEC_ID_WMV3 ||
         s->codec_id == AV_CODEC_ID_VP5  || s->codec_id == AV_CODEC_ID_VP6 ||
         s->codec_id == AV_CODEC_ID_VP6F || s->codec_id == AV_CODEC_ID_VP6A
     ) {