From patchwork Thu Jul 9 01:50:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Kim X-Patchwork-Id: 20903 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 6A24244AAD9 for ; Thu, 9 Jul 2020 04:50:47 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 48D3D68B467; Thu, 9 Jul 2020 04:50:47 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-ed1-f48.google.com (mail-ed1-f48.google.com [209.85.208.48]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id BD7AB6880F7 for ; Thu, 9 Jul 2020 04:50:40 +0300 (EEST) Received: by mail-ed1-f48.google.com with SMTP id a8so553064edy.1 for ; Wed, 08 Jul 2020 18:50:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:from:date:message-id:subject:to; bh=pBQwd8DfmIL0x+XQqXMUS4IbO7iuaM+vujSSEYCrNO0=; b=KPNPYvJs3gzFyIeeQHQTnnOb8g8c/OlziW7oPHiAKv0dHqvBrc1+ZEgXqnAAZFl/zc 46EMeEM/M7pkDE9rSFGLQ9x/yW24F45+ic6NZP8YQsW6PQxez+dF3f/CQb4gcEz3aiPf gTy5E4FBUsT97o18qIKb7p9p8h4nKFSkq9WucLcEyxEbPc58seHGJIJtkix3TRpL9vIW C4pxk41neg2RKluPjOloCcaP4s3OOJrn2xXM/l5/yc4fxuKYKx5ylQYvwb9lmfmjDgz4 LcMwCGjHbTSGifaafXsI8zE7Jgb3AAB+W9yDFMs4Pnx2hSkQviovXqe+jLcrqS1YNUAn ndWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to; bh=pBQwd8DfmIL0x+XQqXMUS4IbO7iuaM+vujSSEYCrNO0=; b=BserzwF5yoLvMiPwX0OWVzr1kIsgXrbmzcATu+C1RaLb8aIGI1U+DJw2/YxukQ9u+l LtgiwUbvUcLzpelLoRTORnUZ4RjfdD9pvxBHOHXTz8CEXN8+VxXgd1msR3mICZ+kcMSL V5T/0YA4giBp8cbj8yDd2ELiZFvqO9OMjGFfhrmsNPEIB5sEjLgof9WDZY4MGoHmTxNV 82/TBvWl2dwmnT8WjsFhNRaXl39wF9LOwSEk2Nb5vvXj1XzwORjRtVpzRnqbucpjWF9D zw+d8gTy2DCR8VKkv4gHwCBdgUH8CIX/SckkNU4hBwKqojuslTCA4YL2bL201Otjm27q zOFA== X-Gm-Message-State: AOAM531bX103KQ/tTFM51IsA23aupDk3twv6gfoG3Fj7IGXYH2kn6lhU 372b6tUfMov9xMwwwjDqtp39ZV4wf+9Psi0tjzkXZY5Hbio= X-Google-Smtp-Source: ABdhPJwqVeJeEmzymvaHek7jbM2GanMwUPQtoEQk+0EsxGF0K4g9DtjlLIC93wOgFrHGEMwXuT41NUXb2EMXSztAinA= X-Received: by 2002:a05:6402:17ee:: with SMTP id t14mr68040693edy.359.1594259439426; Wed, 08 Jul 2020 18:50:39 -0700 (PDT) MIME-Version: 1.0 From: Brian Kim Date: Wed, 8 Jul 2020 18:50:28 -0700 Message-ID: To: FFmpeg development discussions and patches Subject: [FFmpeg-devel] [PATCH v2 1/3] libavutil/imgutils: add utility to get plane sizes X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Patch attached. Main changes from v1 are switching to from int to size_t/ptrdiff_t where relevant and removing av_image_fill_pointers_from_sizes() Some things to note: - The av_image_fill_planes_sizes uses size_t/ptrdiff_t for buffer sizes, but as mentioned during the v1 review, this leads to some inconvenient conversions and range checks when using it with existing functions. We could keep the return type as int to alleviate this, but that seems like it would somewhat defeat the purpose of moving to these types. - SIZE_MAX is used in several places in libavutil, so I used PTRDIFF_MAX, but I could not see any mention of these being allowed. Subject: [PATCH 1/3] 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. Signed-off-by: Brian Kim --- doc/APIchanges | 3 ++ libavutil/frame.c | 15 ++++++--- libavutil/imgutils.c | 74 ++++++++++++++++++++++++++++++++------------ libavutil/imgutils.h | 12 +++++++ libavutil/version.h | 2 +- 5 files changed, 82 insertions(+), 24 deletions(-) diff --git a/doc/APIchanges b/doc/APIchanges index 1d6cc36b8c..44defd9ca8 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,9 @@ libavutil: 2017-10-21 API changes, most recent first: +2020-07-xx - xxxxxxxxxx - lavu 56.56.100 - imgutils.h + Add av_image_fill_plane_sizes(). + 2020-06-12 - b09fb030c1 - lavu 56.55.100 - pixdesc.h Add AV_PIX_FMT_X2RGB10. diff --git a/libavutil/frame.c b/libavutil/frame.c index 9884eae054..b664dcfbe8 100644 --- a/libavutil/frame.c +++ b/libavutil/frame.c @@ -214,6 +214,8 @@ static int get_video_buffer(AVFrame *frame, int align) const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format); int ret, i, padded_height; int plane_padding = FFMAX(16 + 16/*STRIDE_ALIGN*/, align); + ptrdiff_t total_size, linesizes[4]; + size_t size[4]; if (!desc) return AVERROR(EINVAL); @@ -238,12 +240,17 @@ static int get_video_buffer(AVFrame *frame, int align) frame->linesize[i] = FFALIGN(frame->linesize[i], align); } + for (i = 0; i < 4; i++) + linesizes[i] = frame->linesize[i]; + padded_height = FFALIGN(frame->height, 32); - if ((ret = av_image_fill_pointers(frame->data, frame->format, padded_height, - NULL, frame->linesize)) < 0) - return ret; + if ((total_size = av_image_fill_plane_sizes(size, frame->format, padded_height, + linesizes)) < 0) + return total_size; + if (total_size > INT_MAX - 4*plane_padding) + return AVERROR(EINVAL); - frame->buf[0] = av_buffer_alloc(ret + 4*plane_padding); + frame->buf[0] = av_buffer_alloc(total_size + 4*plane_padding); if (!frame->buf[0]) { ret = AVERROR(ENOMEM); goto fail; diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c index 7f9c1b632c..082229cfaf 100644 --- a/libavutil/imgutils.c +++ b/libavutil/imgutils.c @@ -108,26 +108,26 @@ 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]) +ptrdiff_t av_image_fill_plane_sizes(size_t size[4], enum AVPixelFormat pix_fmt, + int height, const ptrdiff_t linesizes[4]) { - int i, total_size, size[4] = { 0 }, has_plane[4] = { 0 }; + int i, has_plane[4] = { 0 }; + ptrdiff_t total_size; 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) + if (linesizes[0] > (PTRDIFF_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,12 +136,11 @@ 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) + if (linesizes[i] > PTRDIFF_MAX / h) return AVERROR(EINVAL); size[i] = h * linesizes[i]; - if (total_size > INT_MAX - size[i]) + if (total_size > PTRDIFF_MAX - size[i]) return AVERROR(EINVAL); total_size += size[i]; } @@ -149,6 +148,31 @@ int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int hei return total_size; } +int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int height, + uint8_t *ptr, const int linesizes[4]) +{ + int i; + ptrdiff_t ret, linesizes1[4]; + size_t size[4]; + + for (i = 0; i < 4; i++) + linesizes1[i] = linesizes[i]; + + ret = av_image_fill_plane_sizes(size, pix_fmt, height, linesizes1); + if (ret < 0) + return ret; + if (ret > INT_MAX) + return AVERROR(EINVAL); + + 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]; + + return ret; +} + int avpriv_set_systematic_pal2(uint32_t pal[256], enum AVPixelFormat pix_fmt) { int i; @@ -194,6 +218,8 @@ int av_image_alloc(uint8_t *pointers[4], int linesizes[4], { const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt); int i, ret; + ptrdiff_t total_size, linesizes1[4]; + size_t size[4]; uint8_t *buf; if (!desc) @@ -204,12 +230,14 @@ int av_image_alloc(uint8_t *pointers[4], int linesizes[4], if ((ret = av_image_fill_linesizes(linesizes, pix_fmt, align>7 ? FFALIGN(w, 8) : w)) < 0) return ret; - for (i = 0; i < 4; i++) + for (i = 0; i < 4; i++) { linesizes[i] = FFALIGN(linesizes[i], align); + linesizes1[i] = linesizes[i]; + } - if ((ret = av_image_fill_pointers(pointers, pix_fmt, h, NULL, linesizes)) < 0) - return ret; - buf = av_malloc(ret + align); + if ((total_size = av_image_fill_plane_sizes(size, pix_fmt, h, linesizes1)) < 0) + return total_size; + buf = av_malloc(total_size + align); if (!buf) return AVERROR(ENOMEM); if ((ret = av_image_fill_pointers(pointers, pix_fmt, h, buf, linesizes)) < 0) { @@ -220,6 +248,7 @@ int av_image_alloc(uint8_t *pointers[4], int linesizes[4], 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); } } @@ -431,9 +460,10 @@ int av_image_fill_arrays(uint8_t *dst_data[4], int dst_linesize[4], int av_image_get_buffer_size(enum AVPixelFormat pix_fmt, int width, int height, int align) { - uint8_t *data[4]; + int ret, i; int linesize[4]; - int ret; + ptrdiff_t aligned_linesize[4]; + size_t size[4]; const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt); if (!desc) return AVERROR(EINVAL); @@ -446,8 +476,14 @@ int av_image_get_buffer_size(enum AVPixelFormat pix_fmt, if (desc->flags & FF_PSEUDOPAL) return FFALIGN(width, align) * height; - return av_image_fill_arrays(data, linesize, NULL, pix_fmt, - width, height, align); + ret = av_image_fill_linesizes(linesize, pix_fmt, width); + if (ret < 0) + return ret; + + for (i = 0; i < 4; i++) + aligned_linesize[i] = FFALIGN(linesize[i], align); + + return av_image_fill_plane_sizes(size, pix_fmt, height, aligned_linesize); } int av_image_copy_to_buffer(uint8_t *dst, int dst_size, diff --git a/libavutil/imgutils.h b/libavutil/imgutils.h index 5b790ecf0a..99772357ae 100644 --- a/libavutil/imgutils.h +++ b/libavutil/imgutils.h @@ -67,6 +67,18 @@ 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 + */ +ptrdiff_t av_image_fill_plane_sizes(size_t size[4], enum AVPixelFormat pix_fmt, + int height, const ptrdiff_t linesizes[4]); + /** * Fill plane data pointers for an image with pixel format pix_fmt and * height height. diff --git a/libavutil/version.h b/libavutil/version.h index 3ce9b1831e..a63f79feb1 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -79,7 +79,7 @@ */ #define LIBAVUTIL_VERSION_MAJOR 56 -#define LIBAVUTIL_VERSION_MINOR 55 +#define LIBAVUTIL_VERSION_MINOR 56 #define LIBAVUTIL_VERSION_MICRO 100 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ From patchwork Thu Jul 9 01:51:49 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Kim X-Patchwork-Id: 20904 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 2BC69449579 for ; Thu, 9 Jul 2020 04:58:13 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0C15268B504; Thu, 9 Jul 2020 04:58:13 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-lj1-f175.google.com (mail-lj1-f175.google.com [209.85.208.175]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 8C9E06881A5 for ; Thu, 9 Jul 2020 04:58:06 +0300 (EEST) Received: by mail-lj1-f175.google.com with SMTP id f5so495968ljj.10 for ; Wed, 08 Jul 2020 18:58:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=ZFCvR7iRJ5KEdoeS+BVaiHzdxmTq8vIgrTlBrosQnis=; b=aXvkYgkSBR8jUI0cUtDMHAb+oT18+tpDJlw+C1TnAdASZ8JvCZi50NkthZ/n7IEgW6 ebRO9glRpplEDSIe3Q7whu/oyBz7g8sSKI27pfliDYiZneTDb3dFmIC9+uYe0ZI2zAk7 qGfZ5DWri9rS85YSRT/La5bO3nAHiL8w2pKUZx7mmwizaIHFTV16zv/XT+oeU+rfW2/S ttN018Oq/6j95aFyMKcvewXoFz++JUv+iBdqbVLhVuJ5h8NC+SwDWiezHg0vGQcj047m pBKTP40PBMNqk2n/2lliZCVGVrMwyACAawt7qE8HVSV7jOP1CJxYxoNaJuzrRQjIYWlH Jrcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=ZFCvR7iRJ5KEdoeS+BVaiHzdxmTq8vIgrTlBrosQnis=; b=L/7OWRlJYM3Qw5W4fDdAUDhxBxAvmI2yHlEHMx2uquBqrvnro5+khyctr8QIfy1jy0 Bw3HowjYfqVmdtMDSaofPGysSlpAeohpKUMzFruVkYw4dKuraOLcEvGMScrO9K72lsBr HIVMKRRaCTJ9/2iNM32y0Y4Giun0YB5qcshsEnKVrwVswB+aDPWFQkrG/8U9RhUpsBf7 QGksVToLHxqH8y5PRFMwAjVBKN0N0t3aHNQqgYiXeUcW1p189ZVynVNj4fYBuojDsluw pLun/cHI2X0i8eGLRcMnED6eYO57ML08D7kO1KK2IAZNfYl8NgIrhszJ20Pe7GtrPJ6T VCHg== X-Gm-Message-State: AOAM531spcCPYPzNEeslKQW/0nI6JcvL7+KNnJisDRuUWYnbSc0XttXj iz6nUdvmRRLPXiHVzNZR3hJWFMku8qhyVk+2VgdPm9CP72w= X-Google-Smtp-Source: ABdhPJzORa94Mmus0DwShonxOAyoFCVZAxaAlM71sMXecwddDCgSapHxmG9Oyp39k6dvTbglNlM7F0rd1sfkct1z+L4= X-Received: by 2002:a17:907:20ba:: with SMTP id pw26mr53039632ejb.425.1594259519982; Wed, 08 Jul 2020 18:51:59 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Brian Kim Date: Wed, 8 Jul 2020 18:51:49 -0700 Message-ID: To: FFmpeg development discussions and patches Subject: [FFmpeg-devel] [PATCH v2 2/3] libavcodec/decode: avoid UB when getting plane sizes X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Patch attached Subject: [PATCH 2/3] libavcodec/decode: avoid UB when getting plane sizes This uses av_image_fill_plane_sizes instead of av_image_fill_pointers when we are getting plane sizes to avoid UB from adding offsets to NULL. Signed-off-by: Brian Kim --- libavcodec/decode.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/libavcodec/decode.c b/libavcodec/decode.c index de9c079f9d..c1753cfffb 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -1471,12 +1471,12 @@ 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 w = frame->width; int h = frame->height; - int tmpsize, unaligned; + int unaligned; + ptrdiff_t tmpsize, linesize1[4]; + size_t size[4]; avcodec_align_dimensions2(avctx, &w, &h, pool->stride_align); @@ -1494,20 +1494,22 @@ 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); + for (i = 0; i < 4; i++) + linesize1[i] = linesize[i]; + tmpsize = av_image_fill_plane_sizes(size, avctx->pix_fmt, h, + linesize1); 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]) { + if (size[i] > INT_MAX - (16 + STRIDE_ALIGN - 1)) { + ret = AVERROR(EINVAL); + goto fail; + } pool->pools[i] = av_buffer_pool_init(size[i] + 16 + STRIDE_ALIGN - 1, CONFIG_MEMORY_POISONING ? NULL :