Message ID | CAMe9rOq-5XqT8CRBrrb69nbzYEL0=0r2fM9CDeBDF8xhzXV_JQ@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Thu, Jul 2, 2015 at 4:41 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Jul 2, 2015 at 7:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, Jul 2, 2015 at 7:03 AM, Zamyatin, Igor <igor.zamyatin@intel.com> wrote: >>> Hi! >>> >>> This patch adds necessary changes for proper work of dl_runtime routines both for 32 and 64 bits in MPX mode. >>> >>> Is it ok for trunk? >>> >>> >>> Thanks, >>> Igor >>> >>> >>> 2015-07-02 Igor Zamyatin <igor.zamyatin@intel.com> >>> >>> * sysdeps/i386/dl-trampoline.S (_dl_runtime_profile): Save >>> and restore Intel MPX return bound registers >>> * sysdeps/x86_64/dl-trampoline.h: Add PRESERVE_BND_REGS_PREFIX to >>> call, jump and ret instructions to not loose bounds. >>> * sysdeps/x86/bits/link.h (La_i86_retval): Add lrv_bnd0 and >>> lrv_bnd1. >> >> Should we add link-defines.sym, similar to x86-64, to avoid >> those magic numbers when accessing the fields in La_i86_regs >> and La_i86_retval? >> > > Here is a patch to add sysdeps/i386/link-defines.sym. > > I think the i386 MPX change will store values into La_i86_retval > with wrong order. You need to store bnd0/bnd1 after lrv_st1, not > before lrv_eax. > > Please add an i386 audit testcase to verify that pltexit gets correct > La_i86_regs and La_i86_retval pointers and fix the i386 MPX patch. > > I updated my patch and pushed it to hjl/audit branch.
On Thu, Jul 2, 2015 at 4:41 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Jul 2, 2015 at 7:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, Jul 2, 2015 at 7:03 AM, Zamyatin, Igor <igor.zamyatin@intel.com> wrote: >>> Hi! >>> >>> This patch adds necessary changes for proper work of dl_runtime routines both for 32 and 64 bits in MPX mode. >>> >>> Is it ok for trunk? >>> >>> >>> Thanks, >>> Igor >>> >>> >>> 2015-07-02 Igor Zamyatin <igor.zamyatin@intel.com> >>> >>> * sysdeps/i386/dl-trampoline.S (_dl_runtime_profile): Save >>> and restore Intel MPX return bound registers >>> * sysdeps/x86_64/dl-trampoline.h: Add PRESERVE_BND_REGS_PREFIX to >>> call, jump and ret instructions to not loose bounds. >>> * sysdeps/x86/bits/link.h (La_i86_retval): Add lrv_bnd0 and >>> lrv_bnd1. >> >> Should we add link-defines.sym, similar to x86-64, to avoid >> those magic numbers when accessing the fields in La_i86_regs >> and La_i86_retval? >> > > Here is a patch to add sysdeps/i386/link-defines.sym. > > I think the i386 MPX change will store values into La_i86_retval > with wrong order. You need to store bnd0/bnd1 after lrv_st1, not > before lrv_eax. > > Please add an i386 audit testcase to verify that pltexit gets correct > La_i86_regs and La_i86_retval pointers and fix the i386 MPX patch. > > Hi Igor, I checked in my i386 audit update and test. Please update and resubmit i386 MPX change. Thanks.
Hi! Please see updated patch (attached) ChangeLog: 2015-07-08 Igor Zamyatin <igor.zamyatin@intel.com> [BZ #18134] * sysdeps/i386/dl-trampoline.S (_dl_runtime_profile): Save and restore Intel MPX return bound registers. * sysdeps/x86_64/dl-trampoline.h: Add PRESERVE_BND_REGS_PREFIX to call, jump and ret instructions to not loose bounds. * sysdeps/x86/bits/link.h (La_i86_retval): Add lrv_bnd0 and lrv_bnd1. * sysdeps/i386/link-defines.sym: Add definitions for LRV_BND0_OFFSET and LRV_BND1_OFFSET. Thanks, Igor > -----Original Message----- > From: H.J. Lu [mailto:hjl.tools@gmail.com] > Sent: Friday, July 3, 2015 2:42 AM > To: Zamyatin, Igor > Cc: libc-alpha@sourceware.org > Subject: Re: [PATCH, MPX] MPX-specific changes in dl_runtime routines > > On Thu, Jul 2, 2015 at 7:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > > On Thu, Jul 2, 2015 at 7:03 AM, Zamyatin, Igor <igor.zamyatin@intel.com> > wrote: > >> Hi! > >> > >> This patch adds necessary changes for proper work of dl_runtime routines > both for 32 and 64 bits in MPX mode. > >> > >> Is it ok for trunk? > >> > >> > >> Thanks, > >> Igor > >> > >> > >> 2015-07-02 Igor Zamyatin <igor.zamyatin@intel.com> > >> > >> * sysdeps/i386/dl-trampoline.S (_dl_runtime_profile): Save > >> and restore Intel MPX return bound registers > >> * sysdeps/x86_64/dl-trampoline.h: Add > PRESERVE_BND_REGS_PREFIX to > >> call, jump and ret instructions to not loose bounds. > >> * sysdeps/x86/bits/link.h (La_i86_retval): Add lrv_bnd0 and > >> lrv_bnd1. > > > > Should we add link-defines.sym, similar to x86-64, to avoid those > > magic numbers when accessing the fields in La_i86_regs and > > La_i86_retval? > > > > Here is a patch to add sysdeps/i386/link-defines.sym. > > I think the i386 MPX change will store values into La_i86_retval with wrong > order. You need to store bnd0/bnd1 after lrv_st1, not before lrv_eax. > > Please add an i386 audit testcase to verify that pltexit gets correct > La_i86_regs and La_i86_retval pointers and fix the i386 MPX patch. > > > -- > H.J.
On Wed, Jul 8, 2015 at 8:25 AM, Zamyatin, Igor <igor.zamyatin@intel.com> wrote: > Hi! > > Please see updated patch (attached) > > ChangeLog: > > 2015-07-08 Igor Zamyatin <igor.zamyatin@intel.com> > > [BZ #18134] > * sysdeps/i386/dl-trampoline.S (_dl_runtime_profile): Save > and restore Intel MPX return bound registers. > * sysdeps/x86_64/dl-trampoline.h: Add PRESERVE_BND_REGS_PREFIX to > call, jump and ret instructions to not loose bounds. > * sysdeps/x86/bits/link.h (La_i86_retval): Add lrv_bnd0 and > lrv_bnd1. > * sysdeps/i386/link-defines.sym: Add definitions for LRV_BND0_OFFSET > and LRV_BND1_OFFSET. > +#else + .byte 0x66,0x0f,0x1b,0x04,0x24 + .byte 0x66,0x0f,0x1b,0x4c,0x24,0x08 +#endif ... +#else + .byte 0x66,0x0f,0x1a,0x04,0x24 + .byte 0x66,0x0f,0x1a,0x4c,0x24,0x08 +#endif Please use LRV_BND0_OFFSET and LRV_BND1_OFFSET.
Fixed in the attached patch Thanks, Igor > -----Original Message----- > From: H.J. Lu [mailto:hjl.tools@gmail.com] > Sent: Wednesday, July 8, 2015 6:35 PM > To: Zamyatin, Igor > Cc: libc-alpha@sourceware.org > Subject: Re: [PATCH, MPX] MPX-specific changes in dl_runtime routines > > On Wed, Jul 8, 2015 at 8:25 AM, Zamyatin, Igor <igor.zamyatin@intel.com> > wrote: > > Hi! > > > > Please see updated patch (attached) > > > > ChangeLog: > > > > 2015-07-08 Igor Zamyatin <igor.zamyatin@intel.com> > > > > [BZ #18134] > > * sysdeps/i386/dl-trampoline.S (_dl_runtime_profile): Save > > and restore Intel MPX return bound registers. > > * sysdeps/x86_64/dl-trampoline.h: Add PRESERVE_BND_REGS_PREFIX > to > > call, jump and ret instructions to not loose bounds. > > * sysdeps/x86/bits/link.h (La_i86_retval): Add lrv_bnd0 and > > lrv_bnd1. > > * sysdeps/i386/link-defines.sym: Add definitions for > LRV_BND0_OFFSET > > and LRV_BND1_OFFSET. > > > > +#else > + .byte 0x66,0x0f,0x1b,0x04,0x24 > + .byte 0x66,0x0f,0x1b,0x4c,0x24,0x08 > +#endif > ... > +#else > + .byte 0x66,0x0f,0x1a,0x04,0x24 > + .byte 0x66,0x0f,0x1a,0x4c,0x24,0x08 > +#endif > > Please use LRV_BND0_OFFSET and LRV_BND1_OFFSET. > > > -- > H.J.
From 4d2115cf349fbc235f4ab8c9a117bd1b20b44f2d Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Thu, 2 Jul 2015 16:33:03 -0700 Subject: [PATCH] Add and use sysdeps/i386/link-defines.sym --- sysdeps/i386/Makefile | 1 + sysdeps/i386/dl-trampoline.S | 39 ++++++++++++++++++++++++--------------- sysdeps/i386/link-defines.sym | 11 +++++++++++ 3 files changed, 36 insertions(+), 15 deletions(-) create mode 100644 sysdeps/i386/link-defines.sym diff --git a/sysdeps/i386/Makefile b/sysdeps/i386/Makefile index c8af591..87b5c6f 100644 --- a/sysdeps/i386/Makefile +++ b/sysdeps/i386/Makefile @@ -33,6 +33,7 @@ sysdep-CFLAGS += -mpreferred-stack-boundary=4 else ifeq ($(subdir),csu) sysdep-CFLAGS += -mpreferred-stack-boundary=4 +gen-as-const-headers += link-defines.sym else # Likewise, any function which calls user callbacks uses-callbacks += -mpreferred-stack-boundary=4 diff --git a/sysdeps/i386/dl-trampoline.S b/sysdeps/i386/dl-trampoline.S index f11972c..b4372c9 100644 --- a/sysdeps/i386/dl-trampoline.S +++ b/sysdeps/i386/dl-trampoline.S @@ -17,6 +17,7 @@ <http://www.gnu.org/licenses/>. */ #include <sysdep.h> +#include <link-defines.h> .text .globl _dl_runtime_resolve @@ -161,24 +162,32 @@ _dl_runtime_profile: +4 free %esp free */ - subl $20, %esp - cfi_adjust_cfa_offset (20) - movl %eax, (%esp) - movl %edx, 4(%esp) - fstpt 8(%esp) - fstpt 20(%esp) +#if LONG_DOUBLE_SIZE != 12 +# error "long double size must be 12 bytes" +#endif + # Allocate space for La_i86_retval and subtract 12 free bytes. + subl $(LRV_SIZE - 12), %esp + cfi_adjust_cfa_offset (LRV_SIZE - 12) + movl %eax, LRV_EAX_OFFSET(%esp) + movl %edx, LRV_EDX_OFFSET(%esp) + fstpt LRV_ST0_OFFSET(%esp) + fstpt LRV_ST1_OFFSET(%esp) pushl %esp cfi_adjust_cfa_offset (4) - leal 36(%esp), %ecx - movl 56(%esp), %eax - movl 60(%esp), %edx + # Address of La_i86_regs area. + leal (LRV_SIZE - 12 + 4 + 12)(%esp), %ecx + # PLT2 + movl (LRV_SIZE - 12 + 4 + 32)(%esp), %eax + # PLT1 + movl (LRV_SIZE - 12 + 4 + 36)(%esp), %edx call _dl_call_pltexit - movl (%esp), %eax - movl 4(%esp), %edx - fldt 20(%esp) - fldt 8(%esp) - addl $60, %esp - cfi_adjust_cfa_offset (-60) + movl LRV_EAX_OFFSET(%esp), %eax + movl LRV_EDX_OFFSET(%esp), %edx + fldt LRV_ST1_OFFSET(%esp) + fldt LRV_ST0_OFFSET(%esp) + # Restore stack before return. + addl $(LRV_SIZE - 12 + 4 + 36), %esp + cfi_adjust_cfa_offset (-(LRV_SIZE - 12 + 4 + 36)) ret cfi_endproc .size _dl_runtime_profile, .-_dl_runtime_profile diff --git a/sysdeps/i386/link-defines.sym b/sysdeps/i386/link-defines.sym new file mode 100644 index 0000000..532a916 --- /dev/null +++ b/sysdeps/i386/link-defines.sym @@ -0,0 +1,11 @@ +#include "link.h" +#include <stddef.h> + +-- +LONG_DOUBLE_SIZE sizeof (long double) + +LRV_SIZE sizeof (struct La_i86_retval) +LRV_EAX_OFFSET offsetof (struct La_i86_retval, lrv_eax) +LRV_EDX_OFFSET offsetof (struct La_i86_retval, lrv_edx) +LRV_ST0_OFFSET offsetof (struct La_i86_retval, lrv_st0) +LRV_ST1_OFFSET offsetof (struct La_i86_retval, lrv_st1) -- 2.4.3