diff mbox series

[FFmpeg-devel] avcodec/flashsv2enc: Fix use of uninitialized value

Message ID 20210127113820.2185240-1-andreas.rheinhardt@gmail.com
State Accepted
Commit 9267e2ff0d5b518bcce6236d09f2941b2c1bba84
Headers show
Series [FFmpeg-devel] avcodec/flashsv2enc: Fix use of uninitialized value
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Jan. 27, 2021, 11:38 a.m. UTC
Before 257a83b969157eb76c18158a4e503e908d8b1125, certain buffers were
zero-allocated in the init function and only reallocated lateron if they
turned out to be too small; now they are only allocated during init,
leading to use-of-uninitialized values lateron. The same could happen
before if the dimensions are big enough so that the buffers would be
reallocated, as the new part of the reallocated buffer would not be
zeroed (happened for 960x960). So always zero the buffers in the
function designed to init them.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
If no one objects, I'll send a patch to remove #ifndef FLASHSV2_DUMB stuff
lateron: It doesn't even compile any more and given that it has never
worked it stands to reason that any successfull non-dumb way needs to be
different from the currently outcommented code. Of course, I don't think
that anyone will ever add a successfull non-dumb way for this encoder
for an old format.
Furthermore, there are more bugs lurking in this code, namely the
ptr = av_realloc_array(ptr, size) which leads to memleaks on
reallocation failures as well as problems if the caller tries to call
the encoder lateron because block_width/height have already been set, so
that no reallocation attempt would be performed.

 libavcodec/flashsv2enc.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Marton Balint Jan. 27, 2021, 8:57 p.m. UTC | #1
On Wed, 27 Jan 2021, Andreas Rheinhardt wrote:

> Before 257a83b969157eb76c18158a4e503e908d8b1125, certain buffers were
> zero-allocated in the init function and only reallocated lateron if they
> turned out to be too small; now they are only allocated during init,
> leading to use-of-uninitialized values lateron. The same could happen
> before if the dimensions are big enough so that the buffers would be
> reallocated, as the new part of the reallocated buffer would not be
> zeroed (happened for 960x960). So always zero the buffers in the
> function designed to init them.

LGTM, thanks.

>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> If no one objects, I'll send a patch to remove #ifndef FLASHSV2_DUMB stuff
> lateron: It doesn't even compile any more and given that it has never
> worked it stands to reason that any successfull non-dumb way needs to be
> different from the currently outcommented code. Of course, I don't think
> that anyone will ever add a successfull non-dumb way for this encoder
> for an old format.

Good idea, i thought about it too.

> Furthermore, there are more bugs lurking in this code, namely the
> ptr = av_realloc_array(ptr, size) which leads to memleaks on
> reallocation failures as well as problems if the caller tries to call
> the encoder lateron because block_width/height have already been set, so
> that no reallocation attempt would be performed.

Yes, indeed.

Thanks for taking care of these.

Regards,
Marton

>
> libavcodec/flashsv2enc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/libavcodec/flashsv2enc.c b/libavcodec/flashsv2enc.c
> index 5139b17a28..430b6806c8 100644
> --- a/libavcodec/flashsv2enc.c
> +++ b/libavcodec/flashsv2enc.c
> @@ -142,6 +142,7 @@ static void init_blocks(FlashSV2Context * s, Block * blocks,
> {
>     int row, col;
>     Block *b;
> +    memset(blocks, 0, s->cols * s->rows * sizeof(*blocks));
>     for (col = 0; col < s->cols; col++) {
>         for (row = 0; row < s->rows; row++) {
>             b = blocks + (col + row * s->cols);
> -- 
> 2.25.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavcodec/flashsv2enc.c b/libavcodec/flashsv2enc.c
index 5139b17a28..430b6806c8 100644
--- a/libavcodec/flashsv2enc.c
+++ b/libavcodec/flashsv2enc.c
@@ -142,6 +142,7 @@  static void init_blocks(FlashSV2Context * s, Block * blocks,
 {
     int row, col;
     Block *b;
+    memset(blocks, 0, s->cols * s->rows * sizeof(*blocks));
     for (col = 0; col < s->cols; col++) {
         for (row = 0; row < s->rows; row++) {
             b = blocks + (col + row * s->cols);