[FFmpeg-devel] [PATCH] Fix crash if av_vdpau_bind_context() is not used.

[Ivan Kalvachev]

On 10/10/17, wm4 <nfxjfg at googlemail.com> wrote:
> On Tue, 10 Oct 2017 03:24:56 +0300
> Ivan Kalvachev <ikalvachev at gmail.com> wrote:
>
>> On 10/9/17, wm4 <nfxjfg at googlemail.com> wrote:
>> > On Mon, 9 Oct 2017 03:04:53 +0300
>> > Ivan Kalvachev <ikalvachev at gmail.com> wrote:
>> >
>> >> The public functions av_alloc_vdpaucontext() and
>> >> av_vdpau_alloc_context() are allocating AVVDPAUContext
>> >> structure that is supposed to be placed in avctx->hwaccel_context.
>> >>
>> >> However the rest of libavcodec/vdpau.c uses avctx->hwaccel_context
>> >> as struct VDPAUHWContext, that is bigger and does contain
>> >> AVVDPAUContext as first member.
>> >>
>> >> The usage includes write to the new variables in the bigger stuct,
>> >> without checking for block size.
>> >>
>> >> Fix by always allocating the bigger structure.
>> >>
>> >> BTW,
>> >> I have no idea why the new fields haven't simply been added to the
>> >> existing struct...
>> >> It seems that the programmer who wrote this has been aware of the
>> >> problem,
>> >> because av_vdpau_bind_context reallocates the structure.
>> >>
>> >> It might be good idea to check the other usages of this reallocation
>> >> function.
>> >>
>> >> Best Regards
>> >>    Ivan Kalvachev
>> >
>> > IMO not really worth fixing at this point, because this is the old-old
>> > vdpau API. Even av_vdpau_bind_context() (which does not require using
>> > av_alloc_vdpaucontext()) is deprecated. Or rather should be - I just
>> > haven't bothered deprecating it because the deprecation dance is too
>> > messy. In any case, you shouldn't use any of those APIs - use the
>> > generic hwaccel API instead (setting hw_frames_ctx or hw_device_ctx).
>>
>> Every bug must be fixed, even if the code is going to be removed next.
>>
>> Since you "didn't bother" to deprecate it, this code will remain even
>> after
>> the API bump. And it is still (mis)used by at least one program that
>> crashed on me.
>>
>> So it MUST be fixed.
>
> Well, if you insist, feel free to attempt to maintain it, but I won't
> care, even if I make changes to the code under the newer API (about
> which I care). I tried to remove some vdpau code earlier which had
> been deprecated for years, but this was rejected, so why care about
> deprecation or whether an ancient API is actually broken? Not me.

You are not making any sense.

Anyway. Do you have any objection for this patch to be committed?