| 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 | expand |
| 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 |
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 --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);
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(+)