lkml.org 
[lkml]   [2019]   [Apr]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] binfmt_elf: Update READ_IMPLIES_EXEC logic for modern CPUs

* Kees Cook <keescook@chromium.org> wrote:

> The READ_IMPLIES_EXEC work-around was designed for old CPUs lacking NX
> (to have the visible permission flags on memory regions reflect reality:
> they are all executable), and for old toolchains that lacked the ELF
> PT_GNU_STACK marking (under the assumption that toolchains that couldn't
> even specify memory protection flags may have it wrong for all memory
> regions).
>
> This logic is sensible, but was implemented in a way that equated having
> a PT_GNU_STACK marked executable as being as "broken" as lacking the
> PT_GNU_STACK marking entirely. This is not a reasonable assumption
> for CPUs that have had NX support from the start (or very close to
> the start). This confusion has led to situations where modern 64-bit
> programs with explicitly marked executable stack are forced into the
> READ_IMPLIES_EXEC state when no such thing is needed. (And leads to
> unexpected failures when mmap()ing regions of device driver memory that
> wish to disallow VM_EXEC[1].)
>
> To fix this, elf_read_implies_exec() is adjusted on arm64 (where NX has
> always existed and toolchains have implemented PT_GNU_STACK for a while),
> and x86 is adjusted to handle this combination of possible outcomes:
>
> CPU: | lacks NX | has NX, ia32 | has NX, x86_64 |
> ELF: | | | |
> ------------------------------|------------------|------------------|
> missing GNU_STACK | needs RIE | needs RIE | no RIE |
> GNU_STACK == RWX | needs RIE | no RIE: stack X | no RIE: stack X |
> GNU_STACK == RW | needs RIE | no RIE: stack NX | no RIE: stack NX |
>
> This has the effect of making binfmt_elf's EXSTACK_DEFAULT actually take
> on the correct architecture default of being non-executable on arm64 and
> x86_64, and being executable on ia32.

Just to make clear, is the change from the old behavior, in essence:


CPU: | lacks NX | has NX, ia32 | has NX, x86_64 |
ELF: | | | |
------------------------------|------------------|------------------|
missing GNU_STACK | exec-all | exec-all | exec-none |
- GNU_STACK == RWX | exec-all | exec-all | exec-all |
+ GNU_STACK == RWX | exec-all | exec-stack | exec-stack |
GNU_STACK == RW | exec-all | exec-none | exec-none |

correct?

Also note that in addition to marking the changes clearly, I also edited
the table to be less confusing and more assertive:

'exec-all' : all user mappings are executable
'exec-none' : only PROT_EXEC user mappings are executable
'exec-stack': only the stack and PROT_EXEC user mappings are executable

Is this correct? (Please double check the edited table.)

In particular, what is the policy for write-only and exec-only mappings,
what does read-implies-exec do for them?

Also, it would be nice to define it precisely what 'stack' means in this
context: it's only the ELF loader defined process stack - other stacks
such as any thread stacks, signal stacks or alt-stacks depend on the C
library - or does the kernel policy extend there too?

I.e. it would be nice to clarify all this, because it's still rather
confusing and ambiguous right now.

Thanks,

Ingo

\
 
 \ /
  Last update: 2019-04-25 07:43    [W:0.080 / U:5.584 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site