On Fri, Feb 26, 2016 at 02:20:33PM +0100, Reimar Döffinger wrote: > On 26.02.2016, at 14:01, Michael Niedermayer <mich...@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.doeffin...@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 dont object, and iam ok with the mjpeg patch though i was wondering how robust using nb_components is ultimately it is a variable read from the bitstream, pix_fmt and frame set after it it is checked but its consistency depends on quite some code (that is pix_fmt being set consistently with it and no change to nb_components or pix_fmt / frame leaking without the other, i think its ok but maybe a small explicit check should be added somewhere to check that the nb_components and pix_fmt are consistent [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Its not that you shouldnt use gotos but rather that you should write readable code and code with gotos often but not always is less readable
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
