[Ffmpeg-devel] [PATCH] fix x86 asm in order to avoid textrels

[Michael Niedermayer]

Hi

On Tue, Jan 30, 2007 at 08:20:08AM +0100, Luca Barbato wrote:
> Guillaume POIRIER wrote:
> > Hi,
> > 
> > On 1/30/07, Luca Barbato <lu_zero at gentoo.org> wrote:
> >>
> >> Reference: http://bugs.gentoo.org/show_bug.cgi?id=115568
> >>
> >> Autor: Pax team
> >>
> >> please test, benchmark and flame
> > 
> > no patch attached here.... :-(
> > 
> 
> Again....
> 
> lu
> 
> -- 
> 
> Luca Barbato
> 
> Gentoo/linux Gentoo/PPC
> http://dev.gentoo.org/~lu_zero
> 

> diff -urp ffmpeg-old/libavcodec/i386/dsputil_mmx.c ffmpeg/libavcodec/i386/dsputil_mmx.c
> --- ffmpeg-old/libavcodec/i386/dsputil_mmx.c	2007-01-30 01:09:30.000000000 +0100
> +++ ffmpeg/libavcodec/i386/dsputil_mmx.c	2007-01-30 01:11:41.000000000 +0100
> @@ -657,15 +657,14 @@ static inline void transpose4x4(uint8_t 
>          "punpckhwd %%mm2, %%mm1         \n\t"
>          "movd  %%mm0, %0                \n\t"
>          "punpckhdq %%mm0, %%mm0         \n\t"
> -        "movd  %%mm0, %1                \n\t"
> -        "movd  %%mm1, %2                \n\t"
> +        "movd  %%mm0, (%0,%1)           \n\t"
> +        "movd  %%mm1, (%0,%1,2)         \n\t"
>          "punpckhdq %%mm1, %%mm1         \n\t"
> -        "movd  %%mm1, %3                \n\t"
> +        "lea (%1,%1,2), %1              \n\t"
> +        "movd  %%mm1, (%0,%1)           \n\t"
>  
> -        : "=m" (*(uint32_t*)(dst + 0*dst_stride)),
> -          "=m" (*(uint32_t*)(dst + 1*dst_stride)),
> -          "=m" (*(uint32_t*)(dst + 2*dst_stride)),
> -          "=m" (*(uint32_t*)(dst + 3*dst_stride))
> +        : "=r" (*(uint32_t*)(dst)), "+r" (dst_stride)
> +        :: "memory"
>      );

this code is wrong , it can be used to write arbitrary data to an arbitrary
address, its ironic that this is written by someone with the name "pax team"
to be more precisse %%mm0 contains some pixels which have been decoded, they
can be set arbitrarily by using PCM macroblocks
%%mm0 is then written into %0 (a register after the patch) which is then later
used as base for writing 3 more decoded 32bit values

also the old code is not what is in
svn please dont send such trash to ffmpeg-dev
patches from distributions should ALWAYS been checked carefully as they
independant of the distro are 90% wrong from my experience

checking here means
1. does it apply?
2. do the regression tests pass?
3. does the changed code still work (h.264 decoding and other cases not 
   covered by the regression tests) also ensure that the changed code
   really is executed during the test
4. changes to asm should be tested with at least gcc 2.95, 3.4, 4.something
5. non cosmatic changes to asm MUST be benchmarked or they are almost
   certainly going to be rejected (see START/STOP_TIMER)

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

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070130/fca399ae/attachment.pgp>