sws_scale() writes out of buffer on ssse3 YUV420P->BGRA conversion

[Sergei Trofimovich]

Summary of the bug:
sws_scale() corrupts memory of destination buffer and crashes the application.

Self-contained example extracted from vcmi project:

// $ cat a.c
#include <libswscale/swscale.h>
#include <libavcodec/avcodec.h> /* AV_INPUT_BUFFER_PADDING_SIZE */

// ffmpeg-4.4 swscale crash observed on an attempt to play real 200x116 YUV420P video.
//
// how to compile:
// $ x86_64-pc-linux-gnu-gcc -Wall -g $(pkg-config --cflags libswscale) a.c -o a $(pkg-config --libs libswscale) $(pkg-config --libs libavutil) && valgrind --quiet ./a

//
// result no sse3 in ffmpeg:
//   [swscaler @ 0x4eb2330] No accelerated colorspace conversion found from yuv420p to bgra.
//   <all ok>
//
// result with sse3 in ffmpeg:
// Invalid write of size 8
//    at 0x48EDE43: ??? (in /usr/lib64/libswscale.so.6.0.100)
//    by 0x48ECB26: yuv420_rgb32_ssse3 (yuv2rgb_template.c:119)
//    by 0x48C31D2: sws_scale (swscale.c:970)
//    by 0x10939E: main (a.c:68)
//  Address 0x4dbb140 is 0 bytes after a block of size 32 alloc'd
//    at 0x48449C0: memalign (vg_replace_malloc.c:1265)
//    by 0x4844B1E: posix_memalign (vg_replace_malloc.c:1429)
//    by 0x49383B4: av_malloc (mAem.c:86)
//    by 0x1092C5: main (a.c:49)

typedef unsigned char u8;
typedef unsigned long u64;

int main() {
    // 200x116 YUV420P pic
    //static const u64 w = 200, h = 116;

    // simplified test example:
    static const u64 w = 4, h = 2;

    static const u64 yuv_bpp = 12, bgra_bpp = 32;

    u8 * yuv_pic  = malloc (w * h * yuv_bpp / 8 + AV_INPUT_BUFFER_PADDING_SIZE /* avoid OOB reads */);
    u8 * yp       = yuv_pic;
    u64  yl       = w;

    u8 * up       = yp + yl * h;
    u64  ul       = w / 4;

    u8 * vp       = up + ul * h;
    u64  vl       = w / 4;

    // plausibly looking data
    memset(yuv_pic, '!', w * h * yuv_bpp / 8);

    //u8 * bgra_pic = malloc(w * h * bgra_bpp / 8);
    u8 * bgra_pic = av_malloc(w * h * bgra_bpp / 8);
    u64  bgral    = w * bgra_bpp / 8;

    struct SwsContext * sws = sws_getContext(
        // src:
        w, h, AV_PIX_FMT_YUV420P,
        // dst:
        w, h, AV_PIX_FMT_BGRA,
        // opts:
        SWS_BICUBIC,
        // filters:
        NULL, NULL, NULL);

    const u8 * src_planes[] = { yp, up, vp, };
    const int  src_slices[] = { yl, ul, vl, };

    u8 * dst_planes[] = { bgra_pic, };
    int  dst_slices[] = { bgral,    };

    sws_scale(
        sws,
        // src:
        src_planes, src_slices,
        // opts:
        0,
        // dst:
        h,
        dst_planes,
        dst_slices);

    sws_freeContext(sws);
    free(bgra_pic);
    free(yuv_pic);

    return 0;
}

How to reproduce:

