Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <103853ef-3afb-bb94-5ffd-8318d1a1d1a0@intel.com>
Date:   Fri, 11 Mar 2022 13:23:26 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Content-Language: en-US
To:     Nadav Amit <nadav.amit@gmail.com>
Cc:     Linux-MM <linux-mm@kvack.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Andrew Cooper <andrew.cooper3@citrix.com>,
        Andy Lutomirski <luto@kernel.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Peter Xu <peterx@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Will Deacon <will@kernel.org>, Yu Zhao <yuzhao@google.com>,
        Nick Piggin <npiggin@gmail.com>,
        "x86@kernel.org" <x86@kernel.org>
References: <20220311190749.338281-1-namit@vmware.com>
 <20220311190749.338281-3-namit@vmware.com>
 <a2a43395-b848-a4f9-4065-109387680701@intel.com>
 <AC8D21EA-CD32-4F9F-B5C1-ED8804EC76FF@vmware.com>
 <70e08bd5-187a-daee-2822-1d9a437a9cff@intel.com>
 <E46E0DA7-6427-4733-9D43-6CC1107194D6@gmail.com>
From:   Dave Hansen <dave.hansen@intel.com>
Subject: Re: [RESEND PATCH v3 2/5] x86/mm: check exec permissions on fault
In-Reply-To: <E46E0DA7-6427-4733-9D43-6CC1107194D6@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 3/11/22 13:16, Nadav Amit wrote:
>> This is really about checking the sanity of the "hardware"-provided
>> error code.  Let's just do it in  handle_page_fault(), maybe hidden in a
>> function like:
>>
>> void check_error_code_sanity(unsigned long error_code)
>> {
>> 	WARN_ON_ONCE(...);
>> }
>>
>> You can leave the X86_PF_PK check in place for now.  It's probably going
>> away soon anyway.
> Done. Thanks. But note that removing the check from access_error() means
> that if the assertion is broken, userspace might crash inadvertently
> (in contrast to the version I sent, which would have potentially led to
> infinite stream of page-faults). I don’t know which behavior is better,
> so let’s go with your version and just hope it doesn’t happen.

Actually, crashing sounds much nicer to me than infinite page faults.
It's a lot easier to debug, *especially* with a warning on dmesg.