Add Prefer_MAP_32BIT_EXEC to map executable pages with MAP_32BIT

Message ID CAMe9rOo1OBOGruWMoLTx96wnkKUYPzBZ5HcOCFdGJEp+jTRzVA@mail.gmail.com
State New, archived
Headers

Commit Message

H.J. Lu Dec. 15, 2015, 9:34 p.m. UTC
  On Tue, Dec 15, 2015 at 1:06 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 12/15/2015 03:08 PM, H.J. Lu wrote:
>> On Tue, Dec 15, 2015 at 10:38 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>>> > On 12/15/2015 01:27 PM, Carlos O'Donell wrote:
>>>> >> +  cpu_features->feature[index_Prefer_MAP_32BIT_EXEC]
>>>> >> +    |= get_prefer_map_32bit_exec ();
>>>> >>
>>>> >> You wouldn't need get_prefer_map_32bit_exec, since this is all part of
>>>> >> the code, like dl-librecon.h, which parses the extra env var.
>>> >
>>> > To be clear:
>>> >
>>> > * Add new bit flag definitions for cpu_features.
>>> > * Add a sysdeps/unix/sysv/linux/x86_64/dl-silvermont.h
>>> >   * Fill in EXTRA_LD_ENVVARS or add new ones.
>>> >   * Write to rtld's GLRO cpu_features the bit you need based
>>> >     on __libc_enable_secure.
>>> >
>>> > That should simplify and concentrate the Silvermont fixes to
>>> > just two files, making future maintenance and documentation
>>> > easier.
>>> >
>>> >
>> This is the updated patch.  I put EXTRA_LD_ENVVARS and
>> EXTRA_UNSECURE_ENVVARS in x86_64/64/dl-librecon.h
>> to be consistent with i386/dl-librecon.h.  If we ever need to
>> update EXTRA_LD_ENVVARS/EXTRA_UNSECURE_ENVVARS
>> in the future, we have a single file to change.
>>
>> Tested on x86-64.  OK for master?
>>
>> Thanks for all the feedbacks and suggestions.
>
> This looks much better and much cleaner. Looks good to me now. It also
> appears you have consesnsus with this last change.
>
> It needs a bug # please since you're fixing a user-visible performance
> problem on Silvermont.

I opened

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

and checked in my patch.

> It appears to meet Zack's requirement to choose a security safe default
> at the expense of performance (I agree with that).
>
> I *strongly* urge you to immediately submit a patch to the linux man
> pages project to document that as of 2.23 this new flag exists and
> does what you describe it does.
>

Here is a patch for Linux man page.

Thanks.


H.J.
  

Comments

Michael Kerrisk \(man-pages\) Dec. 16, 2015, 2:02 p.m. UTC | #1
On 12/15/2015 10:34 PM, H.J. Lu wrote:
> On Tue, Dec 15, 2015 at 1:06 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>> On 12/15/2015 03:08 PM, H.J. Lu wrote:
>>> On Tue, Dec 15, 2015 at 10:38 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>>> On 12/15/2015 01:27 PM, Carlos O'Donell wrote:
>>>>>>> +  cpu_features->feature[index_Prefer_MAP_32BIT_EXEC]
>>>>>>> +    |= get_prefer_map_32bit_exec ();
>>>>>>>
>>>>>>> You wouldn't need get_prefer_map_32bit_exec, since this is all part of
>>>>>>> the code, like dl-librecon.h, which parses the extra env var.
>>>>>
>>>>> To be clear:
>>>>>
>>>>> * Add new bit flag definitions for cpu_features.
>>>>> * Add a sysdeps/unix/sysv/linux/x86_64/dl-silvermont.h
>>>>>   * Fill in EXTRA_LD_ENVVARS or add new ones.
>>>>>   * Write to rtld's GLRO cpu_features the bit you need based
>>>>>     on __libc_enable_secure.
>>>>>
>>>>> That should simplify and concentrate the Silvermont fixes to
>>>>> just two files, making future maintenance and documentation
>>>>> easier.
>>>>>
>>>>>
>>> This is the updated patch.  I put EXTRA_LD_ENVVARS and
>>> EXTRA_UNSECURE_ENVVARS in x86_64/64/dl-librecon.h
>>> to be consistent with i386/dl-librecon.h.  If we ever need to
>>> update EXTRA_LD_ENVVARS/EXTRA_UNSECURE_ENVVARS
>>> in the future, we have a single file to change.
>>>
>>> Tested on x86-64.  OK for master?
>>>
>>> Thanks for all the feedbacks and suggestions.
>>
>> This looks much better and much cleaner. Looks good to me now. It also
>> appears you have consesnsus with this last change.
>>
>> It needs a bug # please since you're fixing a user-visible performance
>> problem on Silvermont.
> 
> I opened
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=19367
> 
> and checked in my patch.
> 
>> It appears to meet Zack's requirement to choose a security safe default
>> at the expense of performance (I agree with that).
>>
>> I *strongly* urge you to immediately submit a patch to the linux man
>> pages project to document that as of 2.23 this new flag exists and
>> does what you describe it does.

