RE: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter

[Aleksandar Markovic <Aleksandar.Markovic@mips.com>]

> 
> ________________________________________
> From: James Hogan [jhogan@kernel.org]
> Sent: Thursday, February 8, 2018 12:55 PM
> 
> Hi,
> 
> On Thu, Dec 07, 2017 at 11:33:47AM +0000, Miodrag Dinic wrote:
> > > On Wed, Dec 06, 2017 at 05:50:52PM +0000, Maciej W. Rozycki wrote:
> > > >  What problem are you trying to solve anyway?  Is it not something that
> > > > can be handled with the `execstack' utility?
> > >
> > > The commit message states that for Android "non-exec stack is required".
> > > Is Android checking that then Aleksandar? If so, how?
> >
> > Android is using SELinux configured to disallow NX mappings by handling
> > the following sepolicy rules :
> > * Executable stack (execstack)
> > * Executable heap (execheap)
> > * File-based executable code which has been modified (execmod)
> > * All other executable memory (execmem)
> 
> ...
> 
> > The effect of not having some workaround like this in the kernel, would
> > be to run Android only in SELinux permissive mode.
> 
> So you want to override the lack of RIXI so that SELinux sees an
> RX->RW->RX transition as execmod instead of execmem (since without RIXI
> its effectively RX->RWX->RX which is execmem)?
> 

That is correct.

> 
> Looking at file_map_prot_check(), it does the execmem check on this
> condition:
> 
> if (default_noexec &&
>     (prot & PROT_EXEC) && (!file || IS_PRIVATE(file_inode(file)) ||
>                            (!shared && (prot & PROT_WRITE)))) {
>         /*
>          * We are making executable an anonymous mapping or a
>          * private file mapping that will also be writable.
>          * This has an additional check.
>          */
> 
> default_noexec is set if VM_DATA_DEFAULT_FLAGS doesn't have the exec
> flag set, and that flag depends on current->personality &
> READ_IMPLIES_EXEC, which depends on elf_read_implies_exec(), i.e.
> mips_elf_read_implies_exec(), and that should already return 1 if RIXI
> is unavailable.
> 
> I.e.
> 
> mips_elf_read_implies_exec() == 1
> 
> elf_read_implies_exec() == 1
> 
> READ_IMPLIES_EXEC will be set in current->personality
> 
> VM_DATA_DEFAULT_FLAGS will have VM_EXEC set
> 
> default_noexec will be set to 0 in selinux_init()
> 
> none of the execmem, execheap, execstack, execmod permission
> checks should take place.
> 
> So whats the problem exactly? Perhaps I misinterpreted something.

Thanks, James, for the analysis of this scenario! Hope the additional
info below will be useful for clarifying this matter.

-------------------

Let me rephrase the scenario (for configurations where cpu_has_rixi
equals to 0) that you described: (line numbers may be approximate)


1. mips_elf_read_implies_exec() will return 1
    (arch/mips/kernel/elf.c:336)

2. elf_read_implies_exec() will return 1
    (arch/mips/include/asm/elf.h:513)

3. READ_IMPLIES_EXEC will be set in current->personality
    (fs/binfmt_elf_fdpic.c:357)

4. VM_DATA_DEFAULT_FLAGS will have VM_EXEC set
    (while execiting selinux_init() in security/selinux/hooks.c:6644)

5. default_noexec will be set to 0 in selinux_init()
    (security/selinux/hooks.c:6644)

6. none of the execmem, execstack permission checks should take place.

-------------------

However, in reality, these steps are not executed in this order, but in the following one:


4a. VM_DATA_DEFAULT_FLAGS will *NOT* have VM_EXEC set
    (while execiting selinux_init() in security/selinux/hooks.c:6644)

5a. default_noexec will be set to *1* in selinux_init()
    (security/selinux/hooks.c:6644)

1. mips_elf_read_implies_exec() will return 1
    (arch/mips/kernel/elf.c:336)

2. elf_read_implies_exec() will return 1
    (arch/mips/include/asm/elf.h:513)

3. READ_IMPLIES_EXEC will be set in current->personality
    (fs/binfmt_elf_fdpic.c:357)

6a. both the execmem, execstack permission checks *do* take place.

------------------

The proposed change does affect the kernel in the sense that it indeed
hides the executable vulnerability of the system without RIXI support.
But we have made it unambiguous in the comments what are the
consequences of using this option.

------------------


Regards,

Aleksandar

> 
> Cheers
> James