Re: [PATCH v2 3/5] x86/mm: check exec permissions on fault
From: Peter Zijlstra <peterz@infradead.org>
Date: 2021-10-25 11:00:16
Also in:
lkml
On Thu, Oct 21, 2021 at 05:21:10AM -0700, Nadav Amit wrote:
From: Nadav Amit <redacted>
Add a check to prevent access_error() from returning mistakenly that page-faults due to instruction fetch are not allowed. Intel SDM does not indicate whether "instruction fetch" and "write" in the hardware error code are mutual exclusive, so check both before returning whether the access is allowed.
Dave, can we get that clarified? It seems a bit naf and leads to confusing code IMO. Other than that, the change looks ok to me.
quoted hunk ↗ jump to hunk
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index b2eefdefc108..e776130473ce 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c@@ -1100,10 +1100,17 @@ access_error(unsigned long error_code, struct vm_area_struct *vma) (error_code & X86_PF_INSTR), foreign)) return 1; - if (error_code & X86_PF_WRITE) { + if (error_code & (X86_PF_WRITE | X86_PF_INSTR)) { /* write, present and write, not present: */ - if (unlikely(!(vma->vm_flags & VM_WRITE))) + if ((error_code & X86_PF_WRITE) && + unlikely(!(vma->vm_flags & VM_WRITE))) return 1; + + /* exec, present and exec, not present: */ + if ((error_code & X86_PF_INSTR) && + unlikely(!(vma->vm_flags & VM_EXEC))) + return 1; + return 0; }