[FFmpeg-devel] [PATCH] lavfi/zscale: Fix unaligned data ptr issues in realign_frame

[Pavel Koshevoy]

I ran into segfaults in zimg when I attempted to use zscale
to convert a 512x538 at yuv444p16le(tv) image from HLG to Bt.709
with this filter chain:

buffer=width=512:height=538:pix_fmt=yuv444p16le:time_base=1/1:sar=1/1,zscale=min=2020_ncl:rin=limited:pin=2020:tin=arib-std-b67:cin=left:t=linear,format=gbrpf32le,tonemap=gamma:desat=0,zscale=tin=linear:npl=100:p=709:m=709:r=limited:t=709,format=pix_fmts=yuv444p16le,buffersink

I found several issues:
- realign_frame called av_pix_fmt_count_planes with incorrect parameter.
- av_frame_get_buffer did not align data pointers to specified alignment.
- s->tmp[job_nr] pointer was also unaligned.

https://github.com/sekrit-twc/zimg/issues/212
---
 libavfilter/vf_zscale.c | 17 ++++++++++++-----
 libavutil/frame.c       |  8 +++++++-
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 4ba059064b..a74a3da8d9 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -112,6 +112,7 @@ typedef struct ZScaleContext {
     int force_original_aspect_ratio;
 
     void *tmp[MAX_THREADS]; //separate for each thread;
+    void *tmp_aligned[MAX_THREADS];
     int nb_threads;
     int jobs_ret[MAX_THREADS];
     double in_slice_start[MAX_THREADS];
@@ -594,6 +595,7 @@ static int graphs_build(AVFrame *in, AVFrame *out, const AVPixFmtDescriptor *des
 {
     ZScaleContext *s = ctx->priv;
     int ret;
+    int misaligned_tmp;
     size_t size;
     zimg_image_format src_format;
     zimg_image_format dst_format;
@@ -628,11 +630,15 @@ static int graphs_build(AVFrame *in, AVFrame *out, const AVPixFmtDescriptor *des
     if (ret)
         return print_zimg_error(ctx);
 
-    if (s->tmp[job_nr])
+    if (s->tmp[job_nr]) {
         av_freep(&s->tmp[job_nr]);
-    s->tmp[job_nr] = av_calloc(size, 1);
+        s->tmp_aligned[job_nr] = NULL;
+    }
+    s->tmp[job_nr] = av_calloc(size + ZIMG_ALIGNMENT - 1, 1);
     if (!s->tmp[job_nr])
         return AVERROR(ENOMEM);
+    misaligned_tmp = ((uintptr_t)(s->tmp[job_nr])) % ZIMG_ALIGNMENT;
+    s->tmp_aligned[job_nr] = s->tmp[job_nr] + (misaligned_tmp ? (ZIMG_ALIGNMENT - misaligned_tmp) : 0);
 
     if (desc->flags & AV_PIX_FMT_FLAG_ALPHA && out_desc->flags & AV_PIX_FMT_FLAG_ALPHA) {
         alpha_src_format = s->alpha_src_format;
@@ -663,7 +669,7 @@ static int realign_frame(const AVPixFmtDescriptor *desc, AVFrame **frame, int ne
     int ret = 0, plane, planes;
 
     /* Realign any unaligned input frame. */
-    planes = av_pix_fmt_count_planes(desc->nb_components);
+    planes = av_pix_fmt_count_planes((*frame)->format);
     for (plane = 0; plane < planes; plane++) {
         int p = desc->comp[plane].plane;
         if ((uintptr_t)(*frame)->data[p] % ZIMG_ALIGNMENT || (*frame)->linesize[p] % ZIMG_ALIGNMENT) {
@@ -750,7 +756,7 @@ static int filter_slice(AVFilterContext *ctx, void *data, int job_nr, int n_jobs
     }
     if (!s->graph[job_nr])
         return AVERROR(EINVAL);
-    ret = zimg_filter_graph_process(s->graph[job_nr], &src_buf, &dst_buf, s->tmp[job_nr], 0, 0, 0, 0);
+    ret = zimg_filter_graph_process(s->graph[job_nr], &src_buf, &dst_buf, s->tmp_aligned[job_nr], 0, 0, 0, 0);
     if (ret)
         return print_zimg_error(ctx);
 
@@ -765,7 +771,7 @@ static int filter_slice(AVFilterContext *ctx, void *data, int job_nr, int n_jobs
 
         if (!s->alpha_graph[job_nr])
             return AVERROR(EINVAL);
-        ret = zimg_filter_graph_process(s->alpha_graph[job_nr], &src_buf, &dst_buf, s->tmp[job_nr], 0, 0, 0, 0);
+        ret = zimg_filter_graph_process(s->alpha_graph[job_nr], &src_buf, &dst_buf, s->tmp_aligned[job_nr], 0, 0, 0, 0);
         if (ret)
             return print_zimg_error(ctx);
     }
@@ -944,6 +950,7 @@ static av_cold void uninit(AVFilterContext *ctx)
 
     for (int i = 0; i < s->nb_threads; i++) {
         av_freep(&s->tmp[i]);
+        s->tmp_aligned[i] = NULL;
         if (s->graph[i]) {
             zimg_filter_graph_free(s->graph[i]);
             s->graph[i] = NULL;
diff --git a/libavutil/frame.c b/libavutil/frame.c
index f0a0dba018..c99d1fd9d9 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -175,8 +175,10 @@ static int get_video_buffer(AVFrame *frame, int align)
     const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format);
     int ret, padded_height, total_size;
     int plane_padding = FFMAX(16 + 16/*STRIDE_ALIGN*/, align);
+    int misaligned;
     ptrdiff_t linesizes[4];
     size_t sizes[4];
+    uint8_t *aligned_ptr;
 
     if (!desc)
         return AVERROR(EINVAL);
@@ -216,14 +218,18 @@ static int get_video_buffer(AVFrame *frame, int align)
         total_size += sizes[i];
     }
 
+    total_size += align - 1;
     frame->buf[0] = av_buffer_alloc(total_size);
     if (!frame->buf[0]) {
         ret = AVERROR(ENOMEM);
         goto fail;
     }
 
+    misaligned = ((uintptr_t)(frame->buf[0]->data)) % align;
+    aligned_ptr = frame->buf[0]->data + (misaligned ? (align - misaligned) : 0);
+
     if ((ret = av_image_fill_pointers(frame->data, frame->format, padded_height,
-                                      frame->buf[0]->data, frame->linesize)) < 0)
+                                      aligned_ptr, frame->linesize)) < 0)
         goto fail;
 
     for (int i = 1; i < 4; i++) {
-- 
2.43.0