[ffmpeg vaapi][DdiDecodeHEVCG11::AllocSliceControlBuffer] realloc() invalid next size

[fulinjie]

Driver reports error if we extend the slice parameter buffer, and set the slice parameter buffer size to sizeof VASliceParameterBufferHEVCExtension when decoding HEVC Main 10 clips.

PR in FFmpeg:
intel-media-ci/ffmpeg#163

https://github.com/fulinjie/ffmpeg/blob/pr-vaapi_y210le/libavcodec/vaapi_hevc.c#L403

CMD:
ffmpeg -hwaccel vaapi -i Allegro_HEVC_Main10_HT51_PROCESSING_12-CTB32_192x200@60Hz_1.0.bin -vframes 10 -y vaapi.yuv
(clips could be shared if required for debug)

Error happens while decoding a HEVC MAIN10 clips if assign:
slice_param_size = sizeof(VASliceParameterBufferHEVCExtension)

Error:
realloc(): invalid next size
Aborted (core dumped)

call stack:
Thread 8 "ffmpeg_g" received signal SIGABRT, Aborted.
[Switching to Thread 0x7fffe9ad5700 (LWP 13269)]
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
51 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007ffff3eb8801 in __GI_abort () at abort.c:79
#2 0x00007ffff3f01897 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7ffff402eb9a "%s\n") at ../sysdeps/posix/libc_fatal.c:181
#3 0x00007ffff3f0890a in malloc_printerr (str=str@entry=0x7ffff402ced3 "realloc(): invalid next size") at malloc.c:5350
#4 0x00007ffff3f0d9b4 in _int_realloc (av=av@entry=0x7fffe4000020, oldp=oldp@entry=0x7fffe40fb950, oldsize=oldsize@entry=1856, nb=nb@entry=2128) at malloc.c:4534
#5 0x00007ffff3f10f9b in __GI___libc_realloc (oldmem=0x7fffe40fb960, bytes=2112) at malloc.c:3230
#6 0x00007fffee2eb880 in DdiDecodeHEVCG11::AllocSliceControlBuffer (this=0x5555558fb570, buf=0x7fffd0024ca0)
at /root/build/media-driver/media_driver/linux/gen11/codec/ddi/media_ddi_decode_hevc_g11.cpp:719
#7 0x00007fffee1e752e in DdiMediaDecode::CreateBuffer (this=0x5555558fb570, type=VASliceParameterBufferType, size=452, numElements=1, data=0x7fffd0023f40, bufId=0x7fffd00245b8)
at /root/build/media-driver/media_driver/linux/common/codec/ddi/media_ddi_decode_base.cpp:884
#8 0x00007fffee1ef62f in DdiDecode_CreateBuffer (ctx=0x5555557cbc00, decCtx=0x7fffe40b7570, type=VASliceParameterBufferType, size=452, numElements=1, data=0x7fffd0023f40, bufId=0x7fffd00245b8)
at /root/build/media-driver/media_driver/linux/common/codec/ddi/media_libva_decoder.cpp:99
#9 0x00007fffee25b041 in DdiMedia_CreateBuffer (ctx=0x5555557cbc00, context=268435456, type=VASliceParameterBufferType, size=452, num_elements=1, data=0x7fffd0023f40, bufId=0x7fffd00245b8)
at /root/build/media-driver/media_driver/linux/common/ddi/media_libva.cpp:2604
#10 0x00007ffff331d70e in vaCreateBuffer (dpy=0x5555557cb410, context=268435456, type=VASliceParameterBufferType, size=452, num_elements=1, data=0x7fffd0023f40, buf_id=0x7fffd00245b8) at va.c:1335
#11 0x00007ffff5d806f3 in ff_vaapi_decode_make_slice_buffer (avctx=0x555555ca9900, pic=0x7fffd0024118, params_data=0x7fffd0023f40, params_size=452, slice_data=0x55555585ae1e, slice_size=287)
at libavcodec/vaapi_decode.c:87
#12 0x00007ffff5d996a5 in vaapi_hevc_decode_slice (avctx=0x555555ca9900,
buffer=0x55555585af40 "\002\001\034D\033|x\374\b\263\001\220"\350B\310N!\v\003O\370>\200\001\066f\225\025\025\064\321Y\031\231>\017\300", size=336) at libavcodec/vaapi_hevc.c:413
#13 0x00007ffff5979ece in decode_nal_unit (s=0x555555caa1c0, nal=0x7fffd0012138) at libavcodec/hevcdec.c:3009
#14 0x00007ffff597a25b in decode_nal_units (s=0x555555caa1c0, buf=0x55555585a850 "", length=5654) at libavcodec/hevcdec.c:3089
#15 0x00007ffff597a87f in hevc_decode_frame (avctx=0x555555ca9900, data=0x555555ca9dc0, got_output=0x55555587af80, avpkt=0x55555587af20) at libavcodec/hevcdec.c:3227
#16 0x00007ffff5c14ee7 in frame_worker_thread (arg=0x55555587ae20) at libavcodec/pthread_frame.c:201
#17 0x00007ffff42706db in start_thread (arg=0x7fffe9ad5700) at pthread_create.c:463
#18 0x00007ffff3f9988f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

