[Bug string/29088] New: On x86_64 all versions of memrchr can segfault with seemingly valid inputs.

[Bug string/29088] New: On x86_64 all versions of memrchr can segfault with seemingly valid inputs.

[goldstein.w.n at gmail dot com]

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

            Bug ID: 29088
           Summary: On x86_64 all versions of memrchr can segfault with
                    seemingly valid inputs.
           Product: glibc
           Version: 2.36
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: string
          Assignee: unassigned at sourceware dot org
          Reporter: goldstein.w.n at gmail dot com
  Target Milestone: ---

```
#define _GNU_SOURCE
#include <assert.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/mman.h>
static __attribute__((noinline, noclone)) char *
invalid_memory() {
    return (char *)(1UL);
}

#define impl memrchr
int
main(int argc, char ** argv) {
    char * buf =
        mmap(NULL, 4096 * 3, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
    assert(buf != MAP_FAILED);

    buf += 4096;
    assert(!mprotect(buf, 4096, PROT_READ | PROT_WRITE));

    uint64_t len = 0;
    uint64_t len2;
    int      i;
    for (i = 0; i < 4096; ++i) {
        buf[i] = 1;
    }


    fprintf(stderr, "Test - 0\n");
    /* Fails for SSE2, fine for AVX2/EVEX. */
    assert(impl(invalid_memory(), 1, len) == NULL);

    len = 2;
    fprintf(stderr, "Test - 1\n");

    len2 = (uint64_t)buf + len - 1;
    /* Segfaults for SSE2, AVX2, and EVEX. */
    assert(impl(invalid_memory(), 1, len2) == impl(buf, 1, len));
}


```

The first case `Test - 0` seems like a clear bug as if length is zero reading
any memory is clearly outside of the "initial n bytes".

The second case also appears to be a bug based on:

"""
This function behaves as if it reads the characters sequentially and stops as
soon as a matching character is found: if the array pointed to by ptr is
smaller than count, but the match is found within the array, the behavior is
well-defined
"""
https://en.cppreference.com/w/c/string/byte/memchr

And 
"""
Implementations shall behave as if they read the memory byte by byte from the
beginning of the bytes pointed to by s and stop at the first occurrence of c
(if it is found in the initial n bytes).
"""
https://pubs.opengroup.org/onlinepubs/9699919799/functions/memchr.html

And the description of memrchr as:

"""
The memrchr() function is like the memchr() function, except that it searches
backward from the end of the n bytes pointed to by s instead of forward from
the beginning.
"""
https://linux.die.net/man/3/memchr

The first potential bug in SSE2 is because there is no zero length check.

The second potential bug in all implementations is because there is no page
cross check if length >= VEC_SIZE.

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

[Bug string/29088] On x86_64 all versions of memrchr can segfault with seemingly valid inputs.

[goldstein.w.n at gmail dot com]

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

--- Comment #1 from Noah Goldstein <goldstein.w.n at gmail dot com> ---
These look like bugs to me but I may be wrong. If so I think the documentation
for `memrchr` should be changed as describing it as the same as `memchr` but
backwards  doesn't seem correct.

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

[Bug string/29088] On x86_64 all versions of memrchr can segfault with seemingly valid inputs.

[schwab@linux-m68k.org]

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

--- Comment #2 from Andreas Schwab <schwab@linux-m68k.org> ---
"Where an argument declared as size_t n specifies the length of the array for a
function, n can have the value zero on a call to that function.  Unless
explicitly stated otherwise in the description of a particular function in this
subclause, pointer arguments on such a call shall still have valid values, as
described in 7.1.4."

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

[Bug string/29088] On x86_64 all versions of memrchr can segfault with seemingly valid inputs.

[goldstein.w.n at gmail dot com]

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

--- Comment #3 from Noah Goldstein <goldstein.w.n at gmail dot com> ---
(In reply to Andreas Schwab from comment #2)
> "Where an argument declared as size_t n specifies the length of the array
> for a function, n can have the value zero on a call to that function. 
> Unless explicitly stated otherwise in the description of a particular
> function in this subclause, pointer arguments on such a call shall still
> have valid values, as described in 7.1.4."

I see. `Test - 0` SSE2 bug is not a bug then.

Does 'pointer arguments on such a call shall still have valid values' imply a
certain amount of memory that must be valid or just a single byte? If its just
a single byte the second bug is still a bug as each of them over-reads into a
vector register.

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

[Bug string/29088] On x86_64 all versions of memrchr can segfault with seemingly valid inputs.

[schwab@linux-m68k.org]

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

--- Comment #4 from Andreas Schwab <schwab@linux-m68k.org> ---
memrchr is a GNU extension, and its behaviour in fringe cases is not as well
defined.

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