Message ID | ac494a6febda4430857df1fc31f64e19@huawei.com |
---|---|
State | New |
Headers | show |
Series | [v3] arm: memcpy: fix copying big data does not meet expectations [BZ #25620] | expand |
* zhuyan: > In ARMv7, the memcpy() implementation allows for program execution > to continue in scenarios where a segmentation fault or crash should > have occurred. The dangers occur in that subsequent execution and > iterations of this code will be executed with this corrupted data. > > Such as, we use 'memcpy' copy 0x80000000 byte to buffer(The buffer > size is 100 bytes), it didn't crash. > > The essence of the problem is that the memcpy implementation uses > signed instructions. So, we use unsigned instructions instead of > signed instructions. Sorry, this still does not fix the new string/tst-memmove-overflow overflow test I added recently: info: testing memmove tst-memmove-overflow.c:136: error: blob comparison failed blob length: 128 bytes left (evaluated from expected_start): "O\036m<\vZ)xG\027f5\004S\"q@\017_.}L\033j9\bW\'vE\024c2\001P\037o>\r\\+zI\030g6\006U$sB\021`/~N\035l;\nY(wF\026e4\003R!p?\016^-|K\032i8\aV%uD\023b1\000O\036m=\f[*yH\027f5\005T#rA\020_.}M\034k:\tX\'vE\025d3\002Q o>\r" 4F 1E 6D 3C 0B 5A 29 78 47 17 66 35 04 53 22 71 40 0F 5F 2E 7D 4C 1B 6A 39 08 57 27 76 45 14 63 32 01 50 1F 6F 3E 0D 5C 2B 7A 49 18 67 36 06 55 24 73 42 11 60 2F 7E 4E 1D 6C 3B 0A 59 28 77 46 16 65 34 03 52 21 70 3F 0E 5E 2D 7C 4B 1A 69 38 07 56 25 75 44 13 62 31 00 4F 1E 6D 3D 0C 5B 2A 79 48 17 66 35 05 54 23 72 41 10 5F 2E 7D 4D 1C 6B 3A 09 58 27 76 45 15 64 33 02 51 20 6F 3E 0D right (evaluated from start): "O\036mm<\vZ)xG\027f5\004S\"q@\017_.}L\033j9\bW\'vE\024c2\001P\037o>\r\\+zI\030g6\006U$sB\021`/~N\035l;\nY(wF\026e4\003R!p?\016^-|K\032i8\aV%uD\023b1\000O\036m=\f[*yH\027f5\005T#rA\020_.}M\034k:\tX\'vE\025d3\002Q o>" 4F 1E 6D 6D 3C 0B 5A 29 78 47 17 66 35 04 53 22 71 40 0F 5F 2E 7D 4C 1B 6A 39 08 57 27 76 45 14 63 32 01 50 1F 6F 3E 0D 5C 2B 7A 49 18 67 36 06 55 24 73 42 11 60 2F 7E 4E 1D 6C 3B 0A 59 28 77 46 16 65 34 03 52 21 70 3F 0E 5E 2D 7C 4B 1A 69 38 07 56 25 75 44 13 62 31 00 4F 1E 6D 3D 0C 5B 2A 79 48 17 66 35 05 54 23 72 41 10 5F 2E 7D 4D 1C 6B 3A 09 58 27 76 45 15 64 33 02 51 20 6F 3E tst-memmove-overflow.c:139: error: blob comparison failed blob length: 128 bytes left (evaluated from expected_end): " o>\r\\+{J\031h7\006U$sC\022a0\177N\035l;\nZ)xG\026e4\003R\"q@\017^-|K\032j9\bW&uD\023b2\001P\037n=\f[*yI\030g6\005T#rA\021`/~M\034k:\tY(wF\025d3\002Q!p?\016],{J\031h8\aV%tC\022a0\000O\036m<\vZ)xH\027f5\004S\"q@\020_" 20 6F 3E 0D 5C 2B 7B 4A 19 68 37 06 55 24 73 43 12 61 30 7F 4E 1D 6C 3B 0A 5A 29 78 47 16 65 34 03 52 22 71 40 0F 5E 2D 7C 4B 1A 6A 39 08 57 26 75 44 13 62 32 01 50 1F 6E 3D 0C 5B 2A 79 49 18 67 36 05 54 23 72 41 11 60 2F 7E 4D 1C 6B 3A 09 59 28 77 46 15 64 33 02 51 21 70 3F 0E 5D 2C 7B 4A 19 68 38 07 56 25 74 43 12 61 30 00 4F 1E 6D 3C 0B 5A 29 78 48 17 66 35 04 53 22 71 40 10 5F right (evaluated from start + allocation_size - sizeof (expected_end) - 1): "Q o>\r\\+{J\031h7\006U$sC\022a0\177N\035l;\nZ)xG\026e4\003R\"q@\017^-|K\032j9\bW&uD\023b2\001P\037n=\f[*yI\030g6\005T#rA\021`/~M\034k:\tY(wF\025d3\002Q!p?\016],{J\031h8\aV%tC\022a0\000O\036m<\vZ)xH\027f5\004S\"q@\020" 51 20 6F 3E 0D 5C 2B 7B 4A 19 68 37 06 55 24 73 43 12 61 30 7F 4E 1D 6C 3B 0A 5A 29 78 47 16 65 34 03 52 22 71 40 0F 5E 2D 7C 4B 1A 6A 39 08 57 26 75 44 13 62 32 01 50 1F 6E 3D 0C 5B 2A 79 49 18 67 36 05 54 23 72 41 11 60 2F 7E 4D 1C 6B 3A 09 59 28 77 46 15 64 33 02 51 21 70 3F 0E 5D 2C 7B 4A 19 68 38 07 56 25 74 43 12 61 30 00 4F 1E 6D 3C 0B 5A 29 78 48 17 66 35 04 53 22 71 40 10 The difference between the two byte patterns is: 4F 1E 6D {+6D+} 3C 0B 5A 29 78 47 17 66 35 04 53 22 71 40 0F 5F 2E 7D 4C 1B 6A 39 08 57 27 76 45 14 63 32 01 50 1F 6F 3E 0D 5C 2B 7A 49 18 67 36 06 55 24 73 42 11 60 2F 7E 4E 1D 6C 3B 0A 59 28 77 46 16 65 34 03 52 21 70 3F 0E 5E 2D 7C 4B 1A 69 38 07 56 25 75 44 13 62 31 00 4F 1E 6D 3D 0C 5B 2A 79 48 17 66 35 05 54 23 72 41 10 5F 2E 7D 4D 1C 6B 3A 09 58 27 76 45 15 64 33 02 51 20 6F 3E [-0D-] So it looks like it stopped copying after the first three bytes because the shift stopped there. I'm going to XFAIL the new test on arm, as requested by Andreas.
How can we move this forward? We have a second patch now, but the copyright status of that is unclear to me: <https://sourceware.org/pipermail/libc-alpha/2020-June/114702.html> Thanks, Florian
Hi, These patches address different implementations, if we could get both to work then together they might solve the issue for memcpy and memmove in GLIBC. There are other functions which use signed conditions as well, and this code is used in lots of different projects. Cheers, Wilco
diff --git a/sysdeps/arm/armv7/multiarch/memcpy_impl.S b/sysdeps/arm/armv7/multiarch/memcpy_impl.S index bf4ac7077f..068474f0ea 100644 --- a/sysdeps/arm/armv7/multiarch/memcpy_impl.S +++ b/sysdeps/arm/armv7/multiarch/memcpy_impl.S @@ -268,7 +268,7 @@ ENTRY(memcpy) mov dst, dstin /* Preserve dstin, we need to return it. */ cmp count, #64 - bge .Lcpy_not_short + bhs .Lcpy_not_short /* Deal with small copies quickly by dropping straight into the exit block. */ @@ -351,10 +351,10 @@ ENTRY(memcpy) 1: subs tmp2, count, #64 /* Use tmp2 for count. */ - blt .Ltail63aligned + blo .Ltail63aligned cmp tmp2, #512 - bge .Lcpy_body_long + bhs .Lcpy_body_long .Lcpy_body_medium: /* Count in tmp2. */ #ifdef USE_VFP @@ -378,7 +378,7 @@ ENTRY(memcpy) add src, src, #64 vstr d1, [dst, #56] add dst, dst, #64 - bge 1b + bhs 1b tst tmp2, #0x3f beq .Ldone @@ -412,7 +412,7 @@ ENTRY(memcpy) ldrd A_l, A_h, [src, #64]! strd A_l, A_h, [dst, #64]! subs tmp2, tmp2, #64 - bge 1b + bhs 1b tst tmp2, #0x3f bne 1f ldr tmp2,[sp], #FRAME_SIZE @@ -482,7 +482,7 @@ ENTRY(memcpy) add src, src, #32 subs tmp2, tmp2, #prefetch_lines * 64 * 2 - blt 2f + blo 2f 1: cpy_line_vfp d3, 0 cpy_line_vfp d4, 64 @@ -494,7 +494,7 @@ ENTRY(memcpy) add dst, dst, #2 * 64 add src, src, #2 * 64 subs tmp2, tmp2, #prefetch_lines * 64 - bge 1b + bhs 1b 2: cpy_tail_vfp d3, 0
In ARMv7, the memcpy() implementation allows for program execution to continue in scenarios where a segmentation fault or crash should have occurred. The dangers occur in that subsequent execution and iterations of this code will be executed with this corrupted data. Such as, we use 'memcpy' copy 0x80000000 byte to buffer(The buffer size is 100 bytes), it didn't crash. The essence of the problem is that the memcpy implementation uses signed instructions. So, we use unsigned instructions instead of signed instructions. Signed-off-by: Yan Zhu <zhuyan34@huawei.com> --- sysdeps/arm/armv7/multiarch/memcpy_impl.S | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)