[FFmpeg-devel] [PATCH v2 3/3] libavutil/imgutils: check for non-null buffer in av_image_fill_pointer

[James Almer]

On 7/8/2020 10:54 PM, Brian Kim wrote:
> Patch attached.
> 
> There was some discussion on the v1 thread on whether it was
> acceptable to break code that was relying on UB, so this patch will
> probably want to get delayed until a major version bump to avoid
> breaking places that were relying on av_image_fill_pointers()
> populating data when the input ptr is null

> From 2c269118523de0911f17a4b560b016c34fc3002f Mon Sep 17 00:00:00 2001
> From: Brian Kim <bkkim at google.com>
> Date: Tue, 7 Jul 2020 11:42:35 -0700
> Subject: [PATCH 3/3] libavutil/imgutils: check for non-null buffer in
>  av_image_fill_pointers
> 
> We were previously always filling data by adding offsets to ptr, which
> was undefined behavior when ptr was NULL.
> 
> Signed-off-by: Brian Kim <bkkim at google.com>
> ---
>  libavutil/imgutils.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> index 082229cfaf..3898c5e771 100644
> --- a/libavutil/imgutils.c
> +++ b/libavutil/imgutils.c
> @@ -155,6 +155,9 @@ int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int hei
>      ptrdiff_t ret, linesizes1[4];
>      size_t size[4];
>  
> +    if (!ptr)
> +        return AVERROR(EINVAL);

No, check !ptr immediately after the memset() call (to zero the data
array), and if it's NULL then return the value from
av_image_fill_plane_sizes().

No reason to break calls with ptr == NULL since calling it to get the
total size of the buffer you want to allocate is a valid scenario where
the contents of *data[4] are meaningless and thus ignored. You just want
to fix the UB of adding offsets to NULL.

This will still need to wait until a major bump nonetheless.

> +
>      for (i = 0; i < 4; i++)
>          linesizes1[i] = linesizes[i];
>  
> -- 
> 2.27.0.383.g050319c2ae-goog
>