Re: [PATCH v3 3/3] MIPS: set stack/data protection as non-executable

[Leonid Yegoshin]

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.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/