(Thanks, Carlos.)

> Here is a patch for Linux man page.

Thanks, H.J. I applied the patch and tweaked your text somewhat.
Does the following look okay?

       LD_PREFER_MAP_32BIT_EXEC
              (x86-64 only)(glibc since 2.23)  According  to  the  Intel
              Silvermont  software optimization guide, for 64-bit appli‐
              cations, branch prediction performance can  be  negatively
              impacted when the target of a branch is more than 4GB away
              from the branch.  If this environment variable is set  (to
              any  value),  ld.so will first try to map executable pages
              using the mmap(2) MAP_32BIT flag, and fall back to mapping
              without  that  flag  if that attempt fails.  NB: MAP_32BIT
              will map to the low 2GB (not 2GB) of  the  address  space.
              Because  MAP_32BIT reduces the address range available for
              address  space  layout   randomization   (ASLR),   LD_PRE‐
              FER_MAP_32BIT_EXEC  is always disabled in secure-execution
              mode.

Thanks,

Michael
  
Carlos O'Donell Dec. 16, 2015, 2:04 p.m. UTC | #2
On 12/16/2015 09:02 AM, Michael Kerrisk (man-pages) wrote:
> (Thanks, Carlos.)
> 
>> Here is a patch for Linux man page.
> 
> Thanks, H.J. I applied the patch and tweaked your text somewhat.
> Does the following look okay?
> 
>        LD_PREFER_MAP_32BIT_EXEC
>               (x86-64 only)(glibc since 2.23)  According  to  the  Intel
>               Silvermont  software optimization guide, for 64-bit appli‐
>               cations, branch prediction performance can  be  negatively
>               impacted when the target of a branch is more than 4GB away
>               from the branch.  If this environment variable is set  (to
>               any  value),  ld.so will first try to map executable pages
>               using the mmap(2) MAP_32BIT flag, and fall back to mapping
>               without  that  flag  if that attempt fails.  NB: MAP_32BIT
>               will map to the low 2GB (not 2GB) of  the  address  space.
>               Because  MAP_32BIT reduces the address range available for
>               address  space  layout   randomization   (ASLR),   LD_PRE‐
>               FER_MAP_32BIT_EXEC  is always disabled in secure-execution
>               mode.

Looks great. Thanks for the quick turnaround Michael!

Cheers,
Carlos.
  
Phil Blundell Dec. 16, 2015, 2:16 p.m. UTC | #3
On Wed, 2015-12-16 at 15:02 +0100, Michael Kerrisk (man-pages) wrote:
> 
> NB: MAP_32BIT will map to the low 2GB (not 2GB) of  the  address
> space.

Isn't that meant to say "not 4GB"?

p.
  
Michael Kerrisk \(man-pages\) Dec. 16, 2015, 2:33 p.m. UTC | #4
On 12/16/2015 03:16 PM, Phil Blundell wrote:
> On Wed, 2015-12-16 at 15:02 +0100, Michael Kerrisk (man-pages) wrote:
>>
>> NB: MAP_32BIT will map to the low 2GB (not 2GB) of  the  address
>> space.
> 
> Isn't that meant to say "not 4GB"?

Quite! Thanks, Phil.

Cheers,

Michael
  

Patch

From 6b8095fb2e123915ec2e586753d3d7ff7f14aee1 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 15 Dec 2015 13:29:37 -0800
Subject: [PATCH] Document LD_PREFER_MAP_32BIT_EXEC for ld.so (8)

Glibc 2.23 added LD_PREFER_MAP_32BIT_EXEC to improve branch prediction
performance on Silvermont.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 man8/ld.so.8 | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/man8/ld.so.8 b/man8/ld.so.8
index 608cf7c..da33404 100644
--- a/man8/ld.so.8
+++ b/man8/ld.so.8
@@ -472,6 +472,16 @@  environment variable has been set.
 (ELF only)(glibc since 2.1.3)
 If set to a nonempty string, warn about unresolved symbols.
 .TP
+.B LD_PREFER_MAP_32BIT_EXEC
+(x86-64 only)(glibc since 2.23)
+According to Silvermont software optimization guide, for 64-bit
+applications, branch prediction performance can be negatively impacted
+when the target of a branch is more than 4GB away from the branch.
+If set, ld.so will try to map executable pages with MAP_32BIT first.
+NB: MAP_32BIT will map to lower 2GB, not lower 4GB, address.  Since
+MAP_32BIT reduces bits available for address space layout randomization
+(ASLR), LD_PREFER_MAP_32BIT_EXEC is always disabled for SUID programs.
+.TP
 .B LDD_ARGV0
 (libc5)
 .IR argv [0]
-- 
2.5.0