linux-kernel - Re: [PATCH] MIPS: Prevent READ_IMPLIES

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 17 Jul 2020 11:00:18 +0100 (BST)
From:   "Maciej W. Rozycki" <macro@....com>
To:     Tiezhu Yang <yangtiezhu@...ngson.cn>
cc:     Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org,
        Kees Cook <keescook@...omium.org>,
        Xuefeng Li <lixuefeng@...ngson.cn>,
        Juxin Gao <gaojuxin@...ngson.cn>,
        "Maciej W. Rozycki" <macro@...ux-mips.org>
Subject: Re: [PATCH] MIPS: Prevent READ_IMPLIES_EXEC propagation

On Wed, 8 Jul 2020, Tiezhu Yang wrote:

> >> In the MIPS architecture, we should clear the security-relevant
> >> flag READ_IMPLIES_EXEC in the function SET_PERSONALITY2() of the
> >> file arch/mips/include/asm/elf.h.
> >>
> >> Otherwise, with this flag set, PROT_READ implies PROT_EXEC for
> >> mmap to make memory executable that is not safe, because this
> >> condition allows an attacker to simply jump to and execute bytes
> >> that are considered to be just data [1].
> >   Why isn't the arrangement made with `mips_elf_read_implies_exec'
> > sufficient?
> 
> We inherit the READ_IMPLIES_EXEC personality flag across fork().
> If we do not explicitly clear this flag in SET_PERSONALITY2(),
> PROT_READ implies PROT_EXEC for mmap to make memory executable
> even if used with the GCC option "-z noexecstack" when compile.

 It makes no sense to me to repeat this across all the architectures, and 
even less so to do it individually one by one as people rediscover this 
issue.

 Why don't we maintain the flag globally in `fs/binfmt_elf.c' which is 
where we already set it?  E.g.:

	SET_PERSONALITY2(*elf_ex, &arch_state);
	if (elf_read_implies_exec(*elf_ex, executable_stack))
		current->personality |= READ_IMPLIES_EXEC;
	else
		current->personality &= ~READ_IMPLIES_EXEC;

  Maciej