[FFmpeg-devel] [PATCH] mjpegdec: Do not assume unused plane pointer are NULL.

Reimar Döffinger Reimar.Doeffinger at gmx.de
Fri Feb 26 14:20:33 CET 2016


On 26.02.2016, at 14:01, Michael Niedermayer <michael at niedermayer.cc> wrote:

> On Fri, Feb 26, 2016 at 01:17:29PM +0100, Reimar Döffinger wrote:
>> Well, there I go, doing my own idle discussions
>> I just complained about ;)
>> 
>> On Fri, Feb 26, 2016 at 01:00:23PM +0100, wm4 wrote:
>>> On Fri, 26 Feb 2016 12:19:18 +0100
>>> Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
>>>> I am not exactly sure what their opinion is to be honest.
>>>> However this attitude of "it's of course a bug in the calling code! Nobody can actually know it because it's 100% undocumented requirement but it's their bug" pisses me of quite a bit.
>>> 
>>> Well, that often happens to me too in general. You're free top audit
>>> ALL the decoders and filters to use the number of planes instead of
>>> checking plane pointers. As you see, there's a bit of a practical
>>> problem with this. It doesn't help either making half of the API handle
>>> this "correctly" (as you or any other sane person might consider it),
>>> while the other half still exposes the other behavior.
>> 
>> For something that is potentially exploitable?
>> I disagree. Every single one fixed is a relevant improvement.
>> There is also the point that every place that uses a better way
>> is one place less where someone might copy-paste non-optimal
>> code from.
>> But I admit I consider this mostly a stop-gap, I am kind of in
>> favour of checking the get_buffer2 result.
>> But that has some issues and tricky points so probably needs to
>> be done with care.
> 
> so the possible "solutions" are (not neccessarily exclusive)
> 1. document that unused pointers must be NULL
> 2. Check if the unused pointers from the user app are NULL, fail
>   if not.
> 3. clear the pointers from the user app which are unused
> 4. make a copy and clear the pointers from the user app which are
>   unused, if they arent NULL
> 5. audit alot of code and change it so it works even if some pointers
>   are invalid
> 6. Ignore it, assuming all user apps which do this have hit crashes
>   and no longer pass stale pointers
> 
> 
> either 1, 3, 4 or 5 is needed, as docs and code mismatch

I think I'd like to do 3 + print warning, with hwaccel excepted.
I might also look into 5, as a kind of "defense in depth" thing, so please tell me if you think you'd object to such "unnecessary" changes so I do not waste my time.
I'd also add a check the opposite way: that none of the required pointers are NULL.
But I wonder: do we have a function to get the number of planes for a format?
It seems pretty painful (well, compared to how simple I think it should be) to get the information from the pixel format descriptor.


More information about the ffmpeg-devel mailing list