[FFmpeg-devel] [PATCH v2 3/4] checkasm/sw_rgb: add tests for yuv2packed{1, 2, X}

[Andreas Rheinhardt]

Michael Niedermayer:
> On Fri, Dec 20, 2024 at 01:54:30PM +0100, Niklas Haas wrote:
>> From: Niklas Haas <git at haasn.dev>
>>
>> Signed-off-by: Niklas Haas <git at haasn.dev>
>> Sponsored-by: Sovereign Tech Fund
>> ---
>>  tests/checkasm/sw_rgb.c | 308 ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 308 insertions(+)
> 
> This seems to have some issue: (the code in git) if run under asan, it works fine otherwise
> 
> TEST    checkasm-sw_rgb
> ./tests/fate-run.sh fate-checkasm-sw_rgb "" "" "ffmpeg" 'run tests/checkasm/checkasm --test=sw_rgb' 'null' '' '' '1' '' '' '' '' '' '' '' '' '' ''
>  tests/checkasm/checkasm --test=sw_rgb
> Test checkasm-sw_rgb failed. Look at tests/data/fate/checkasm-sw_rgb.err for details.
> checkasm: using random seed 486148533
> =================================================================
> ==3872252==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff71741d40 at pc 0x00000073c0d8 bp 0x7fff71740b70 sp 0x7fff71740b68
> READ of size 4 at 0x7fff71741d40 thread T0
>     #0 0x73c0d7 in check_yuv2packed1 tests/checkasm/sw_rgb.c:530:31
>     #1 0x73346e in checkasm_check_sw_rgb tests/checkasm/sw_rgb.c:802:5
>     #2 0x677b1c in check_cpu_flag tests/checkasm/checkasm.c:823:13
>     #3 0x67772d in main tests/checkasm/checkasm.c:1009:5
>     #4 0x7fc6dee7f082 in __libc_start_main /build/glibc-LcI20x/glibc-2.31/csu/../csu/libc-start.c:308:16
>     #5 0x5b4ead in _start (tests/checkasm/checkasm+0x5b4ead)
> 
> Address 0x7fff71741d40 is located in stack of thread T0 at offset 4320 in frame
>     #0 0x73ae1f in check_yuv2packed1 tests/checkasm/sw_rgb.c:494
> 
>   This frame has 8 object(s):
>     [32, 48) 'chru' (line 507)
>     [64, 80) 'chrv' (line 508)
>     [96, 2144) 'la_src_y' (line 511)
>     [2272, 4320) 'la_src_u' (line 512) <== Memory access at offset 4320 overflows this variable
>     [4448, 6496) 'la_src_v' (line 513)
>     [6624, 8672) 'la_src_a' (line 514)
>     [8800, 16992) 'la_dst0' (line 516)
>     [17248, 25440) 'la_dst1' (line 517)
> HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
>       (longjmp and C++ exceptions *are* supported)
> SUMMARY: AddressSanitizer: stack-buffer-overflow tests/checkasm/sw_rgb.c:530:31 in check_yuv2packed1
> Shadow bytes around the buggy address:
>   0x10006e2e0350: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x10006e2e0360: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x10006e2e0370: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x10006e2e0380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x10006e2e0390: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> =>0x10006e2e03a0: 00 00 00 00 00 00 00 00[f2]f2 f2 f2 f2 f2 f2 f2
>   0x10006e2e03b0: f2 f2 f2 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00
>   0x10006e2e03c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x10006e2e03d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x10006e2e03e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x10006e2e03f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> Shadow byte legend (one shadow byte represents 8 application bytes):
>   Addressable:           00
>   Partially addressable: 01 02 03 04 05 06 07
>   Heap left redzone:       fa
>   Freed heap region:       fd
>   Stack left redzone:      f1
>   Stack mid redzone:       f2
>   Stack right redzone:     f3
>   Stack after return:      f5
>   Stack use after scope:   f8
>   Global redzone:          f9
>   Global init order:       f6
>   Poisoned by user:        f7
>   Container overflow:      fc
>   Array cookie:            ac
>   Intra object redzone:    bb
>   ASan internal:           fe
>   Left alloca redzone:     ca
>   Right alloca redzone:    cb
>   Shadow gap:              cc
> ==3872252==ABORTING
> threads=1
> make: *** [tests/Makefile:311: fate-checkasm-sw_rgb] Error 1
> 
> 
To quote myself from IRC:
"<mkver> haasn: This is wrong:
https://github.com/FFmpeg/FFmpeg/blob/master/tests/checkasm/sw_rgb.c#L579-L580
<mkver> If one uses (int16_t *)src_u + i*INPUT_SIZE, the stack-buffer
overflow is fixed; but I don't get why you use int32_t for data that is
only treated as int16_t later (this is a effective-type violation and
therefore UB).
<mkver> Is it because it simplified initialization?"

- Andreas