Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9f92d530-1cfc-6e50-a717-321ac64ed1c2@redhat.com>
Date:   Wed, 19 Apr 2023 11:07:04 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH] mm/mmap: Map MAP_STACK to VM_STACK
Content-Language: en-US
To:     Matthew Wilcox <willy@infradead.org>
Cc:     Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, Joe Mario <jmario@redhat.com>,
        Barry Marson <bmarson@redhat.com>,
        Rafael Aquini <aquini@redhat.com>
References: <20230418210230.3495922-1-longman@redhat.com>
 <20230418141852.75e551e57e97f4b522957c5c@linux-foundation.org>
 <6c3c68b1-c4d4-dd82-58e8-f7013fb6c8e5@redhat.com>
 <ZD9kKTsPVlzlEZLp@casper.infradead.org>
From:   Waiman Long <longman@redhat.com>
In-Reply-To: <ZD9kKTsPVlzlEZLp@casper.infradead.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 4/18/23 23:46, Matthew Wilcox wrote:
> On Tue, Apr 18, 2023 at 09:16:37PM -0400, Waiman Long wrote:
>>   1) App runs creating lots of threads.
>>   2) It mmap's 256K pages of anonymous memory.
>>   3) It writes executable code to that memory.
>>   4) It calls mprotect() with PROT_EXEC on that memory so
>>      it can subsequently execute the code.
>>
>> The above mprotect() will fail if the mmap'd region's VMA gets merged with
>> the VMA for one of the thread stacks.  That's because the default RHEL
>> SELinux policy is to not allow executable stacks.
> By the way, this is a daft policy.  The policy you really want is
> EXEC|WRITE is not allowed.  A non-writable stack is useless, so it's
> actually a superset of your current policy.  Forbidding _simultaneous_
> write and executable is just good programming.  This way, you don't need
> to care about the underlying VMA's current permissions, you just need
> to do:
>
> 	if ((prot & (PROT_EXEC|PROT_WRITE)) == (PROT_EXEC|PROT_WRITE))
> 		return -EACCESS;

I am not totally sure if the application changes the VMA to read-only 
first. Even if it does that, it highlights another possible issue when 
an anonymous VMA is merged with a stack VMA. Either the mprotect() to 
write-protect the VMA will fail or the application will segfault if it 
writes stuff to the stack. This particular issue is not related to 
SELinux. It provides another good idea why we should avoid merging stack 
VMA to anonymous VMA.

Cheers,
Longman