On Sun, Mar 29, 2015 at 11:05:40AM +0000, Donny Yang wrote: > Signed-off-by: Donny Yang <w...@kota.moe> > --- > libavcodec/pngenc.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/pngenc.c b/libavcodec/pngenc.c > index d6233d0..3697dbb 100644 > --- a/libavcodec/pngenc.c > +++ b/libavcodec/pngenc.c > @@ -360,12 +360,23 @@ static int encode_frame(AVCodecContext *avctx, AVPacket > *pkt, > return -1; > > enc_row_size = deflateBound(&s->zstream, row_size); > - max_packet_size = avctx->height * (int64_t)(enc_row_size + > - ((enc_row_size + IOBUF_SIZE - 1) / > IOBUF_SIZE) * 12) > - + FF_MIN_BUFFER_SIZE; > + max_packet_size = > + 8 + // PNGSIG > + 13 + 12 + // IHDR > + 9 + 12 + // pHYs > + 1 + 12 + // sRGB > + 32 + 12 + // cHRM > + 4 + 12 + // gAMA > + 256 * 3 + 12 + // PLTE > + 256 + 12 + // tRNS > + avctx->height * ( > + enc_row_size + > + 12 * (((int64_t)enc_row_size + IOBUF_SIZE - 1) / IOBUF_SIZE) // > 12 * ceil(enc_row_size / IOBUF_SIZE) > + ); > if (max_packet_size > INT_MAX) > return AVERROR(ENOMEM); > - if ((ret = ff_alloc_packet2(avctx, pkt, max_packet_size)) < 0) > + ret = ff_alloc_packet2(avctx, pkt, max_packet_size < FF_MIN_BUFFER_SIZE > ? FF_MIN_BUFFER_SIZE : max_packet_size);
i understand the change is well meant and it makes the allocated size closer to what is actually written but this would require the list of sizes to be kept in sync with what is actually written. This is a additional maintaince burden and has serious consequence if one makes a change and then forgets updating the size as it could result in out of array writes becoming possible and then theres a possibly exploitable security issues if that happens so i suggest to leave the "+ FF_MIN_BUFFER_SIZE" in there unless there is something else that protects against more data being written than what is listed here > + if (ret) (ret < 0) [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If a bugfix only changes things apparently unrelated to the bug with no further explanation, that is a good sign that the bugfix is wrong.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel