ppc64 sbrk returns executable heap in 32-bit emulation mode

ppc64 sbrk returns executable heap in 32-bit emulation mode

[Florian Weimer]

We noticed that on ppc64, the sbrk system call in the 32-bit subsystem 
returns executable memory.  I assume it is related to this, in 
arch/powerpc/include/asm/page.h:

/*
  * Unfortunately the PLT is in the BSS in the PPC32 ELF ABI,
  * and needs to be executable.  This means the whole heap ends
  * up being executable.
  */
#define VM_DATA_DEFAULT_FLAGS32 (VM_READ | VM_WRITE | VM_EXEC | \
                                  VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)


What is the rationale for this?  This comment must be *really* old, 
because ld.so in glibc should make sure that the PLT is executable.  And 
for current binaries, .bss is *not* executable, contrary to what the 
comment suggests.

Is this comment about pre-ELF binaries?  If yes, would it possible to 
change the default for ELF binaries?

Thanks,
Florian

Re: ppc64 sbrk returns executable heap in 32-bit emulation mode

[Alan Modra]

On Thu, May 12, 2016 at 03:41:09PM +0200, Florian Weimer wrote:
> We noticed that on ppc64, the sbrk system call in the 32-bit subsystem 
> returns executable memory.  I assume it is related to this, in 
> arch/powerpc/include/asm/page.h:
> 
> /*
>   * Unfortunately the PLT is in the BSS in the PPC32 ELF ABI,
>   * and needs to be executable.  This means the whole heap ends
>   * up being executable.
>   */
> #define VM_DATA_DEFAULT_FLAGS32 (VM_READ | VM_WRITE | VM_EXEC | \
>                                   VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
> 
> 
> What is the rationale for this?  This comment must be *really* old, 

I think the comment is just plain wrong.  ppc32 needs an executable
stack because it builds trampolines on the stack to support calling
nested functions.  I presume that's why the heap is executable.  (If
I'm wrong about heap+stack needing the same protection then I can't
think of any reason to require an executable heap.)

> because ld.so in glibc should make sure that the PLT is executable.  And 
> for current binaries, .bss is *not* executable, contrary to what the 
> comment suggests.
> 
> Is this comment about pre-ELF binaries?  If yes, would it possible to 
> change the default for ELF binaries?
> 
> Thanks,
> Florian

-- 
Alan Modra
Australia Development Lab, IBM

Re: ppc64 sbrk returns executable heap in 32-bit emulation mode

[Florian Weimer]

On 05/16/2016 08:24 AM, Alan Modra wrote:
> On Thu, May 12, 2016 at 03:41:09PM +0200, Florian Weimer wrote:
>> We noticed that on ppc64, the sbrk system call in the 32-bit subsystem
>> returns executable memory.  I assume it is related to this, in
>> arch/powerpc/include/asm/page.h:
>>
>> /*
>>   * Unfortunately the PLT is in the BSS in the PPC32 ELF ABI,
>>   * and needs to be executable.  This means the whole heap ends
>>   * up being executable.
>>   */
>> #define VM_DATA_DEFAULT_FLAGS32 (VM_READ | VM_WRITE | VM_EXEC | \
>>                                   VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
>>
>>
>> What is the rationale for this?  This comment must be *really* old,
>
> I think the comment is just plain wrong.  ppc32 needs an executable
> stack because it builds trampolines on the stack to support calling
> nested functions.  I presume that's why the heap is executable.  (If
> I'm wrong about heap+stack needing the same protection then I can't
> think of any reason to require an executable heap.)

But the stack is *not* executable.  The program is marked correctly with 
GNU_STACK:

   GNU_STACK      0x000000 0x00000000 0x00000000 0x000000 0x000000 RW  0x10

If this program header were not there (or said RWX instead of RW), then 
I expect the kernel would make the stack executable, for backwards 
compatibility.

And the test reports this:

# Subtests with executable mappings or inconsistent results
        exec  MAP_ANONYMOUS with R|W, then mprotect R|X
        exec  MAP_ANONYMOUS with R|X, then mprotect R|W, mprotect R|X
   FAIL exec  malloc (small) (unexpected result)
   FAIL exec  malloc (page size) (unexpected result)
        exec  MAP_ANONYMOUS, with PROT_EXEC, without mprotect
        exec  file with MAP_PRIVATE, with PROT_EXEC, without mprotect, 
without alias
        exec  file with MAP_SHARED, with PROT_EXEC, without mprotect, 
without alias
        exec  shared memory with MAP_PRIVATE, with PROT_EXEC, without 
mprotect, without alias
        exec  shared memory with MAP_SHARED, with PROT_EXEC, without 
mprotect, without alias
        exec  file with MAP_SHARED, with PROT_EXEC, without mprotect, 
with alias
        exec  shared memory with MAP_SHARED, with PROT_EXEC, without 
mprotect, with alias
        exec  MAP_ANONYMOUS, with PROT_EXEC, with mprotect
        exec  file with MAP_PRIVATE, with PROT_EXEC, with mprotect, 
without alias
        exec  file with MAP_SHARED, with PROT_EXEC, with mprotect, 
without alias
        exec  shared memory with MAP_PRIVATE, with PROT_EXEC, with 
mprotect, without alias
        exec  shared memory with MAP_SHARED, with PROT_EXEC, with 
mprotect, without alias
        exec  file with MAP_SHARED, with PROT_EXEC, with mprotect, with 
alias
        exec  shared memory with MAP_SHARED, with PROT_EXEC, with 
mprotect, with alias

(Sorry for the line wrapping, also available at 
<https://paste.fedoraproject.org/367036/>.)

The tests I'm using are these: <https://pagure.io/execmod-tests>  It 
also covers the stack, but it is not listed in the summary because it is 
not executable.

Only the heap is executable.  Everything else is set up as expected by 
the kernel or by glibc.

Florian

Re: ppc64 sbrk returns executable heap in 32-bit emulation mode

[Andreas Schwab]

Alan Modra <amodra@gmail.com> writes:

> On Thu, May 12, 2016 at 03:41:09PM +0200, Florian Weimer wrote:
>> We noticed that on ppc64, the sbrk system call in the 32-bit subsystem 
>> returns executable memory.  I assume it is related to this, in 
>> arch/powerpc/include/asm/page.h:
>> 
>> /*
>>   * Unfortunately the PLT is in the BSS in the PPC32 ELF ABI,
>>   * and needs to be executable.  This means the whole heap ends
>>   * up being executable.
>>   */
>> #define VM_DATA_DEFAULT_FLAGS32 (VM_READ | VM_WRITE | VM_EXEC | \
>>                                   VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
>> 
>> 
>> What is the rationale for this?  This comment must be *really* old, 
>
> I think the comment is just plain wrong.  ppc32 needs an executable
> stack because it builds trampolines on the stack to support calling
> nested functions.  I presume that's why the heap is executable.

The stack and the heap are separate mappings.

>  (If
> I'm wrong about heap+stack needing the same protection then I can't
> think of any reason to require an executable heap.)

The heap and the BSS initially share the same page.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: ppc64 sbrk returns executable heap in 32-bit emulation mode

[Florian Weimer]

On 05/16/2016 10:49 AM, Andreas Schwab wrote:

>>  (If
>> I'm wrong about heap+stack needing the same protection then I can't
>> think of any reason to require an executable heap.)
>
> The heap and the BSS initially share the same page.

But my test says that at least part of .bss in the main executable is 
*not* executable.

Any ideas why the kernel behaves the way it does?  Can we change the 
behavior at least for ELF binaries which use glibc (or something else 
which indicates a non-legacy binary)?

Thanks,
Florian

Re: ppc64 sbrk returns executable heap in 32-bit emulation mode

[Andreas Schwab]

Florian Weimer <fweimer@redhat.com> writes:

> But my test says that at least part of .bss in the main executable is
> *not* executable.

Build with -mbss-plt -Wl,--bss-plt.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: ppc64 sbrk returns executable heap in 32-bit emulation mode

[Florian Weimer]

On 05/16/2016 11:09 AM, Andreas Schwab wrote:
> Florian Weimer <fweimer@redhat.com> writes:
>
>> But my test says that at least part of .bss in the main executable is
>> *not* executable.
>
> Build with -mbss-plt -Wl,--bss-plt.

This gives me:

   FAIL exec  .bss data (unexpected result)
   FAIL exec  .data (unexpected result)
   FAIL exec  malloc (small) (unexpected result)
   FAIL exec  malloc (page size) (unexpected result)

And this is somewhat expected: There is an RWX LOAD segment which covers 
.bss and .data, and it also overlaps with the heap.

So maybe we can change the kernel heap default in case there is no RWX 
LOAD segment, so that most binaries get a non-executable heap?  What do 
you think?

Thanks,
Florian