[FFmpeg-devel] [PATCH] *alloc(type)

[Michael Niedermayer]

On Sat, Nov 20, 2010 at 08:57:54PM +0300, Yuriy Kaminskiy wrote:
> Reimar D?ffinger wrote:
> > On Sat, Nov 20, 2010 at 04:05:32PM +0300, Yuriy Kaminskiy wrote:
> >> Reimar D?ffinger wrote:
> >>> On Sat, Nov 20, 2010 at 04:37:30AM +0100, Michael Niedermayer wrote:
> >>>> patchset below fixes the type used in malloc and co
> >>>> The sense behind this patch is that feeding things that dont fit in unsigned
> >>>> int into *alloc() can lead to successfull allocation of too small arrays which
> >>>> is pretty bad.
> >>>> There are probably more functions that should be changed like av_new_packet()
> >>>> but i had to start somewhere and will look into the others too if noone else
> >>>> does.
> >>>> Note, i will apply this in a few days if there are no objections
> >>> This has some side-effects I do not like.
> >>> For example, allocating more than 4 GB now becomes possible, even
> >>> though such an allocation is almost certain to be a bug.
> >> No. A bit more context:
> >> === cut ===
> >> void *av_malloc(unsigned int size)
> >> {
> >>     void *ptr = NULL;
> >> #if CONFIG_MEMALIGN_HACK
> >>     long diff;
> >> #endif
> >>
> >>     /* let's disallow possible ambiguous cases */
> >>     if(size > (INT_MAX-16) )
> >>         return NULL;
> > 
> > Ok, I'll change my suggestions to:
> > How about using uint64_t always?
> 
> Are you sure it won't break e.g. those matroska demuxer trickery? (fwiw, *if* it
> will - *then* size_t is unsafe too) [maybe there are other places, that relies
> on that too, have no idea].
> FWIW, relying on "fate did not break, so let's deploy it" is very wrong, IMO -
> brokenness in this area won't show up with normal non-broken files (or even just
> randomly fuzzied files), you need carefully craft broken file to test it.

Do you have a hypothetical example that would break if malloc(uint ->size_t) ?

[...]
--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101120/454d9945/attachment.pgp>