Hi On Wed, Jul 01, 2020 at 11:14:13AM -0700, Brian Kim wrote: > 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?
There is certainly interrest in avoiding this
>
> 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
> libavcodec/decode.c | 11 +++--------
> libavutil/frame.c | 9 ++++-----
> libavutil/imgutils.c | 49 ++++++++++++++++++++++++++++++++++++-------------
> libavutil/imgutils.h | 22 ++++++++++++++++++++++
> 4 files changed, 65 insertions(+), 26 deletions(-)
> 1807613b16b290ccac0574586b42e13230dc824b
> 0001-libavutil-imgutils-add-utility-to-get-plane-sizes.patch
> 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 ++++++++++++++++++++
the changes to libavcodec and libavutil need to be in seperate patches
API additions in libavutil have to also bump the minor version and add
an entry in doc/APIchanges
[...]
> 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);
I wonder if linesizes for newly added functions should be ptrdiff_t
this would add some type converting loops though
And size probably should be ptrdiff_t or int64_t to similarly be more future
proof
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.
signature.asc
Description: PGP signature
_______________________________________________ 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".
