diff mbox series

[FFmpeg-devel,2/2] avdevice/libopenh264dec: Increase array sizes, fix stack-buffer overread

Message ID AM7PR03MB666051232411653D3E9CD8E68F6D9@AM7PR03MB6660.eurprd03.prod.outlook.com
State Accepted
Commit b0b90100bfda8c5cf95889e00183589de0abce60
Headers show
Series [FFmpeg-devel,1/2] avdevice/libkvazaar: Increase array size
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Andreas Rheinhardt Dec. 6, 2021, 11:37 a.m. UTC
av_image_copy() expects an array of four pointers and linesizes
according to its declaration; it currently only pointers that are
actually in use (depending upon the pixel format), but this might
change at any time. It has already happened for the linesizes in
d7bc52bf456deba0f32d9fe5c288ec441f1ebef5 and so increasing their
array fixes a stack-buffer overread.

This fixes a -Wstringop-overflow= and -Wstringop-overread warning
from GCC 11.2.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/libopenh264dec.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Linjie Fu Dec. 6, 2021, 12:29 p.m. UTC | #1
On Mon, Dec 6, 2021 at 7:37 PM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> av_image_copy() expects an array of four pointers and linesizes
> according to its declaration; it currently only pointers that are
> actually in use (depending upon the pixel format), but this might
> change at any time. It has already happened for the linesizes in
> d7bc52bf456deba0f32d9fe5c288ec441f1ebef5 and so increasing their
> array fixes a stack-buffer overread.
>
> This fixes a -Wstringop-overflow= and -Wstringop-overread warning
> from GCC 11.2.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/libopenh264dec.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/libopenh264dec.c b/libavcodec/libopenh264dec.c
> index ea70a8e143..7f5e85402a 100644
> --- a/libavcodec/libopenh264dec.c
> +++ b/libavcodec/libopenh264dec.c
> @@ -91,8 +91,8 @@ static int svc_decode_frame(AVCodecContext *avctx, void
> *data,
>  {
>      SVCContext *s = avctx->priv_data;
>      SBufferInfo info = { 0 };
> -    uint8_t* ptrs[3];
> -    int ret, linesize[3];
> +    uint8_t *ptrs[4] = { NULL };
> +    int ret, linesize[4];
>      AVFrame *avframe = data;
>      DECODING_STATE state;
>  #if OPENH264_VER_AT_LEAST(1, 7)
> @@ -140,6 +140,7 @@ static int svc_decode_frame(AVCodecContext *avctx,
> void *data,
>
>      linesize[0] = info.UsrData.sSystemBuffer.iStride[0];
>      linesize[1] = linesize[2] = info.UsrData.sSystemBuffer.iStride[1];
> +    linesize[3] = 0;
>      av_image_copy(avframe->data, avframe->linesize, (const uint8_t **)
> ptrs, linesize, avctx->pix_fmt, avctx->width, avctx->height);
>
>      avframe->pts     = info.uiOutYuvTimeStamp;
> --
> 2.32.0
>
 lgtm. (guess the title is referring to  "avcodec/libopenh264dec: xxx" ?)
Andreas Rheinhardt Dec. 6, 2021, 12:32 p.m. UTC | #2
Linjie Fu:
> On Mon, Dec 6, 2021 at 7:37 PM Andreas Rheinhardt <
> andreas.rheinhardt@outlook.com> wrote:
> 
>> av_image_copy() expects an array of four pointers and linesizes
>> according to its declaration; it currently only pointers that are
>> actually in use (depending upon the pixel format), but this might
>> change at any time. It has already happened for the linesizes in
>> d7bc52bf456deba0f32d9fe5c288ec441f1ebef5 and so increasing their
>> array fixes a stack-buffer overread.
>>
>> This fixes a -Wstringop-overflow= and -Wstringop-overread warning
>> from GCC 11.2.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>  libavcodec/libopenh264dec.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/libopenh264dec.c b/libavcodec/libopenh264dec.c
>> index ea70a8e143..7f5e85402a 100644
>> --- a/libavcodec/libopenh264dec.c
>> +++ b/libavcodec/libopenh264dec.c
>> @@ -91,8 +91,8 @@ static int svc_decode_frame(AVCodecContext *avctx, void
>> *data,
>>  {
>>      SVCContext *s = avctx->priv_data;
>>      SBufferInfo info = { 0 };
>> -    uint8_t* ptrs[3];
>> -    int ret, linesize[3];
>> +    uint8_t *ptrs[4] = { NULL };
>> +    int ret, linesize[4];
>>      AVFrame *avframe = data;
>>      DECODING_STATE state;
>>  #if OPENH264_VER_AT_LEAST(1, 7)
>> @@ -140,6 +140,7 @@ static int svc_decode_frame(AVCodecContext *avctx,
>> void *data,
>>
>>      linesize[0] = info.UsrData.sSystemBuffer.iStride[0];
>>      linesize[1] = linesize[2] = info.UsrData.sSystemBuffer.iStride[1];
>> +    linesize[3] = 0;
>>      av_image_copy(avframe->data, avframe->linesize, (const uint8_t **)
>> ptrs, linesize, avctx->pix_fmt, avctx->width, avctx->height);
>>
>>      avframe->pts     = info.uiOutYuvTimeStamp;
>> --
>> 2.32.0
>>
>  lgtm. (guess the title is referring to  "avcodec/libopenh264dec: xxx" ?)
> 

Yes. I reused and adapted the commit message of
9b17273c77ee2868ef34abc49efa70260453235b, but apparently forgot this.
Will fix before committing.

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/libopenh264dec.c b/libavcodec/libopenh264dec.c
index ea70a8e143..7f5e85402a 100644
--- a/libavcodec/libopenh264dec.c
+++ b/libavcodec/libopenh264dec.c
@@ -91,8 +91,8 @@  static int svc_decode_frame(AVCodecContext *avctx, void *data,
 {
     SVCContext *s = avctx->priv_data;
     SBufferInfo info = { 0 };
-    uint8_t* ptrs[3];
-    int ret, linesize[3];
+    uint8_t *ptrs[4] = { NULL };
+    int ret, linesize[4];
     AVFrame *avframe = data;
     DECODING_STATE state;
 #if OPENH264_VER_AT_LEAST(1, 7)
@@ -140,6 +140,7 @@  static int svc_decode_frame(AVCodecContext *avctx, void *data,
 
     linesize[0] = info.UsrData.sSystemBuffer.iStride[0];
     linesize[1] = linesize[2] = info.UsrData.sSystemBuffer.iStride[1];
+    linesize[3] = 0;
     av_image_copy(avframe->data, avframe->linesize, (const uint8_t **) ptrs, linesize, avctx->pix_fmt, avctx->width, avctx->height);
 
     avframe->pts     = info.uiOutYuvTimeStamp;