[Bug target/94158] Expanded strlen causes out-of-bounds read on AMD64 target

2020-03-12 Thread par...@cyber-itl.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94158

--- Comment #7 from Parker Thompson  ---
(In reply to Jakub Jelinek from comment #6)
> GCC assumes pointers returned by malloc are at least MALLOC_ABI_ALIGNMENT
> bytes aligned.  That is because:
> "The pointer returned if the allocation succeeds is suitably aligned so that
> it may be assigned to a pointer to any type of object and then used to
> access such an object or an array of such objects in the space allocated
> (until the space is explicitly deallocated)."
> glibc normally guarantees 2 * sizeof (void *) alignment, GCC by default
> assumes just a wordsize alignment.
> So, if your malloc replacement doesn't provide such alignment, it is
> incorrect.

Ahhh I see, I apologize for my misunderstanding.

[Bug target/94158] Expanded strlen causes out-of-bounds read on AMD64 target

2020-03-12 Thread jakub at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94158

--- Comment #6 from Jakub Jelinek  ---
GCC assumes pointers returned by malloc are at least MALLOC_ABI_ALIGNMENT bytes
aligned.  That is because:
"The pointer returned if the allocation succeeds is suitably aligned so that it
may be assigned to a pointer to any type of object and then used to access such
an object or an array of such objects in the space allocated (until the space
is explicitly deallocated)."
glibc normally guarantees 2 * sizeof (void *) alignment, GCC by default assumes
just a wordsize alignment.
So, if your malloc replacement doesn't provide such alignment, it is incorrect.

[Bug target/94158] Expanded strlen causes out-of-bounds read on AMD64 target

2020-03-12 Thread pinskia at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94158

--- Comment #5 from Andrew Pinski  ---
(In reply to Andrew Pinski from comment #4)
> (In reply to Parker Thompson from comment #3)
> > As for alloc alignment, glibc strdup() does not use aligned_alloc, just
> > malloc.  Which by my read of the spec does not guarantee alignment.
> 
> malloc requires alignement of alignof(max_align_t) (definition in the newest
> C and C++ standards) but beforehand it was required to be aligned enough to
> support all standard types.

C11 is where the definition changes to use max_align_t.

https://en.cppreference.com/w/c/types/max_align_t

[Bug target/94158] Expanded strlen causes out-of-bounds read on AMD64 target

2020-03-12 Thread pinskia at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94158

Andrew Pinski  changed:

   What|Removed |Added

 Status|WAITING |RESOLVED
 Resolution|--- |INVALID

--- Comment #4 from Andrew Pinski  ---
(In reply to Parker Thompson from comment #3)
> As for alloc alignment, glibc strdup() does not use aligned_alloc, just
> malloc.  Which by my read of the spec does not guarantee alignment.

malloc requires alignement of alignof(max_align_t) (definition in the newest C
and C++ standards) but beforehand it was required to be aligned enough to
support all standard types.

[Bug target/94158] Expanded strlen causes out-of-bounds read on AMD64 target

2020-03-12 Thread par...@cyber-itl.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94158

--- Comment #3 from Parker Thompson  ---
(In reply to Andrew Pinski from comment #2)
> Also aligned_alloc normally does not allow alignment of 1.
> 
> So GCC is doing the correct thing.

The replacement of strdup here is just to illustrate the issue with expansion
alignment of strlen() by forcing a crash.

I encountered this issue when working with a custom malloc replacement that
would enforce out-of-bounds read checks. Using the same reproduction with clang
did not produce a crash / oob-read.

As for alloc alignment, glibc strdup() does not use aligned_alloc, just malloc.
 Which by my read of the spec does not guarantee alignment.

[Bug target/94158] Expanded strlen causes out-of-bounds read on AMD64 target

2020-03-12 Thread pinskia at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94158

Andrew Pinski  changed:

   What|Removed |Added

 Status|UNCONFIRMED |WAITING
 Ever confirmed|0   |1
   Last reconfirmed||2020-03-12

--- Comment #2 from Andrew Pinski  ---
Also aligned_alloc normally does not allow alignment of 1.

So GCC is doing the correct thing.

[Bug target/94158] Expanded strlen causes out-of-bounds read on AMD64 target

2020-03-12 Thread pinskia at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94158

--- Comment #1 from Andrew Pinski  ---
A pointer returned from strdup has to be valid to be able pass to free.

Your testcase makes that invalid.