Message ID | 20191010094741.814-1-andreas.rheinhardt@gmail.com |
---|---|
State | Accepted |
Commit | def04022f4a7058f99e669bfd978d431d79aec18 |
Headers | show |
tor 2019-10-10 klockan 11:47 +0200 skrev Andreas Rheinhardt: > zmbvenc allocates a buffer for a picture with padding on all four sides: > The stride is choosen so large that it already contains padding on the > right; the height also includes padding rows. The padding on the right > of each row is also reused as padding for the left of the next row. So > one still needs to add padding on the left for the first row. This is done > by offsetting the actual pointer used to access the picture from the > pointer returned by av_mallocz and the formula for this offset was > wrong, because it ignored that a pixel can take more than one byte when > calculating the offset resulting from the left padding of the first row. > > This fixes accesses outside of the allocated buffer that were reported > in tickets #7980 and #7994. No writes were ever attempted outside of > the buffer. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavcodec/zmbvenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/zmbvenc.c b/libavcodec/zmbvenc.c > index 0e22ce687f..319381dd48 100644 > --- a/libavcodec/zmbvenc.c > +++ b/libavcodec/zmbvenc.c > @@ -409,7 +409,7 @@ static av_cold int encode_init(AVCodecContext *avctx) > */ > c->pstride = FFALIGN((avctx->width + c->lrange) * c->bypp, 16); > prev_size = FFALIGN(c->lrange * c->bypp, 16) + c->pstride * (c->lrange + avctx->height + c->urange); > - prev_offset = FFALIGN(c->lrange, 16) + c->pstride * c->lrange; > + prev_offset = FFALIGN(c->lrange * c->bypp, 16) + c->pstride * c->lrange; > if (!(c->prev_buf = av_mallocz(prev_size))) { > av_log(avctx, AV_LOG_ERROR, "Can't allocate picture.\n"); > return AVERROR(ENOMEM); Looks OK. Perhaps there should be a static function in some header to get a pointer to the start of a given line in a picture. That'd cut down on bugs like this.. /Tomas
On Thu, Oct 10, 2019 at 01:38:32PM +0200, Tomas Härdin wrote: > tor 2019-10-10 klockan 11:47 +0200 skrev Andreas Rheinhardt: > > zmbvenc allocates a buffer for a picture with padding on all four sides: > > The stride is choosen so large that it already contains padding on the > > right; the height also includes padding rows. The padding on the right > > of each row is also reused as padding for the left of the next row. So > > one still needs to add padding on the left for the first row. This is done > > by offsetting the actual pointer used to access the picture from the > > pointer returned by av_mallocz and the formula for this offset was > > wrong, because it ignored that a pixel can take more than one byte when > > calculating the offset resulting from the left padding of the first row. > > > > This fixes accesses outside of the allocated buffer that were reported > > in tickets #7980 and #7994. No writes were ever attempted outside of > > the buffer. > > > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > > --- > > libavcodec/zmbvenc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/zmbvenc.c b/libavcodec/zmbvenc.c > > index 0e22ce687f..319381dd48 100644 > > --- a/libavcodec/zmbvenc.c > > +++ b/libavcodec/zmbvenc.c > > @@ -409,7 +409,7 @@ static av_cold int encode_init(AVCodecContext *avctx) > > */ > > c->pstride = FFALIGN((avctx->width + c->lrange) * c->bypp, 16); > > prev_size = FFALIGN(c->lrange * c->bypp, 16) + c->pstride * (c->lrange + avctx->height + c->urange); > > - prev_offset = FFALIGN(c->lrange, 16) + c->pstride * c->lrange; > > + prev_offset = FFALIGN(c->lrange * c->bypp, 16) + c->pstride * c->lrange; > > if (!(c->prev_buf = av_mallocz(prev_size))) { > > av_log(avctx, AV_LOG_ERROR, "Can't allocate picture.\n"); > > return AVERROR(ENOMEM); > > Looks OK. will apply thx [...]
diff --git a/libavcodec/zmbvenc.c b/libavcodec/zmbvenc.c index 0e22ce687f..319381dd48 100644 --- a/libavcodec/zmbvenc.c +++ b/libavcodec/zmbvenc.c @@ -409,7 +409,7 @@ static av_cold int encode_init(AVCodecContext *avctx) */ c->pstride = FFALIGN((avctx->width + c->lrange) * c->bypp, 16); prev_size = FFALIGN(c->lrange * c->bypp, 16) + c->pstride * (c->lrange + avctx->height + c->urange); - prev_offset = FFALIGN(c->lrange, 16) + c->pstride * c->lrange; + prev_offset = FFALIGN(c->lrange * c->bypp, 16) + c->pstride * c->lrange; if (!(c->prev_buf = av_mallocz(prev_size))) { av_log(avctx, AV_LOG_ERROR, "Can't allocate picture.\n"); return AVERROR(ENOMEM);
zmbvenc allocates a buffer for a picture with padding on all four sides: The stride is choosen so large that it already contains padding on the right; the height also includes padding rows. The padding on the right of each row is also reused as padding for the left of the next row. So one still needs to add padding on the left for the first row. This is done by offsetting the actual pointer used to access the picture from the pointer returned by av_mallocz and the formula for this offset was wrong, because it ignored that a pixel can take more than one byte when calculating the offset resulting from the left padding of the first row. This fixes accesses outside of the allocated buffer that were reported in tickets #7980 and #7994. No writes were ever attempted outside of the buffer. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavcodec/zmbvenc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)