While running under Clang's UndefinedBehaviorSanitizer, I found a few
places where av_image_fill_pointers is called before buffers for the image
are allocated, so ptr is passed in as NULL.
This leads to (currently harmless) UB when the plane sizes are added to the
null pointer, so I was wondering if there was interest in avoiding it?
I've attached a patch to expose some extra utilities and avoid passing in
the null pointer, but is this an appropriate way to work around it?
Thank you,
Brian
From 9db97425b2b3ca0913b7ce8f6f7c096a8aa5c964 Mon Sep 17 00:00:00 2001
From: Brian Kim <bk...@google.com>
Date: Tue, 30 Jun 2020 17:59:53 -0700
Subject: [PATCH] libavutil/imgutils: add utility to get plane sizes
This utility helps avoid undefined behavior when doing things like
checking how much memory we need to allocate for an image before we have
allocated a buffer.
---
libavcodec/decode.c | 11 +++-------
libavutil/frame.c | 9 ++++----
libavutil/imgutils.c | 49 ++++++++++++++++++++++++++++++++------------
libavutil/imgutils.h | 22 ++++++++++++++++++++
4 files changed, 65 insertions(+), 26 deletions(-)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index de9c079f9d..cd0424b467 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1471,9 +1471,8 @@ static int update_frame_pool(AVCodecContext *avctx, AVFrame *frame)
switch (avctx->codec_type) {
case AVMEDIA_TYPE_VIDEO: {
- uint8_t *data[4];
int linesize[4];
- int size[4] = { 0 };
+ int size[4];
int w = frame->width;
int h = frame->height;
int tmpsize, unaligned;
@@ -1494,17 +1493,13 @@ static int update_frame_pool(AVCodecContext *avctx, AVFrame *frame)
unaligned |= linesize[i] % pool->stride_align[i];
} while (unaligned);
- tmpsize = av_image_fill_pointers(data, avctx->pix_fmt, h,
- NULL, linesize);
+ tmpsize = av_image_fill_plane_sizes(size, avctx->pix_fmt, h,
+ linesize);
if (tmpsize < 0) {
ret = tmpsize;
goto fail;
}
- for (i = 0; i < 3 && data[i + 1]; i++)
- size[i] = data[i + 1] - data[i];
- size[i] = tmpsize - (data[i] - data[0]);
-
for (i = 0; i < 4; i++) {
pool->linesize[i] = linesize[i];
if (size[i]) {
diff --git a/libavutil/frame.c b/libavutil/frame.c
index 9884eae054..3abb1f12d5 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -212,6 +212,7 @@ void av_frame_free(AVFrame **frame)
static int get_video_buffer(AVFrame *frame, int align)
{
const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format);
+ int size[4];
int ret, i, padded_height;
int plane_padding = FFMAX(16 + 16/*STRIDE_ALIGN*/, align);
@@ -239,8 +240,8 @@ static int get_video_buffer(AVFrame *frame, int align)
}
padded_height = FFALIGN(frame->height, 32);
- if ((ret = av_image_fill_pointers(frame->data, frame->format, padded_height,
- NULL, frame->linesize)) < 0)
+ if ((ret = av_image_fill_plane_sizes(size, frame->format, padded_height,
+ frame->linesize)) < 0)
return ret;
frame->buf[0] = av_buffer_alloc(ret + 4*plane_padding);
@@ -249,9 +250,7 @@ static int get_video_buffer(AVFrame *frame, int align)
goto fail;
}
- if ((ret = av_image_fill_pointers(frame->data, frame->format, padded_height,
- frame->buf[0]->data, frame->linesize)) < 0)
- goto fail;
+ av_image_fill_pointers_from_sizes(frame->data, size, frame->buf[0]->data);
for (i = 1; i < 4; i++) {
if (frame->data[i])
diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
index 7f9c1b632c..7045a9df65 100644
--- a/libavutil/imgutils.c
+++ b/libavutil/imgutils.c
@@ -108,26 +108,25 @@ int av_image_fill_linesizes(int linesizes[4], enum AVPixelFormat pix_fmt, int wi
return 0;
}
-int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int height,
- uint8_t *ptr, const int linesizes[4])
+int av_image_fill_plane_sizes(int size[4], enum AVPixelFormat pix_fmt, int height,
+ const int linesizes[4])
{
- int i, total_size, size[4] = { 0 }, has_plane[4] = { 0 };
+ int i, total_size, has_plane[4] = { 0 };
const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
- memset(data , 0, sizeof(data[0])*4);
+ memset(size , 0, sizeof(size[0])*4);
if (!desc || desc->flags & AV_PIX_FMT_FLAG_HWACCEL)
return AVERROR(EINVAL);
- data[0] = ptr;
if (linesizes[0] > (INT_MAX - 1024) / height)
return AVERROR(EINVAL);
size[0] = linesizes[0] * height;
if (desc->flags & AV_PIX_FMT_FLAG_PAL ||
desc->flags & FF_PSEUDOPAL) {
- data[1] = ptr + size[0]; /* palette is stored here as 256 32 bits words */
- return size[0] + 256 * 4;
+ size[1] = 256 * 4; /* palette is stored here as 256 32 bits words */
+ return size[0] + size[1];
}
for (i = 0; i < 4; i++)
@@ -136,7 +135,6 @@ int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int hei
total_size = size[0];
for (i = 1; i < 4 && has_plane[i]; i++) {
int h, s = (i == 1 || i == 2) ? desc->log2_chroma_h : 0;
- data[i] = data[i-1] + size[i-1];
h = (height + (1 << s) - 1) >> s;
if (linesizes[i] > INT_MAX / h)
return AVERROR(EINVAL);
@@ -149,6 +147,31 @@ int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int hei
return total_size;
}
+void av_image_fill_pointers_from_sizes(uint8_t *data[4], int size[4], uint8_t *ptr)
+{
+ int i;
+
+ memset(data , 0, sizeof(data[0])*4);
+
+ data[0] = ptr;
+ for (i = 1; i < 4 && size[i - 1] > 0; i++)
+ data[i] = data[i - 1] + size[i - 1];
+}
+
+int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int height,
+ uint8_t *ptr, const int linesizes[4]) {
+ int size[4];
+ int ret;
+
+ ret = av_image_fill_plane_sizes(size, pix_fmt, height, linesizes);
+ if (ret < 0)
+ return ret;
+
+ av_image_fill_pointers_from_sizes(data, size, ptr);
+
+ return ret;
+}
+
int avpriv_set_systematic_pal2(uint32_t pal[256], enum AVPixelFormat pix_fmt)
{
int i;
@@ -194,6 +217,7 @@ int av_image_alloc(uint8_t *pointers[4], int linesizes[4],
{
const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
int i, ret;
+ int size[4];
uint8_t *buf;
if (!desc)
@@ -207,19 +231,18 @@ int av_image_alloc(uint8_t *pointers[4], int linesizes[4],
for (i = 0; i < 4; i++)
linesizes[i] = FFALIGN(linesizes[i], align);
- if ((ret = av_image_fill_pointers(pointers, pix_fmt, h, NULL, linesizes)) < 0)
+ if ((ret = av_image_fill_plane_sizes(size, pix_fmt, h, linesizes)) < 0)
return ret;
buf = av_malloc(ret + align);
if (!buf)
return AVERROR(ENOMEM);
- if ((ret = av_image_fill_pointers(pointers, pix_fmt, h, buf, linesizes)) < 0) {
- av_free(buf);
- return ret;
- }
+ av_image_fill_pointers_from_sizes(pointers, size, buf);
+
if (desc->flags & AV_PIX_FMT_FLAG_PAL || (desc->flags & FF_PSEUDOPAL && pointers[1])) {
avpriv_set_systematic_pal2((uint32_t*)pointers[1], pix_fmt);
if (align < 4) {
av_log(NULL, AV_LOG_ERROR, "Formats with a palette require a minimum alignment of 4\n");
+ av_free(buf);
return AVERROR(EINVAL);
}
}
diff --git a/libavutil/imgutils.h b/libavutil/imgutils.h
index 5b790ecf0a..cbdef12928 100644
--- a/libavutil/imgutils.h
+++ b/libavutil/imgutils.h
@@ -67,6 +67,28 @@ int av_image_get_linesize(enum AVPixelFormat pix_fmt, int width, int plane);
*/
int av_image_fill_linesizes(int linesizes[4], enum AVPixelFormat pix_fmt, int width);
+/**
+ * Fill plane sizes for an image with pixel format pix_fmt and height height.
+ *
+ * @param size the array to be filled with the size of each image plane
+ * @param linesizes the array containing the linesize for each
+ * plane, should be filled by av_image_fill_linesizes()
+ * @return the size in bytes required for the image buffer, a negative
+ * error code in case of failure
+ */
+int av_image_fill_plane_sizes(int size[4], enum AVPixelFormat pix_fmt, int height,
+ const int linesizes[4]);
+
+/**
+ * Fill plane data pointers for an image with plane sizes size.
+ *
+ * @param data pointers array to be filled with the pointer for each image plane
+ * @param size the array containing the size of each plane, should be filled
+ * by av_image_fill_plane_sizes()
+ * @param ptr the pointer to a buffer which will contain the image
+ */
+void av_image_fill_pointers_from_sizes(uint8_t *data[4], int size[4], uint8_t *ptr);
+
/**
* Fill plane data pointers for an image with pixel format pix_fmt and
* height height.
--
2.27.0.212.ge8ba1cc988-goog
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
