[FFmpeg-devel] [PATCH] avutil/hwcontext_vaapi: fix SEGV in vaTerminate when vaInitialize fails

[Mark Thompson]

On 03/02/17 22:44, Aman Gupta wrote:
> On Fri, Feb 3, 2017 at 12:19 PM, Mark Thompson <sw at jkqxz.net> wrote:
> 
>> On 03/02/17 05:45, wm4 wrote:
>>> On Thu,  2 Feb 2017 09:29:13 -0800
>>> Aman Gupta <ffmpeg at tmm1.net> wrote:
>>>
>>>> From: Aman Gupta <aman at tmm1.net>
>>>>
>>>> Program terminated with signal SIGSEGV, Segmentation fault.
>>>>     opts=opts at entry=0x0, flags=flags at entry=0) at
>> libavutil/hwcontext.c:494
>>>> ---
>>>>  libavutil/hwcontext_vaapi.c | 3 +--
>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
>>>> index 6176bdc..0051acb 100644
>>>> --- a/libavutil/hwcontext_vaapi.c
>>>> +++ b/libavutil/hwcontext_vaapi.c
>>>> @@ -961,14 +961,13 @@ static int vaapi_device_create(AVHWDeviceContext
>> *ctx, const char *device,
>>>>          return AVERROR(EINVAL);
>>>>      }
>>>>
>>>> -    hwctx->display = display;
>>>> -
>>>>      vas = vaInitialize(display, &major, &minor);
>>>>      if (vas != VA_STATUS_SUCCESS) {
>>>>          av_log(ctx, AV_LOG_ERROR, "Failed to initialise VAAPI "
>>>>                 "connection: %d (%s).\n", vas, vaErrorStr(vas));
>>>>          return AVERROR(EIO);
>>>>      }
>>>> +    hwctx->display = display;
>>>>      av_log(ctx, AV_LOG_VERBOSE, "Initialised VAAPI connection: "
>>>>             "version %d.%d\n", major, minor);
>>>>
>>>
>>> Would that mean it doesn't free the display that was created with
>>> vaGetDisplay? Is that right?
>>>
>>> In my experiments, calling vaTerminate right after vaGetDisplay works
>>> just fine.
>>
>> Right, looking more carefully at libva that is exactly what you are meant
>> to do, and the code there is careful to make it all work.  The segfault
>> case I was thinking of here isn't exactly the same (and used the Intel
>> proprietary driver, which should probably be considered dubious), so
>> applying it was premature.
>>
>> Aman, can you explain more about the case you saw this in?
>>
> 
> I saw this when I was using libva master. vaInitialize() was failing in my
> environment (see https://github.com/01org/libva/issues/20) and after the
> failure ffmpeg crashed.
> 
> Here was the output from ffmpeg:
> 
> libva info: VA-API version 0.40.0
> libva info: va_getDriverName() returns 1
> libva error: va_getDriverName() failed with operation
> failed,driver_name=i965
> [AVHWDeviceContext @ 0x1b03d80] Failed to initialise VAAPI connection: 1
> (operation failed).
> Segmentation fault
> 
> And the backtrace:
> 
>   #0  0x0000000000aff8a4 in vaTerminate ()
>   #1  0x0000000000ae50ce in vaapi_device_free (ctx=<optimized out>) at
> libavutil/hwcontext_vaapi.c:882
>   #2  0x0000000000ae1f9e in hwdevice_ctx_free (opaque=<optimized out>,
> data=<optimized out>) at libavutil/hwcontext.c:66
>   #3  0x0000000000ad856f in buffer_replace (src=0x0, dst=0x7fffa26ef1b8) at
> libavutil/buffer.c:119
>   #4  av_buffer_unref (buf=buf at entry=0x7fffa26ef1f8) at
> libavutil/buffer.c:129
>   #5  0x0000000000ae299f in av_hwdevice_ctx_create (pdevice_ref=0x170ac50
> <hw_device_ctx>, type=type at entry=AV_HWDEVICE_TYPE_VAAPI, device=<optimized
> out>,
>       opts=opts at entry=0x0, flags=flags at entry=0) at libavutil/hwcontext.c:494
>   #6  0x0000000000400968 in vaapi_device_init (device=<optimized out>) at
> ffmpeg_vaapi.c:223
> 
> Definitely possible that this is a bug in libva instead, and that failure
> midway through vaInitialize() is not dealt with appropriately during
> vaTerminate().
> 
> Feel free to revert the commit.

Can you build libva with debug enabled and clarify exactly how and where it's failing there?  From your description on github I'm inclined to think it is some bad interaction in libva with running as root, but it would be good to be sure.  (And we should revert the change here.)

Thanks,

- Mark