On 12/05/2014 11:06 AM, David Daney wrote:
On 12/05/2014 10:51 AM, Kees Cook wrote:
On Fri, Dec 5, 2014 at 9:28 AM, David Daney <ddaney.cavm@xxxxxxxxx>
wrote:
Some programs require an executable stack, this patch will break them.
Have you tested this?
Do you require empirical evidence that the patch is incorrect, or is
it enough to just to trust me when I say that it is incorrect?
Typically the burden of proof is with those proposing the patches.
You can only select a non-executable stack in response to PT_GNU_STACK
program headers in the ELF file of the executable program.
This is already handled by fs/binfmt_elf.c. It does the parsing of the
PT_GNU_STACK needs, and sets up the stack flags appropriately. All the
change to VM_DATA_DEFAULT_FLAGS does is make sure that EXSTACK_DEFAULT
now means no VM_EXEC by default. If PT_GNU_STACK requires it, it gets
added back in.
The problem is not with "modern" executables that are properly
annotated with PT_GNU_STACK.
My objection is to the intentional breaking of old executables that
have no PT_GNU_STACK annotation, but require an executable stack.
Since we usually try not to break userspace, we cannot merge a patch
like this one.
I ran Debian, buildroot etc and I had only one problem - ssh_keygen is
built with PT_GNU_STACK annotation stack executable protection. And
debug output shows a spectacular discarding this setup from kernel by
GLIBC loader a couple of milliseconds later, at first library load. You
can test it yourself - run ssh_keygen and look into it's
/proc/<pid>/maps, stack would have +X.
The rest of Debian distribution, buildroot and Android runs fine with
this patch series.
Without any step forward the stack protection would be never solved.
GLIBC team looked into problem and they agree to fix a default
cancellation of no-X but they need a platform for that.
But of course, we can delay this specific non-X setup for stack until
GLIBC fixes loader and do this patch optional or via boot flag.
Note: I push this series not because of default non-X stack but because
an explicit PT_GNU_STACK no-X is ignored.
