[x264-devel] Buffer overflow in x264_plane_copy_interleave_neon

[Kirill Batuzov]

On Thu, 25 Aug 2016, Henrik Gramner wrote:

> 
> The ARM code should be fixed to do the same.

Looks like x86 wrappers can be used for ARM as well without any changes.
Here is a proof of concept patch. I copy-pasted the wrapper and renamed
the assembler function. This did fix the problem for my case. Just to be
clear, I'm not ready to sign up to do a proper fix, just reporting a bug
and sharing whatever relevant information I dug up.

diff --git a/common/arm/mc-a.S b/common/arm/mc-a.S
index 76295cd..549d5bf 100644
--- a/common/arm/mc-a.S
+++ b/common/arm/mc-a.S
@@ -1577,7 +1577,7 @@ block4:
     pop             {r4-r8, r10, r11, pc}
 endfunc
 
-function x264_plane_copy_interleave_neon
+function x264_plane_copy_interleave_core_neon
     push            {r4-r7, lr}
     ldrd            r6, r7, [sp, #28]
     ldrd            r4, r5, [sp, #20]
diff --git a/common/arm/mc-c.c b/common/arm/mc-c.c
index d330bc3..40ef325 100644
--- a/common/arm/mc-c.c
+++ b/common/arm/mc-c.c
@@ -57,7 +57,7 @@ void x264_plane_copy_deinterleave_rgb_neon( pixel *dsta, intptr_t i_dsta,
                                             pixel *dstb, intptr_t i_dstb,
                                             pixel *dstc, intptr_t i_dstc,
                                             pixel *src,  intptr_t i_src, int pw, int w, int h );
-void x264_plane_copy_interleave_neon( pixel *dst,  intptr_t i_dst,
+void x264_plane_copy_interleave_core_neon( pixel *dst,  intptr_t i_dst,
                                       pixel *srcu, intptr_t i_srcu,
                                       pixel *srcv, intptr_t i_srcv, int w, int h );
 void x264_plane_copy_swap_neon( pixel *dst, intptr_t i_dst,
@@ -67,6 +67,36 @@ void x264_store_interleave_chroma_neon( pixel *dst, intptr_t i_dst, pixel *srcu,
 void x264_load_deinterleave_chroma_fdec_neon( pixel *dst, pixel *src, intptr_t i_src, int height );
 void x264_load_deinterleave_chroma_fenc_neon( pixel *dst, pixel *src, intptr_t i_src, int height );
 
+#define PLANE_INTERLEAVE(cpu) \
+static void x264_plane_copy_interleave_##cpu( pixel *dst,  intptr_t i_dst,\
+                                              pixel *srcu, intptr_t i_srcu,\
+                                              pixel *srcv, intptr_t i_srcv, int w, int h )\
+{\
+    int c_w = 16 / sizeof(pixel) - 1;\
+    if( !(w&c_w) )\
+        x264_plane_copy_interleave_core_##cpu( dst, i_dst, srcu, i_srcu, srcv, i_srcv, w, h );\
+    else if( w > c_w && (i_srcu ^ i_srcv) >= 0 ) /* only works correctly for strides with identical signs */\
+    {\
+        if( --h > 0 )\
+        {\
+            if( i_srcu > 0 )\
+            {\
+                x264_plane_copy_interleave_core_##cpu( dst, i_dst, srcu, i_srcu, srcv, i_srcv, (w+c_w)&~c_w, h );\
+                dst  += i_dst  * h;\
+                srcu += i_srcu * h;\
+                srcv += i_srcv * h;\
+            }\
+            else\
+                x264_plane_copy_interleave_core_##cpu( dst+i_dst, i_dst, srcu+i_srcu, i_srcu, srcv+i_srcv, i_srcv, (w+c_w)&~c_w, h );\
+        }\
+        x264_plane_copy_interleave_c( dst, 0, srcu, 0, srcv, 0, w, 1 );\
+    }\
+    else\
+        x264_plane_copy_interleave_c( dst, i_dst, srcu, i_srcu, srcv, i_srcv, w, h );\
+}
+
+PLANE_INTERLEAVE(neon)
+
 #if !HIGH_BIT_DEPTH
 #define MC_WEIGHT(func)\
 void x264_mc_weight_w20##func##_neon( uint8_t *, intptr_t, uint8_t *, intptr_t, const x264_weight_t *, int );\