This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Add Prefer_MAP_32BIT_EXEC for Silvermont
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Fri, 11 Dec 2015 15:02:54 -0200
- Subject: Re: [PATCH] Add Prefer_MAP_32BIT_EXEC for Silvermont
- Authentication-results: sourceware.org; auth=none
- References: <20151211143706 dot GA7868 at intel dot com> <alpine dot DEB dot 2 dot 10 dot 1512111539300 dot 17023 at digraph dot polyomino dot org dot uk> <CAMe9rOqbqyFw3CMa35vwOEefdFq1xK2Q9hX8GXoGMKVZ-A2y0g at mail dot gmail dot com> <566AF894 dot 4060300 at linaro dot org> <CAMe9rOr-LypZXvq4Y4uwE_JybYoTXctZXMLjo4TH517NnC6omg at mail dot gmail dot com>
On 11-12-2015 14:40, H.J. Lu wrote:
> On Fri, Dec 11, 2015 at 8:23 AM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>> On 11-12-2015 13:59, H.J. Lu wrote:
>>> On Fri, Dec 11, 2015 at 7:39 AM, Joseph Myers <joseph@codesourcery.com> wrote:
>>>>> On Fri, 11 Dec 2015, H.J. Lu wrote:
>>>>>
>>>>>>> +++ b/sysdeps/unix/sysv/linux/x86_64/64/mmap.c
>>>>>>> @@ -0,0 +1,49 @@
>>>>>>> +/* Copyright (C) 2015 Free Software Foundation, Inc.
>>>>>
>>>>> All new files should have a descriptive first line before the copyright
>>>>> notice.
>>>>>
>>> Here is the updated patch.
>>
>>>
>>> diff --git a/sysdeps/unix/sysv/linux/x86_64/64/mmap.c b/sysdeps/unix/sysv/linux/x86_64/64/mmap.c
>>> new file mode 100644
>>> index 0000000..c34f633
>>> --- /dev/null
>>> +++ b/sysdeps/unix/sysv/linux/x86_64/64/mmap.c
>>
>>> +
>>> +__ptr_t
>>> +__mmap (__ptr_t addr, size_t len, int prot, int flags, int fd, off_t offset)
>>> +{
>>> + /* If the Prefer_MAP_32BIT_EXEC bit is set, try to map executable pages
>>> + with MAP_32BIT first. */
>>> + if (addr == NULL
>>> + && (prot & PROT_EXEC) != 0
>>> + && HAS_ARCH_FEATURE (Prefer_MAP_32BIT_EXEC))
>>> + {
>>> + addr = (__ptr_t) INLINE_SYSCALL (mmap, 6, addr, len, prot,
>>> + flags | MAP_32BIT,
>>> + fd, offset);
>>> + if (addr != MAP_FAILED)
>>> + return addr;
>>> + }
>>> + return (__ptr_t) INLINE_SYSCALL (mmap, 6, addr, len, prot, flags,
>>> + fd, offset);
>>> +}
>>> +
>>
>> I would advise not add another syscall variant implementation, but rather
>> work on make a generic implementation with adjustments made by each platform.
>> Something like
>>
>> __ptr_t
>> __mmap (__ptr_t addr, size_t len, int prot, int flags, int fd, off_t offset)
>> {
>> flags = MMAP_ARCH_FLAGS (flags);
>> return (__ptr_t) INLINE_SYSCALL( mmap, 6, addr, len, prot, flags, fd, offset);
>> }
>>
>> And then define MMAP_ARCH_FLAG for x86 to add the MAP_32BIT when required.
>>
>
> This won't work here since we fallback to the default mmap when
> MAP_32BIT fails.
So just change the MMAP_ARCH_FLAGS hook to something like:
__ptr_t ret = __mmap_arch (addr, len, prot, flags, fd, offset));
if (ret != MAP_FAILED)
ret = INLINE_SYSCALL (mmap, 6, addr, len, prot, flags, fd, offset);
return ret
The default value for __mmap_arch could be a constant value so compiler
could remove the the comparison if it is the case.
Another issue is this is basically limiting ALSR really hard on x86_64.
I also would prefer to make the default to *not* include this flag and
set the env. variable to actually enable it. If the cpu is slow doing
what's intended because it is buggy, let it be slow at default. Do not
break what was intended (full ALSR).