Re: [PATH] [BZ 15674] Fix reading past the array boundary in __memcmp_ssse3

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Wed, Jun 26, 2013 at 12:13 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Jun 26, 2013 at 11:45 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Jun 26, 2013 at 8:01 AM, Liubov Dmitrieva
>> <liubov.dmitrieva@gmail.com> wrote:
>>> Fixed typo.
>>>
>>> Can anybody please commit it on my behalf, my getting access is in progress.
>>>
>>
>> I don't think testcase is correct:
>>
>> +  int max_length = BUF1PAGES * page_size / sizeof (CHAR);
>> +
>> +  char * buf = (char *) malloc (sizeof (char) * max_length);
>>                                                        ^^^ Shouldn't it be CHAR?
>> +  /* 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;
>>
>> I think you should use "buf1" instead of malloc.
>>
>
> Something like this:
>
>
> static void
> check2 (void)
> {
>   int max_length = page_size / sizeof (CHAR);
>
>   CHAR * buf = (CHAR *) buf2;
>   /* Initialize buf to the same values as buf1.  The bug requires the
>      last compared byte to be different.  */
>   memset (buf, 0xa5, page_size - 1);
>
>   int length;
>
>   for (length = 1; length < max_length; length++)
>     {
>       CHAR * s1 = (CHAR *) buf1 + max_length - length;
>       CHAR * s2 = (CHAR *) buf + max_length - length;
>
>       const int exp_result = SIMPLE_MEMCMP (s1, s2, length);
>
>       FOR_EACH_IMPL (impl, 0)
>         check_result (impl, s1, s2, length, exp_result);
>     }
> }
>

I checked in the fix without the new testcase.  Here is the
updated testcase change.  OK to install?

Thanks.


--
H.J.
---
diff --git a/string/test-memcmp.c b/string/test-memcmp.c
index b30e34d..d24e934 100644
--- a/string/test-memcmp.c
+++ b/string/test-memcmp.c
@@ -448,6 +448,31 @@ check1 (void)
     }
 }

+/* This test checks that memcmp doesn't overrun buffers.  */
+static void
+check2 (void)
+{
+  int max_length = page_size / sizeof (CHAR);
+
+  CHAR * buf = (CHAR *) buf2;
+  /* Initialize buf to the same values as buf1.  The bug requires the
+     last compared byte to be different.  */
+  memset (buf, 0xa5, page_size - 1);
+
+  int length;
+
+  for (length = 1; length < max_length; length++)
+    {
+      CHAR * s1 = (CHAR *) buf1 + max_length - length;
+      CHAR * s2 = (CHAR *) buf + max_length - length;
+
+      const int exp_result = SIMPLE_MEMCMP (s1, s2, length);
+
+      FOR_EACH_IMPL (impl, 0)
+    check_result (impl, s1, s2, length, exp_result);
+    }
+}
+
 int
 test_main (void)
 {
@@ -456,6 +481,7 @@ test_main (void)
   test_init ();

   check1 ();
+  check2 ();

   printf ("%23s", "");
   FOR_EACH_IMPL (impl, 0)