[Bug string/31934] New: wcsncmp crash on s390x on vlbb instruction

[Bug string/31934] New: wcsncmp crash on s390x on vlbb instruction

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31934

            Bug ID: 31934
           Summary: wcsncmp crash on s390x on vlbb instruction
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: string
          Assignee: unassigned at sourceware dot org
          Reporter: fweimer at redhat dot com
                CC: stli at linux dot ibm.com
  Target Milestone: ---
            Target: s390x-linux-gnu

This test case:

“
#include <string.h>
#include <support/check.h>
#include <support/next_to_fault.h>
#include <wchar.h>

enum { buffer_length = 80 };

static int
do_test (void)
{
  struct support_next_to_fault left_ntf
    = support_next_to_fault_allocate (buffer_length * sizeof (wchar_t));
  wchar_t *left_buffer = (wchar_t *) left_ntf.buffer;
  wmemset (left_buffer, L'A', buffer_length);
  struct support_next_to_fault right_ntf
    = support_next_to_fault_allocate (buffer_length * sizeof (wchar_t));
  wchar_t *right_buffer = (wchar_t *) right_ntf.buffer;
  wmemset (right_buffer, L'A', buffer_length);

  TEST_COMPARE (wcsncmp (left_buffer, right_buffer + buffer_length - 1, 1),
                0);
  return 0;
}

#include <support/test-driver.c>
“

crashes on s390x at line 71 in sysdeps/s390/wcsncmp-vx.S:

66              l       %r1,0(%r3)
67              je      .Lend_cmp_one_char
68              crjne   %r0,%r1,.Lend_cmp_one_char
69
70      .Lloop:
71              vlbb    %v17,0(%r5,%r3),6 /* Load s2 to block boundary.  */
72              vlbb    %v16,0(%r5,%r2),6 /* Load s1 to block boundary.  */
73              lcbb    %r0,0(%r5,%r2),6 /* Get loaded byte count of s1.  */
74              jo      .Llt16_1        /* Jump away if vector not fully
loaded.  */
75              lcbb    %r1,0(%r5,%r3),6 /* Get loaded byte count of s2.  */

(gdb) print $r5
$1 = 4
(gdb) print/x $r3
$2 = 0x3fff7ff3ffc

The first and second arguments were 0x3fff7ff5ec0 and 0x3fff7ff3ffc.

I believe these are valid inputs to wcsncmp under the most rigid interpretation
of the standard.

Found through systematic testing, no application impact known.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug string/31934] wcsncmp crash on s390x on vlbb instruction

[carlos at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31934

Carlos O'Donell <carlos at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |carlos at redhat dot com

--- Comment #1 from Carlos O'Donell <carlos at redhat dot com> ---
(In reply to Florian Weimer from comment #0)
>   wmemset (left_buffer, L'A', buffer_length);
>   struct support_next_to_fault right_ntf
>     = support_next_to_fault_allocate (buffer_length * sizeof (wchar_t));
>   wchar_t *right_buffer = (wchar_t *) right_ntf.buffer;
>   wmemset (right_buffer, L'A', buffer_length);
> 
>   TEST_COMPARE (wcsncmp (left_buffer, right_buffer + buffer_length - 1, 1),
>                 0);

You wmemset both buffers to full length with L'A' (80 wide-character codes),
but do not null terminate them.

Use of support_next_to_fault_allocate puts the last L'A' at the end of page
next to a faulting page.

You then proceed to compare both buffers, but on the right buffer you start at
one character before the end.

The POSIX standard notes "wide-character codes that follow a null
wide-character code are not compared"

The call to wcsncmp asks for at most 1 wide-character code to be compared.

This looks entirely valid to me. It should return 0 since L'A' == L'A' within
the length specified.

I would have expected that string/test-strncmp, which has wcsncmp support would
test this?

Why doesn't wcsmbs/test-wcsncmp.c catch this case? It has very similar tests?
Not a tight enough bound?

Stefan Liebler added this test specifically for work being done on s390x that
should catch this.

> crashes on s390x at line 71 in sysdeps/s390/wcsncmp-vx.S:

This should not crash, and I assume it is an overread of the right_buffer in
the vector case.

> 66              l       %r1,0(%r3)
> 67              je      .Lend_cmp_one_char
> 68              crjne   %r0,%r1,.Lend_cmp_one_char
> 69
> 70      .Lloop:
> 71              vlbb    %v17,0(%r5,%r3),6 /* Load s2 to block boundary.  */
> 72              vlbb    %v16,0(%r5,%r2),6 /* Load s1 to block boundary.  */
> 73              lcbb    %r0,0(%r5,%r2),6 /* Get loaded byte count of s1.  */
> 74              jo      .Llt16_1        /* Jump away if vector not fully
> loaded.  */
> 75              lcbb    %r1,0(%r5,%r3),6 /* Get loaded byte count of s2.  */
> 
> (gdb) print $r5
> $1 = 4
> (gdb) print/x $r3
> $2 = 0x3fff7ff3ffc
> 
> The first and second arguments were 0x3fff7ff5ec0 and 0x3fff7ff3ffc.
> 
> I believe these are valid inputs to wcsncmp under the most rigid
> interpretation of the standard.
> 
> Found through systematic testing, no application impact known.

It would be good to know if this is a 1-byte overread or more.

I see Stefan is already on CC for the issue.

Thanks for the extra testing!

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug string/31934] wcsncmp crash on s390x on vlbb instruction

[stli at linux dot ibm.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31934

--- Comment #2 from Stefan Liebler <stli at linux dot ibm.com> ---
Yes, you are right. This should not crash. This is a bug.
Thanks for the extra-testing and the report. I will look into it.

Do you plan to bring your extra-tests upstream?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug string/31934] wcsncmp crash on s390x on vlbb instruction

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31934

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fweimer at redhat dot com

--- Comment #3 from Florian Weimer <fweimer at redhat dot com> ---
(In reply to Stefan Liebler from comment #2)
> Yes, you are right. This should not crash. This is a bug.
> Thanks for the extra-testing and the report. I will look into it.
> 
> Do you plan to bring your extra-tests upstream?

It was found by the test from this patch:

[PATCH] manual: Document a GNU extension for strncmp/wcsncmp
<https://inbox.sourceware.org/libc-alpha/87r0cipcps.fsf@igel.home/>

But I don't know if the test will be considered fast enough for general
inclusion.

-- 
You are receiving this mail because:
You are on the CC list for the bug.