[FFmpeg-devel] [PATCH 3/3] vp9: recon: Use emulated edge to prevent buffer overflows
Ronald S. Bultje
rsbultje at gmail.com
Sat Dec 21 19:25:02 EET 2024
Hi Janne,
On Thu, Dec 19, 2024 at 4:12 PM Janne Grunau <janne-ffmpeg at jannau.net>
wrote:
> The arm/aarch64 horizontal filter reads one additional pixel beyond what
> the filter uses. This can become an issue if the application does not
> allocate larger buffers than what's required for the pixel data. If the
> motion vector points to the bottom right edge of the picture this
> becomes a read buffer overflow. This triggers segfaults in Firefox for
> video resolutions which result in a page aligned picture size like
> 1280x640.
> Prevent this by using emulated edge in this case.
>
> Fixes: https://bugzilla.mozilla.org/show_bug.cgi?id=1881185
> Signed-off-by: Janne Grunau <janne-ffmpeg at jannau.net>
> ---
> libavcodec/vp9recon.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/libavcodec/vp9recon.c b/libavcodec/vp9recon.c
> index ef08ed17c8e5..1f1b99a03c8a 100644
> --- a/libavcodec/vp9recon.c
> +++ b/libavcodec/vp9recon.c
> @@ -319,7 +319,11 @@ static av_always_inline void
> mc_luma_unscaled(VP9TileData *td, const vp9_mc_func
> // The arm/aarch64 _hv filters read one more row than what actually is
> // needed, so switch to emulated edge one pixel sooner vertically
> // (!!my * 5) than horizontally (!!mx * 4).
> + // The arm/aarch64 _h filters read one more pixel than what actually
> is
> + // needed, so switch to emulated edge if that would read beyond the
> bottom
> + // right block.
> if (x < !!mx * 3 || y < !!my * 3 ||
> + ((x + !!mx * 5 > w - bw) && (y + !!my * 5 + 1 > h - bh)) ||
> x + !!mx * 4 > w - bw || y + !!my * 5 > h - bh) {
> s->vdsp.emulated_edge_mc(td->edge_emu_buffer,
> ref - !!my * 3 * ref_stride - !!mx * 3 *
> bytesperpixel,
>
Short-term, this is probably a good idea. No objection from me under #if
ARCH_.
Long-term, do we want to update the docs for get_buffer()? I realize this
is an ABI change and probably needs a new function name or at least a major
bump. But my impression was always that the over-allocation was intended
(same as for input buffers) so SIMD could overread by a few bytes from
refs... If it is intentional that we don't over-read (which is fine with me
also), we probably want to fix the SIMD, that seems like it would be more
efficient and simpler.
Ronald
More information about the ffmpeg-devel
mailing list