$ x86_64-pc-linux-gnu-gcc -Wall -g $(pkg-config --cflags libswscale) a.c -o a $(pkg-config --libs libswscale) $(pkg-config --libs libavutil) && valgrind --quiet ./a
==3571664== Invalid write of size 8
==3571664==    at 0x48EF083: ??? (in /usr/lib64/libswscale.so.5.9.100)
==3571664==    by 0x48EDD66: yuv420_rgb32_ssse3 (yuv2rgb_template.c:119)
==3571664==    by 0x48C41D2: sws_scale (swscale.c:970)
==3571664==    by 0x10939E: main (a.c:71)
==3571664==  Address 0x4eb1140 is 0 bytes after a block of size 32 alloc'd
==3571664==    at 0x48449C0: memalign (vg_replace_malloc.c:1265)
==3571664==    by 0x4844B1E: posix_memalign (vg_replace_malloc.c:1429)
==3571664==    by 0x493A714: av_malloc (mem.c:86)
==3571664==    by 0x1092C5: main (a.c:52)
==3571664==
==3571664== Invalid write of size 8
==3571664==    at 0x48EF088: ??? (in /usr/lib64/libswscale.so.5.9.100)
==3571664==    by 0x48EDD66: yuv420_rgb32_ssse3 (yuv2rgb_template.c:119)
==3571664==    by 0x48C41D2: sws_scale (swscale.c:970)
==3571664==    by 0x10939E: main (a.c:71)
==3571664==  Address 0x4eb1150 is 16 bytes after a block of size 32 alloc'd
==3571664==    at 0x48449C0: memalign (vg_replace_malloc.c:1265)
==3571664==    by 0x4844B1E: posix_memalign (vg_replace_malloc.c:1429)
==3571664==    by 0x493A714: av_malloc (mem.c:86)
==3571664==    by 0x1092C5: main (a.c:52)
==3571664==
==3571664== Invalid write of size 8
==3571664==    at 0x48EF07E: ??? (in /usr/lib64/libswscale.so.5.9.100)
==3571664==    by 0x48EDD66: yuv420_rgb32_ssse3 (yuv2rgb_template.c:119)
==3571664==    by 0x48C41D2: sws_scale (swscale.c:970)
==3571664==    by 0x10939E: main (a.c:71)
==3571664==  Address 0x4eb1140 is 0 bytes after a block of size 32 alloc'd
==3571664==    at 0x48449C0: memalign (vg_replace_malloc.c:1265)
==3571664==    by 0x4844B1E: posix_memalign (vg_replace_malloc.c:1429)
==3571664==    by 0x493A714: av_malloc (mem.c:86)
==3571664==    by 0x1092C5: main (a.c:52)
==3571664==

$ ./a
free(): invalid next size (fast)
Aborted (core dumped)

Note: the example crashes with both valgrind enabled and disabled which suggests it's a real corruption.

This is a ffmpeg-4.4 system with SSSE3 support (ffmpeg from git behaves the same).

A few observations (== bugs):

1. ffmpeg-4.4 built without SSSE3 does not overwrite past end of buffer. Would be nice SSSE3/non-SSSE3 bahave the same here: either both wrote past expected buffer end to expose the same expectations by ffmpeg or both did not overwrite.

2. sws_scale() also reads past input buffer (hence the need for AV_INPUT_BUFFER_PADDING_SIZE). Documentation line for sws_scale() should lention alignment and padding requirements. Now it does not seem to: ​https://lists.ffmpeg.org/doxygen/2.8/group__libsws.html#gae531c9754c9205d90ad6800015046d74

3. While at it glancing at ​https://git.ffmpeg.org/gitweb/ffmpeg.git/blob_plain/HEAD:/libswscale/x86/yuv_2_rgb.asm parameters are not always set:

%macro yuv2rgb_fn 3

%if %3 == 32
    %ifidn %1, yuva
    %define parameters index, image, pu_index, pv_index, pointer_c_dither, py_2index, pa_2index
    %define GPR_num 7
    %endif
    ;;; 
    ;;; How about the %else part? Does it not leave parameters from previous call?
    ;;;
%else
    %define parameters index, image, pu_index, pv_index, pointer_c_dither, py_2index
    %define GPR_num 6
%endif

Thank you!

[bmegli]

I am getting similar crash in n4.4 source build

• with YUV420P -> RGB24 color conversion, 1932x1096 resolution
• FWIW, the input from H264 decoder has padding (e.g. linesize[0] == 2048)

A quick workaround is to add some padding to output buffer.

AV_INPUT_BUFFER_PADDING_SIZE seems to work but whether it is safe would require diving into code.