Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: A1UpebW7/PWJ6JLdUyFI7rQX4qZyGjpeQD33D4uTqS44rt+BfepdRBh8XUYfACx9EbdKYk/Y2c
 3vtsWoYBzuTaJLG220LBxr7568mVNSLKAAraPjR6dR9UzHKO+bADvE6zcXQJC1sakToN62vo5q
 NgbeiyvBm87JPPUCUxFtmnQ666fS+65aUbaNTcnZkyFIImA4/SOJ6L/1mSip2xCoTzf5QnWozc
 5yqm1N8mVC49V27cM5E1vGSCGqeyhffyvKWeGVGUi0dWx+YioAbN0t2lQmmPrhsNCoLxJCcY17
 udU=
IronPort-SDR: f9vfUf2pFQWJ8+lNQ04XXBbuhljU2nuLirptshi7O7+2iWJSHKEb0ZldaqpO5YoQHH4B87NSyj
 2aH+BnDRDA07sX6hY9oSpvpmHYthkqk70=
IronPort-SDR: Few0pq9UDnEi1rkPglf8o17OfuANd9zMmPZqD5dk+SyOrI9wD0xwtIiGehCM8iIdlFweLIXJ/l
 aZH7rRhluDzA==
WDCIronportException: Internal
Date:   Fri, 17 Jul 2020 11:00:18 +0100 (BST)
From:   "Maciej W. Rozycki" <macro@wdc.com>
To:     Tiezhu Yang <yangtiezhu@loongson.cn>
cc:     Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org,
        Kees Cook <keescook@chromium.org>,
        Xuefeng Li <lixuefeng@loongson.cn>,
        Juxin Gao <gaojuxin@loongson.cn>,
        "Maciej W. Rozycki" <macro@linux-mips.org>
Subject: Re: [PATCH] MIPS: Prevent READ_IMPLIES_EXEC propagation
In-Reply-To: <b62c76c5-3548-c3d5-ad3e-5664c463a52f@loongson.cn>
Message-ID: <alpine.LFD.2.21.2007171044320.24175@redsun52.ssa.fujisawa.hgst.com>
References: <1594114741-26852-1-git-send-email-yangtiezhu@loongson.cn> <alpine.LFD.2.21.2007072043250.31807@redsun52.ssa.fujisawa.hgst.com> <b62c76c5-3548-c3d5-ad3e-5664c463a52f@loongson.cn>
User-Agent: Alpine 2.21 (LFD 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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