This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATH] [BZ 15674] Fix reading past the array boundary in __memcmp_ssse3
- From: Dmitrieva Liubov <liubov dot dmitrieva at gmail dot com>
- To: Richard Henderson <rth at twiddle dot net>, "Carlos O'Donell" <carlos at redhat dot com>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Wed, 26 Jun 2013 16:16:26 +0400
- Subject: Re: [PATH] [BZ 15674] Fix reading past the array boundary in __memcmp_ssse3
- References: <CAHjhQ92DBAVCozvunaCNhRgswUHcQL42Yc24pieVVU=GGBFrww at mail dot gmail dot com> <51C9BC0F dot 6030201 at twiddle dot net>
Is the patch ok now?
2013-06-25 Liubov Dmitrieva <liubov.dmitrieva@intel.com>
[BZ #15674]
* sysdeps/x86_64/multiarch/memcmp-ssse3.S: Fix
buffers overrun.
* string/test-memcmp.c: Add new regression test.
diff --git a/string/test-memcmp.c b/string/test-memcmp.c
index b30e34d..db6b28a 100644
--- a/string/test-memcmp.c
+++ b/string/test-memcmp.c
@@ -448,6 +448,37 @@ check1 (void)
}
}
+/* This test checks that memccmp doesn't overrun buffers. */
+static void
+check2 (void)
+{
+ int max_length = BUF1PAGES * page_size / sizeof (CHAR);
+
+ char * buf = (char *) malloc (sizeof (char) * max_length);
+ /* Initialize buf to the same values as buf1. */
+ memset (buf, 0xa5, max_length);
+ /* The bug requires the last compared byte to be different. */
+ buf[max_length - 1] = 0x5a;
+
+ int length;
+
+ for (length = 1; length < max_length; length++)
+ {
+ char * s1 = (char *) buf1 + max_length - length;
+ char * s2 = buf + max_length - length;
+
+ const int exp_result = SIMPLE_MEMCMP (s1, s2, length);
+
+ FOR_EACH_IMPL (impl, 0)
+ {
+ printf ("check2: length=%d, %s\n", length, impl->name);
+ check_result (impl, s1, s2, length, exp_result);
+ }
+ }
+
+ free(buf);
+}
+
int
test_main (void)
{
@@ -456,6 +487,7 @@ test_main (void)
test_init ();
check1 ();
+ check2 ();
printf ("%23s", "");
FOR_EACH_IMPL (impl, 0)
diff --git a/sysdeps/x86_64/multiarch/memcmp-ssse3.S
b/sysdeps/x86_64/multiarch/memcmp-ssse3.S
index bdd2ed2..e319df9 100644
--- a/sysdeps/x86_64/multiarch/memcmp-ssse3.S
+++ b/sysdeps/x86_64/multiarch/memcmp-ssse3.S
@@ -1463,10 +1463,8 @@ L(next_24_bytes):
test $0x40, %dh
jnz L(Byte22)
- mov -9(%rdi), %eax
- and $0xff, %eax
- mov -9(%rsi), %edx
- and $0xff, %edx
+ movzbl -9(%rdi), %eax
+ movzbl -9(%rsi), %edx
sub %edx, %eax
ret
# else
--
Liubov
On Tue, Jun 25, 2013 at 7:49 PM, Richard Henderson <rth@twiddle.net> wrote:
> On 06/25/2013 07:11 AM, Dmitrieva Liubov wrote:
>> - mov -9(%rdi), %eax
>> - and $0xff, %eax
>> - mov -9(%rsi), %edx
>> - and $0xff, %edx
>> +/* Movzbl reads only one byte and
>> + doesn't read past the array boundary. */
>> +
>> + movzbl -9(%rdi), %eax
>> + movzbl -9(%rsi), %edx
>
> We surely don't need a comment about how a non-obscure x86 architecture insn
> operates. The fix itself itself is obviously correct.
>
>
> r~