[wangyan-intel]

@fulinjie Is this issue existed still? If yes, could you please share your clip for debugging?

[fulinjie]

@fulinjie Is this issue existed still? If yes, could you please share your clip for debugging?

Yes, it still exists.
In FFmpeg, we've implemented some work around for this:
(e.g. set slice_param_size to sizeof(VASliceParameterBufferHEVCExtension) only if its profile is FF_PROFILE_HEVC_REXT )
https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/vaapi_hevc.c#L302
https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/vaapi_hevc.c#L403

You may verify after changing it into:
int slice_param_size = sizeof(pic->last_slice_param);

clip would be provided through mail.

[wangyan-intel]

@fulinjie
I have reproduced and debugged it.
From driver side, DdiDecodeHEVCG11::AllocSliceControlBuffer doesn't accept the input size from ffmeg and it always operate the request based on the real size. if it is HEVC main 10, it always uses VASliceParameterBufferHEVC to do it. But its caller DdiMediaDecode::CreateBuffer() will use the input size and doesn't double check it.
From FFmpeg side, IMHO, it would not use VASliceParameterBufferHEVCREXT whatever the clip is or is not non REXT/SCC clips.
https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/vaapi_hevc.c#L36

So I can add check in DdiDecodeHEVCG11::AllocSliceControlBuffer. If the size is not matched, it will return VA_STATUS_ERROR_ALLOCATION_FAILED.

Do you agree my solution?
Thanks.

[fulinjie]

Hi,

@fulinjie
I have reproduced and debugged it.
From driver side, DdiDecodeHEVCG11::AllocSliceControlBuffer doesn't accept the input size from ffmeg and it always operate the request based on the real size. if it is HEVC main 10, it always uses VASliceParameterBufferHEVC to do it. But its caller DdiMediaDecode::CreateBuffer() will use the input size and doesn't double check it.
From FFmpeg side, IMHO, it would not use VASliceParameterBufferHEVCREXT whatever the clip is or is not non REXT/SCC clips.
https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/vaapi_hevc.c#L36

So I can add check in DdiDecodeHEVCG11::AllocSliceControlBuffer. If the size is not matched, it will return VA_STATUS_ERROR_ALLOCATION_FAILED.
Do you agree my solution?

Yes please, a CHECK is necessary to avoid the potential crash from user.

As to the usage of VASliceParameterBufferHEVCREXT, I'd prefer to use the extended structure for all profiles from the perspective of downward compatibility, and the memory overhead is not very large.

But we'd keep it as a open for now, and make some alignments with gstreamer side before we touch the code of real allocation size in driver.

Thanks.

[wangyan-intel]

@fulinjie
Thanks for your comments.
I will submit the PR for adding the checking at first.
Please sync with @xhaihao about gstreamer status for this..
Thanks.

[wangyan-intel]

@fulinjie Could you please take a look: #996? Thanks.