[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

Martin Liška  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #16 from Martin Liška  ---
No reply, closing as resolved.

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

--- Comment #15 from Martin Liška  ---
(In reply to Ilya Enkovich from comment #13)
> (In reply to Alexander Ivchenko from comment #12)
> > Fixed with r243942
> 
> It should be backported to GCC6.

I would be happy to backport that, however the patch changes version in
libmpx/mpxwrap/libtool-version. Isn't it blocker for backporting?

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

Martin Liška  changed:

   What|Removed |Added

 CC||marxin at gcc dot gnu.org
   Assignee|unassigned at gcc dot gnu.org  |marxin at gcc dot 
gnu.org

--- Comment #14 from Martin Liška  ---
I'll prepare backport to active branches.

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[ienkovich at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

--- Comment #13 from Ilya Enkovich  ---
(In reply to Alexander Ivchenko from comment #12)
> Fixed with r243942

It should be backported to GCC6.

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[aivchenk at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

Alexander Ivchenko  changed:

   What|Removed |Added

 CC||aivchenk at gmail dot com

--- Comment #12 from Alexander Ivchenko  ---
Fixed with r243942

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[ienkovich at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

--- Comment #11 from Ilya Enkovich  ---
(In reply to H.J. Lu from comment #10)
> 
> MPX wrapper library isn't built with -z bndplt since it doesn't call
> external functions with bounds.  But some internal functions, which
> need bounds, are called via PLT which clear bounds.  It should call
> call those internal functions directly instead of via PLT.

Thanks for explanation. I thought we added '-fcheck-pointer-bounds -mmpx'
libmpxwrappers link flags but now I see we didn't. We also added tests to check
memcpy copies bounds correctly but didn't check memcpy detects bounds
violation. I suppose your patch is OK with such test added.

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

H.J. Lu  changed:

   What|Removed |Added

 Status|REOPENED|NEW

--- Comment #10 from H.J. Lu  ---
(In reply to Ilya Enkovich from comment #9)
> (In reply to H.J. Lu from comment #8)
> > 
> > MPX wrapper library doesn't call external functions with bounds
> > and there is no need to use PLT to call internal functions within
> > MPX wrapper library.
> 
> It doesn't explain why it doesn't work.

MPX wrapper library isn't built with -z bndplt since it doesn't call
external functions with bounds.  But some internal functions, which
need bounds, are called via PLT which clear bounds.  It should call
call those internal functions directly instead of via PLT.

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[ienkovich at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

--- Comment #9 from Ilya Enkovich  ---
(In reply to H.J. Lu from comment #8)
> 
> MPX wrapper library doesn't call external functions with bounds
> and there is no need to use PLT to call internal functions within
> MPX wrapper library.

It doesn't explain why it doesn't work.

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

--- Comment #8 from H.J. Lu  ---
(In reply to Ilya Enkovich from comment #7)
> (In reply to H.J. Lu from comment #6)
> > -z bndplt is needed to call external functions with bounds.  But
> > it isn't needed for internal function calls.
> 
> That doesn't explain why we need a hack you propose. Code like 
> 
> void *fn1 (void *p) {
>   return fn2(p);
> }
> 
> should work fine for any fn1 and fn2 compiled with MPX. Both internal and
> external functions should work. If call goes through PLT then BNDPLT should
> be used. If it's not then it's a bug in toolchain.

MPX wrapper library doesn't call external functions with bounds
and there is no need to use PLT to call internal functions within
MPX wrapper library.

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[ienkovich at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

--- Comment #7 from Ilya Enkovich  ---
(In reply to H.J. Lu from comment #6)
> -z bndplt is needed to call external functions with bounds.  But
> it isn't needed for internal function calls.

That doesn't explain why we need a hack you propose. Code like 

void *fn1 (void *p) {
  return fn2(p);
}

should work fine for any fn1 and fn2 compiled with MPX. Both internal and
external functions should work. If call goes through PLT then BNDPLT should be
used. If it's not then it's a bug in toolchain.

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

--- Comment #6 from H.J. Lu  ---
(In reply to Ilya Enkovich from comment #5)
> (In reply to H.J. Lu from comment #3)
> > The problem is that the internal MPX wrapper calls in libmpxwrappers.so:
> Why doesn't call go through bndplt? Users might use similar code in their
> libraries and expect it to work.

-z bndplt is needed to call external functions with bounds.  But
it isn't needed for internal function calls.

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[ienkovich at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

--- Comment #5 from Ilya Enkovich  ---
(In reply to H.J. Lu from comment #3)
> The problem is that the internal MPX wrapper calls in libmpxwrappers.so:
Why doesn't call go through bndplt? Users might use similar code in their
libraries and expect it to work.

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

--- Comment #4 from H.J. Lu  ---
Created attachment 40232
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=40232=edit
A patch

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

H.J. Lu  changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
   Last reconfirmed||2016-12-02
 Resolution|INVALID |---
 Ever confirmed|0   |1

--- Comment #3 from H.J. Lu  ---
The problem is that the internal MPX wrapper calls in libmpxwrappers.so:

(gdb) disass
Dump of assembler code for function __mpx_wrapper_memcpy:
=> 0x779d1120 <+0>: sub$0x8,%rsp
   0x779d1124 <+4>: bnd callq 0x779cfb50
<__mpx_wrapper_memmove@plt>
   0x779d112a <+10>:add$0x8,%rsp
   0x779d112e <+14>:bnd retq 
End of assembler dump.

(gdb) disass
Dump of assembler code for function __mpx_wrapper_memmove@plt:
=> 0x779cfb50 <+0>: jmpq   *0x2024c2(%rip)# 0x77bd2018
   0x779cfb56 <+6>: pushq  $0x0
   0x779cfb5b <+11>:jmpq   0x779cfb40
End of assembler dump.

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[dmitrii.kuvais...@tu-dresden.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

--- Comment #2 from Dmitrii Kuvaiskii  ---
(In reply to Ilya Enkovich from comment #1)
> PLT section is generated by linker. To have MPX friendly PLT you have to
> have MPX enabled linker supporting '-z bndplt' flag and GCC should be
> configured using MPX enabled toolchain to pass this linker option it by
> default.
> Situation you describe might happen when you use old bfd or any version of
> gold.

I'm not sure I understand the answer.
If I understand right, you mean that I need to build my shared libraries using
a friendly linker which supports '-z bndplt'. This I understand, but I do not
build a shared library. My problem is that I am using the 'libmpxwrappers'
library (already built together with gcc) which I believe contains a bug
because it was built incorrectly.

Here is the test case which reproduces the problem:

#include 
#include 
char s[10];
char d[10];
__attribute__((noinline))
char* foo(char* dst, char* src, size_t size) {
return memcpy(dst, src, size);
}
int main() {
char* r = foo(d, s, 11); // out-of-bounds!
printf("r = %p\n", r);
return 0;
}

Here is how I run it:
> gcc -fcheck-pointer-bounds -mmpx test.c
> CHKP_RT_BNDPRESERVE=0 ./a.out 
r = 0x600bd8
> CHKP_RT_BNDPRESERVE=1 ./a.out 
Saw a #BR! status 1 at 0x7f17bdb84189
Saw a #BR! status 1 at 0x7f17bdb84192
r = 0x600bd8

So the obvious buffer overflow in memcpy() is detected only when I set
BNDPRESERVE=1. Debugging this with gdb, I see:

Program received signal SIGSEGV, Segmentation fault.
=> 0x779cf189 <__mpx_wrapper_memmove+89>:   bndcu  bnd0,[r15]

So the upper-bound check detected the overflow as expected.

Can you reproduce the same behavior on your machine?

(My GCC is 6.1.0, ld is bfd version 2.26.1, I see that GCC passes '-z bndplt'
to the linker and linker eats it without problems.)

[Bug target/78631] [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

[ienkovich at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

Ilya Enkovich  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |INVALID

--- Comment #1 from Ilya Enkovich  ---
PLT section is generated by linker. To have MPX friendly PLT you have to have
MPX enabled linker supporting '-z bndplt' flag and GCC should be configured
using MPX enabled toolchain to pass this linker option it by default.

Situation you describe might happen when you use old bfd or any version of
gold.